diff --git a/CHANGELOG.md b/CHANGELOG.md index d3adc2e47..796fcad80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - Add message pagination over a shared cursor core: new JSON-RPC `chat/history` method and optional `limit`/`before`/`after` window params on `chat/open`, plus opt-in pagination on remote HTTP `GET /api/v1/chats/:id`. Opaque cursors with a `lastCompaction` sentinel; meta exposes before/after/compaction cursors. - Support declaring image input for custom models via a per-model `imageInput` config flag; openai-chat now round-trips tool-result images. (#503) - Bedrock: honor Anthropic thinking variants (effort + adaptive thinking) for Claude models, fixing Claude 4.7+ failing with `thinking.type.enabled` not supported. (#502) +- Reworked system-prompt handling: fix stale static cache, quote-safe context attributes, dedicated workspace-roots/MCP sections, content-named context headings (`## Context`/`## MCP Resources`), and a read-only, reorganized `/prompt-show`. ## 0.140.1 diff --git a/integration-test/integration/chat/commands_test.clj b/integration-test/integration/chat/commands_test.clj index a382a5ef8..19a1ad0cf 100644 --- a/integration-test/integration/chat/commands_test.clj +++ b/integration-test/integration/chat/commands_test.clj @@ -80,9 +80,11 @@ resp)) (match-content chat-id "user" {:type "text" :text "/prompt-show\n"}) - (match-content chat-id "system" {:type "text" :text (m/pred #(and (string/includes? % "You are ECA") - (not (string/includes? % ":static")) - (not (string/includes? % ":dynamic"))))}) + (match-content chat-id "system" {:type "text" :text (m/pred #(and (string/includes? % "# Instructions (System prompt)") + (string/includes? % "You are ECA") + (not (string/includes? % "# Chat (User prompt)")) + (not (string/includes? % "/prompt-show")) + (string/includes? % "Tool schemas are sent separately")))}) (match-content chat-id "system" {:type "progress" :state "finished"})))) (deftest mcp-prompts diff --git a/resources/prompts/additional_system_info.md b/resources/prompts/additional_system_info.md index e9efdff1c..431562f9f 100644 --- a/resources/prompts/additional_system_info.md +++ b/resources/prompts/additional_system_info.md @@ -4,9 +4,3 @@ OS: {{osName}} Default shell: {{shell}} User: {{userName}} Home directory: {{homeDir}} -Workspaces: {{workspaceRoots}} - -**Path Resolution & Context:** -*Workspaces:* Directories containing code or data relevant to this session. -*Rule:* Use Workspaces as the base to resolve relative paths into absolute paths when required by tools. - diff --git a/resources/prompts/code_agent.md b/resources/prompts/code_agent.md index a2453aa44..ccd4d8ecf 100644 --- a/resources/prompts/code_agent.md +++ b/resources/prompts/code_agent.md @@ -13,7 +13,6 @@ For each file, give a short description of what needs to be edited, then use the {% if toolEnabled_eca__editor_diagnostics %} After finishing your changes, use eca__editor_diagnostics to check for diagnostics, making sure you didn't introduce any errors or warnings. {% endif %} - ## Communication The chat is markdown mode. @@ -27,9 +26,9 @@ You have tools at your disposal to solve the coding task. Follow these rules reg 2. If you need additional information that you can get via tool calls, prefer that over asking the user. 3. If you are not sure about file content or codebase structure pertaining to the user's request, use your tools to read files and gather the relevant information: do NOT guess or make up an answer. 4. You have the capability to call multiple tools in a single response, batch your tool calls together for optimal performance. - {% if toolEnabled_eca__task %} ## Task Tracking + You have access to the `eca__task` tool for task management. Use `eca__task` as the canonical task list when you need to plan and track non-trivial, multi-step execution (e.g., multiple tasks, dependencies, or iterative debugging), or when the user explicitly asks for a plan/todo list. Skip it for a single small action or purely informational replies. diff --git a/src/eca/cache.clj b/src/eca/cache.clj index 668d1b365..a35ff3de5 100644 --- a/src/eca/cache.clj +++ b/src/eca/cache.clj @@ -4,6 +4,7 @@ [babashka.fs :as fs] [clojure.java.io :as io] [clojure.string :as string] + [eca.digest :as digest] [eca.logger :as logger]) (:import [java.io File])) @@ -53,11 +54,10 @@ Order-independent: the same set of folders always yields the same hash." [workspaces uri->filename-fn] (let [joined (string/join ":" (sorted-workspace-paths workspaces uri->filename-fn)) - md (java.security.MessageDigest/getInstance "SHA-256") - digest (.digest (doto md (.update (.getBytes joined "UTF-8")))) + digest-bytes (digest/sha-256-bytes joined) encoder (-> (java.util.Base64/getUrlEncoder) (.withoutPadding)) - key (.encodeToString encoder digest)] + key (.encodeToString encoder digest-bytes)] (subs key 0 (min 8 (count key))))) (def ^:private logger-tag "[CACHE]") diff --git a/src/eca/digest.clj b/src/eca/digest.clj new file mode 100644 index 000000000..a12a91477 --- /dev/null +++ b/src/eca/digest.clj @@ -0,0 +1,15 @@ +(ns eca.digest + (:import + [java.nio.charset StandardCharsets] + [java.security MessageDigest])) + +(set! *warn-on-reflection* true) + +(defn sha-256-bytes ^bytes [^String s] + (-> (MessageDigest/getInstance "SHA-256") + (.digest (.getBytes s StandardCharsets/UTF_8)))) + +(defn sha-256-hex ^String [^String s] + (->> (sha-256-bytes s) + (map #(format "%02x" (bit-and % 0xff))) + (apply str))) diff --git a/src/eca/features/chat.clj b/src/eca/features/chat.clj index 335c94b66..78a27c987 100644 --- a/src/eca/features/chat.clj +++ b/src/eca/features/chat.clj @@ -4,8 +4,10 @@ [clojure.java.io :as io] [clojure.set :as set] [clojure.string :as string] + [eca.cache :as cache] [eca.config :as config] [eca.db :as db] + [eca.digest :as digest] [eca.features.background-tasks :as bg] [eca.features.chat.history :as history] [eca.features.chat.lifecycle :as lifecycle] @@ -64,6 +66,29 @@ (str (System/getProperty "user.name") "@ECA" (when (not-empty agent) (str "/" agent)))) +(defn ^:private static-prompt-cache-signature + "SHA-256 cache identity for static prompt reuse." + [refined-contexts static-rules path-scoped-rules skills agent config chat-id all-tools db] + (let [static-contexts (vec (filter f.prompt/static-prompt-context? refined-contexts))] + (digest/sha-256-hex + (pr-str + {:agent agent + :chat-prompt-template (f.prompt/eca-chat-prompt agent config chat-id db) + :workspace-roots (mapv (comp shared/uri->filename :uri) (:workspace-folders db)) + :environment {:os-name (str (System/getProperty "os.name") " " (System/getProperty "os.version")) + :shell (or (System/getenv "SHELL") (System/getenv "ComSpec")) + :user-name (System/getProperty "user.name") + :home-dir (cache/user-home)} + :is-subagent (boolean (get-in db [:chats chat-id :subagent])) + :startup-context (get-in db [:chats chat-id :startup-context]) + :static-contexts static-contexts + :repo-map (when (some #(= :repoMap (:type %)) static-contexts) + :present) + :static-rules (mapv #(select-keys % [:id :name :scope :content]) static-rules) + :path-scoped-rules (mapv #(select-keys % [:id :name :scope :workspace-root :paths :enforce]) path-scoped-rules) + :skills (mapv #(select-keys % [:name :description]) skills) + :tools (sort (map :full-name all-tools))})))) + (defn ^:private prune-tool-results! "Prunes old tool result content from chat history to reduce context size. Walks messages backwards, protecting the most recent tool outputs up to @@ -1451,9 +1476,13 @@ agent))))) repo-map* (delay (f.index/repo-map db config {:as-string? true})) prompt-cache (get-in db [:chats chat-id :prompt-cache]) + static-signature (static-prompt-cache-signature + refined-contexts static-rules path-scoped-rules skills + agent config chat-id all-tools db) instructions (if (and prompt-cache (= (:agent prompt-cache) agent) - (= (:model prompt-cache) full-model)) + (= (:model prompt-cache) full-model) + (= (:static-signature prompt-cache) static-signature)) {:static (:static prompt-cache) :dynamic (f.prompt/build-dynamic-instructions refined-contexts db)} (let [result (f.prompt/build-chat-instructions @@ -1461,6 +1490,7 @@ agent config chat-id all-tools db)] (swap! db* assoc-in [:chats chat-id :prompt-cache] {:static (:static result) + :static-signature static-signature :agent agent :model full-model}) result)) @@ -1470,15 +1500,21 @@ seq (f.prompt/contexts-str repo-map* nil))] [{:type :text :text contexts-str}]) + decision (message->decision message db config) ;; Cursor (and other volatile editor-state) is delivered per-turn in the ;; user message - never the system prompt - and only re-sent when it ;; changed. This keeps the cached system/prefix stable across turns, ;; avoiding llama.cpp full prompt re-processing on every cursor move. #464 + ;; Only normal prompts send this synthesized user message to the model; + ;; /prompt-show and MCP prompts use different messages. editor-state-context (f.prompt/build-editor-state-context refined-contexts) - editor-state-contents (when (and editor-state-context - (not= editor-state-context - (get-in db [:chats chat-id :last-editor-state]))) - (swap! db* assoc-in [:chats chat-id :last-editor-state] editor-state-context) + editor-state-changed? (and editor-state-context + (not= editor-state-context + (get-in db [:chats chat-id :last-editor-state]))) + _ (when (and editor-state-changed? + (= :prompt-message (:type decision))) + (swap! db* assoc-in [:chats chat-id :last-editor-state] editor-state-context)) + editor-state-contents (when editor-state-changed? [{:type :text :text editor-state-context}]) user-messages [{:role "user" :content (vec (concat [{:type :text :text message}] expanded-prompt-contexts @@ -1492,8 +1528,7 @@ :full-model full-model :provider provider :model model - :messenger messenger}) - decision (message->decision message db config)] + :messenger messenger})] ;; Show original prompt to user, but LLM receives the modified version (lifecycle/send-content! chat-ctx :user {:type :text :content-id (:user-content-id chat-ctx) diff --git a/src/eca/features/commands.clj b/src/eca/features/commands.clj index a66d28e29..b2943ab73 100644 --- a/src/eca/features/commands.clj +++ b/src/eca/features/commands.clj @@ -512,6 +512,45 @@ (multi-str "Context Usage" "" header "" "Estimated usage" "" zipped)) (multi-str "Context Usage" header "" (mapv #(fmt-cat "" %) all-rows))))) +(defn ^:private prompt-show-section [title body] + (when-not (string/blank? body) + (multi-str + "────────────────────────────────────────" + (str "# " title) + "────────────────────────────────────────" + (string/trim body)))) + +(defn ^:private prompt-show-user-messages-body [user-messages] + (->> (mapcat :content user-messages) + (map-indexed (fn [idx content] + (when-let [text (shared/not-blank + (cond + (some? (:text content)) (:text content) + (= :image (:type content)) (format "[image: media-type=%s, base64 omitted (%s chars)]" + (:media-type content) + (count (or (:base64 content) ""))) + :else (pr-str (dissoc content :base64))))] + (if (and (zero? idx) text) + (string/replace-first text #"^/prompt-show(?:\s+|$)" "") + text)))) + (remove string/blank?) + (string/join "\n\n"))) + +(defn ^:private prompt-show-text [instructions user-messages] + (let [{:keys [static dynamic]} (if (map? instructions) + instructions + {:static instructions :dynamic nil}) + system-prompt (->> [static dynamic] + (remove string/blank?) + (string/join "\n\n")) + sections (remove nil? + [(prompt-show-section "Instructions (System prompt)" system-prompt) + (prompt-show-section "Chat (User prompt)" (prompt-show-user-messages-body user-messages))])] + (multi-str + (string/join "\n\n" sections) + "" + "_Tool schemas are sent separately and are not included in this text dump._"))) + (defn handle-command! [command args {:keys [chat-id db* config messenger full-model agent all-tools instructions user-messages metrics] :as chat-ctx}] (let [db @db* custom-cmds (custom-commands config (:workspace-folders db)) @@ -782,21 +821,10 @@ msg (rules-msg config roots agent full-model all-tools)] {:type :chat-messages :chats {chat-id {:messages [{:role "system" :content [{:type :text :text msg}]}]}}}) - "prompt-show" (let [full-prompt (str "Instructions:\n" (f.prompt/instructions->str instructions) "\n" - "Prompt:\n" (reduce - (fn [s {:keys [content]}] - (str - s - (reduce - #(str %1 (string/replace-first (:text %2) "/prompt-show " "") "\n") - "" - content))) - "" - user-messages))] - {:type :chat-messages - :chats {chat-id {:messages [{:role "system" - :content [{:type :text - :text full-prompt}]}]}}}) + "prompt-show" {:type :chat-messages + :chats {chat-id {:messages [{:role "system" + :content [{:type :text + :text (prompt-show-text instructions user-messages)}]}]}}} "subagents" (let [msg (subagents-msg config)] {:type :chat-messages :chats {chat-id {:messages [{:role "system" :content [{:type :text :text msg}]}]}}}) diff --git a/src/eca/features/prompt.clj b/src/eca/features/prompt.clj index 0f0204f7f..29278977e 100644 --- a/src/eca/features/prompt.clj +++ b/src/eca/features/prompt.clj @@ -33,7 +33,13 @@ (or (get-in config [:agent agent-name :prompts key]) (get-in config [:prompts key]))) -(defn ^:private eca-chat-prompt [agent-name config chat-id db] +(defn ^:private absolute-path? [path] + (and (string? path) + (.isAbsolute (io/file path)))) + +(defn eca-chat-prompt + "Selected chat prompt template; single source of truth for rendering and cache identity." + [agent-name config chat-id db] (let [agent-config (get-in config [:agent agent-name]) is-subagent? (boolean (get-in db [:chats chat-id :subagent])) subagent-prompt (and is-subagent? @@ -52,7 +58,7 @@ config-prompt ;; agent with absolute path - (and legacy-config-prompt-file (string/starts-with? legacy-config-prompt-file "/")) + (absolute-path? legacy-config-prompt-file) (slurp legacy-config-prompt-file) ;; Built-in or resource path @@ -63,36 +69,66 @@ :else (load-builtin-prompt "code_agent.md")))) -(defn contexts-str [refined-contexts repo-map* startup-ctx] - (multi-str - "" - "" - (reduce - (fn [context-str {:keys [type path position content lines-range uri]}] - (str context-str (case type - :file (if lines-range - (format "%s\n\n" - (:start lines-range) - (:end lines-range) - path - content) - (format "%s\n\n" path content)) - :agents-file (multi-str - (format "" path) - content - "\n\n") - :repoMap (format "%s\n\n" @repo-map*) - :cursor (format "\n\n" - path - (str (:line (:start position)) ":" (:character (:start position))) - (str (:line (:end position)) ":" (:character (:end position)))) - :mcpResource (format "%s\n\n" uri content) - ""))) - "" - refined-contexts) - (when startup-ctx - (str "\n\n" startup-ctx "\n\n\n")) - "")) +(defn ^:private attr-value-str [value] + (let [value (str value)] + (cond + (not (string/includes? value "\"")) + (format "\"%s\"" value) + + (not (string/includes? value "'")) + (format "'%s'" value) + + :else + (format "\"%s\"" (string/replace value "\"" """))))) + +(defn ^:private attr-str [attrs] + (->> attrs + (keep (fn [[k v]] + (when (some? v) + (format " %s=%s" (name k) (attr-value-str v))))) + string/join)) + +(defn ^:private render-context [{:keys [type path position content lines-range uri]} repo-map*] + (case type + :file (if lines-range + (format "%s" + (attr-str {:line-start (:start lines-range) + :line-end (:end lines-range) + :path path}) + content) + (format "%s" (attr-str {:path path}) content)) + :agents-file (multi-str + (format "" + (attr-str {:description "Primary System Directives & Coding Standards." + :path path})) + content + "") + :repoMap (format "%s" + (attr-str {:description "Workspace structure tree; spaces represent file hierarchy."}) + @repo-map*) + :cursor (format "" + (attr-str {:description "Editor cursor position (line:character)." + :path path + :start (str (:line (:start position)) ":" (:character (:start position))) + :end (str (:line (:end position)) ":" (:character (:end position)))})) + :mcpResource (format "%s" (attr-str {:uri uri}) content) + nil)) + +(defn contexts-str + ([refined-contexts repo-map* startup-ctx] + (contexts-str refined-contexts repo-map* startup-ctx {})) + ([refined-contexts repo-map* startup-ctx {:keys [tag description] + :or {tag "contexts" + description "User-provided context. Treat as current and accurate."}}] + (let [body (cond-> (vec (keep #(render-context % repo-map*) refined-contexts)) + startup-ctx (conj (multi-str "" + startup-ctx + "")))] + (multi-str + (format "<%s%s>" tag (attr-str {:description description})) + (when (seq body) + (string/join "\n\n" body)) + (format "" tag))))) (defn ->base-selmer-ctx ([all-tools db] @@ -111,6 +147,26 @@ {} all-tools)))) +(defn ^:private workspace-roots-section [db] + (when-let [roots (seq (:workspace-folders db))] + (multi-str + "## Workspace Roots" + "" + (format "" (attr-str {:description "Workspace roots used for path resolution and tool scoping."})) + (->> roots + (map (fn [{:keys [uri]}] + (format "" + (attr-str {:path (shared/uri->filename uri)})))) + (string/join "\n")) + "" + ""))) + +(def ^:private path-resolution-section + (multi-str + "## Path Resolution" + "" + "Use workspace roots as the base for resolving relative paths. Tool paths must be absolute unless the tool says otherwise.")) + (defn ^:private mcp-instructions-section [db] (let [servers-with-instructions (->> (:mcp-clients db) @@ -122,20 +178,20 @@ seq)] (when servers-with-instructions (multi-str - "" + "## MCP Server Instructions" "" - (reduce - (fn [acc {:keys [name instructions]}] - (str acc (format "\n%s\n\n\n" name instructions))) - "" - servers-with-instructions) - "" - "")))) + (format "" (attr-str {:description "Instructions from running MCP servers."})) + (->> servers-with-instructions + (map (fn [{:keys [name instructions]}] + (multi-str + (format "" (attr-str {:name name})) + instructions + ""))) + (string/join "\n\n")) + "")))) (def ^:private editor-state-context-types - "Volatile editor-state contexts (e.g. cursor) delivered per-turn in the user - message instead of the system prompt, so a moving cursor doesn't invalidate - the cached system prefix." + "Volatile editor-state contexts (e.g. cursor) delivered per-turn in the user message." #{:cursor}) (def ^:private volatile-context-types @@ -143,27 +199,31 @@ static prompt." (into #{:mcpResource} editor-state-context-types)) +(def ^:private static-prompt-context-types + "Context types rendered into the cached static prompt block." + #{:file :agents-file :repoMap}) + +(defn static-prompt-context? + "True when ctx renders into the cached static prompt block." + [ctx] + (boolean (static-prompt-context-types (:type ctx)))) + (defn ^:private rule-section [tag description rendered-rules] (when (seq rendered-rules) - [(format "<%s description=\"%s\">" tag description) + [(format "<%s%s>" tag (attr-str {:description description})) (string/join "\n" rendered-rules) (format "" tag) ""])) (defn ^:private path-scoped-rule-attrs [{rule-name :name :keys [id paths enforce scope workspace-root]}] - (let [attr (fn [[k v]] - (when v - (str " " k "=\"" v "\"")))] - (->> [["id" id] - ["name" rule-name] - ["scope" (name scope)] - ["workspace-root" workspace-root] - ["paths" (string/join "," paths)] - ["enforce" (string/join "," (or enforce ["modify"]))]] - (keep attr) - string/join))) + (attr-str {:id id + :name rule-name + :scope (some-> scope name) + :workspace-root workspace-root + :paths (string/join "," paths) + :enforce (string/join "," (or enforce ["modify"]))})) (defn ^:private path-scoped-rule-catalog-entry [rule] @@ -175,7 +235,7 @@ project-rules (filter #(= :project (:scope %)) path-scoped-rules)] (multi-str (when (seq global-rules) - ["" + [(format "" (attr-str {:description "Path-scoped rules loaded outside the current workspace."})) (->> global-rules (map render-entry) (string/join "\n")) @@ -184,7 +244,7 @@ (group-by :workspace-root) (sort-by first) (map (fn [[workspace-root rules]] - (str "\n" + (str "\n" (->> rules (map render-entry) (string/join "\n")) @@ -195,18 +255,15 @@ (path-scoped-rule-sections path-scoped-rules path-scoped-rule-catalog-entry)) (defn build-static-instructions - "Builds the stable portion of the system prompt: agent prompt, rules, skills, - stable contexts, and additional system info. Volatile contexts and MCP server - instructions are handled by build-dynamic-instructions; editor-state contexts - (cursor) by build-editor-state-context." + "Builds the cacheable static system-prompt prefix." [refined-contexts static-rules path-scoped-rules skills repo-map* agent-name config chat-id all-tools db] (let [selmer-ctx (->base-selmer-ctx all-tools chat-id db) - stable-contexts (remove #(volatile-context-types (:type %)) refined-contexts) + stable-contexts (filter static-prompt-context? refined-contexts) rendered-static-rules (keep (fn [{:keys [name content scope]}] (when-let [rendered (shared/safe-selmer-render content selmer-ctx (str "rule:" name) nil)] (when-not (string/blank? rendered) {:scope (if (= :global scope) :global :project) - :content (format "%s" name rendered)}))) + :content (format "%s" (attr-str {:name name}) rendered)}))) static-rules) fetch-rule-available? (tools.util/tool-available? all-tools "eca__fetch_rule") global-rules (->> rendered-static-rules @@ -216,22 +273,24 @@ (remove #(= :global (:scope %))) (map :content)) path-scoped-section (when (and fetch-rule-available? (seq path-scoped-rules)) - ["" + [(format "" (attr-str {:description "Rules that apply to matching file paths. Use fetch_rule before actions required by enforce (read, modify, or both). Each rule only needs to be fetched once per chat."})) (path-scoped-rule-catalog path-scoped-rules) ""]) has-static-rules? (seq rendered-static-rules)] (multi-str - (shared/safe-selmer-render (eca-chat-prompt agent-name config chat-id db) - selmer-ctx "chat-prompt") + (string/trimr + (shared/safe-selmer-render (eca-chat-prompt agent-name config chat-id db) + selmer-ctx "chat-prompt")) + "" (when (or has-static-rules? path-scoped-section) ["## Rules" "" - "" + (format "" (attr-str {:description "Rules defined by user. Follow them as closely as possible."})) (rule-section "global-rules" - "Broader rules loaded outside the current workspace. Project rules below are more specific if guidance conflicts." + "Global user rules; project rules are more specific." global-rules) (rule-section "project-rules" - "Rules loaded from the current workspace. Prefer these when they conflict with broader global rules." + "Workspace rules; prefer over global rules when they conflict." project-rules) path-scoped-section "" @@ -239,66 +298,62 @@ (when (seq skills) ["## Skills" "" - "\n" + (str (format "" (attr-str {:description "Available skills; load with eca__skill when relevant."})) "\n") (reduce (fn [skills-str {:keys [name description]}] - (str skills-str (format "\n" name description))) + (str skills-str (format "\n" (attr-str {:name name :description description})))) "" skills) "" ""]) (shared/safe-selmer-render (load-builtin-prompt "additional_system_info.md") selmer-ctx "additional-system-info") - "" + (workspace-roots-section db) + path-resolution-section (let [startup-ctx (get-in db [:chats chat-id :startup-context])] (when (or (seq stable-contexts) (not (string/blank? startup-ctx))) - ["## Static Contexts" + ["" + "## Context" "" (contexts-str stable-contexts repo-map* startup-ctx)]))))) (defn build-dynamic-instructions - "Builds the volatile portion of the system prompt: MCP resource contexts and - MCP server instructions. Editor-state contexts (cursor) are excluded here - - they are delivered per-turn in the user message via build-editor-state-context. - Recomputed every turn. Returns nil when empty." + "Per-turn dynamic system instructions (MCP resources, server instructions). Returns nil when empty." [refined-contexts db] (let [volatile-contexts (filter #(and (volatile-context-types (:type %)) (not (editor-state-context-types (:type %)))) refined-contexts) result (multi-str (when (seq volatile-contexts) - ["## Dynamic Contexts" + ["## MCP Resources" "" - (contexts-str volatile-contexts nil nil)]) + (contexts-str volatile-contexts nil nil + {:description "Resources from connected MCP servers."}) + ""]) (mcp-instructions-section db))] (when-not (string/blank? result) result))) (defn build-editor-state-context - "Renders the volatile editor-state contexts (e.g. cursor) that are delivered - per-turn in the user message instead of the system prompt. Returns nil when - there are none." + "Renders editor-state contexts (e.g. cursor) for the user message. Returns nil when none." [refined-contexts] (let [editor-state-contexts (filter #(editor-state-context-types (:type %)) refined-contexts)] (when (seq editor-state-contexts) - (contexts-str editor-state-contexts nil nil)))) + (contexts-str editor-state-contexts nil nil + {:tag "editor-state" + :description "Editor state reference; not a user request. Use only when relevant."})))) (defn build-chat-instructions - "Returns {:static \"...\" :dynamic \"...\"}. - Static content (agent prompt, rules, skills, stable contexts) can be cached - when unchanged across turns. - Dynamic content (MCP resources, MCP instructions) is recomputed every turn. - Editor-state (cursor) is delivered separately in the user message." + "Returns {:static :dynamic} system instructions." [refined-contexts static-rules path-scoped-rules skills repo-map* agent-name config chat-id all-tools db] {:static (build-static-instructions refined-contexts static-rules path-scoped-rules skills repo-map* agent-name config chat-id all-tools db) :dynamic (build-dynamic-instructions refined-contexts db)}) (defn instructions->str - "Flattens a {:static :dynamic} instructions map into a single string. - Used by non-Anthropic providers that don't support split system prompt blocks." + "Flattens {:static :dynamic} into a single string for non-Anthropic providers." [{:keys [static dynamic]}] (if dynamic - (multi-str static dynamic) + (multi-str static "" dynamic) static)) (defn build-rewrite-instructions [text path full-text range all-tools config db] @@ -313,7 +368,7 @@ config-prompt ;; Absolute path - (and legacy-prompt-file (string/starts-with? legacy-prompt-file "/")) + (absolute-path? legacy-prompt-file) (slurp legacy-prompt-file) ;; Resource path @@ -382,7 +437,7 @@ config-prompt ;; Absolute path - (and legacy-config-file-prompt (string/starts-with? legacy-config-file-prompt "/")) + (absolute-path? legacy-config-file-prompt) (slurp legacy-config-file-prompt) ;; Resource path diff --git a/src/eca/oauth.clj b/src/eca/oauth.clj index e998bb3ef..764a09537 100644 --- a/src/eca/oauth.clj +++ b/src/eca/oauth.clj @@ -3,6 +3,7 @@ [cheshire.core :as json] [clojure.java.io :as io] [clojure.string :as string] + [eca.digest :as digest] [eca.logger :as logger] [selmer.parser :as selmer] [hato.client :as http] @@ -12,8 +13,7 @@ [ring.util.codec :as ring.util] [ring.util.response :as response]) (:import - [java.nio.charset StandardCharsets] - [java.security KeyStore MessageDigest SecureRandom] + [java.security KeyStore SecureRandom] [java.util Base64] [javax.net.ssl KeyManagerFactory SSLContext] [org.eclipse.jetty.server Server])) @@ -96,17 +96,13 @@ (defn ^:private ->base64url [base64-str] (-> base64-str (string/replace "+" "-") (string/replace "/" "_"))) -(defn ^:private str->sha256 [^String s] - (-> (MessageDigest/getInstance "SHA-256") - (.digest (.getBytes s StandardCharsets/UTF_8)))) - (defn ^:private random-verifier [] (->base64url (->base64 (rand-bytes 63)))) (defn generate-pkce [] (let [verifier (random-verifier)] {:verifier verifier - :challenge (-> verifier str->sha256 ->base64 ->base64url (string/replace "=" ""))})) + :challenge (-> verifier digest/sha-256-bytes ->base64 ->base64url (string/replace "=" ""))})) (defn ^:private oauth-handler [request on-success on-error] ;; The HTML response is built and returned synchronously, but on-success/on-error @@ -320,16 +316,16 @@ :redirect_uri redirect-uri :resource url} scope (assoc :scope scope)))] - (cond-> {:callback-port callback-port - :token-endpoint (or (:token_endpoint meta) - (str auth-server "/access_token")) - :verifier verifier - :client-id client-id - :redirect-uri redirect-uri - :resource url - :authorization-endpoint (str base-auth-endpoint "?" query-params)} - ssl? (assoc :ssl? true) - client-secret (assoc :client-secret client-secret))))))))) + (cond-> {:callback-port callback-port + :token-endpoint (or (:token_endpoint meta) + (str auth-server "/access_token")) + :verifier verifier + :client-id client-id + :redirect-uri redirect-uri + :resource url + :authorization-endpoint (str base-auth-endpoint "?" query-params)} + ssl? (assoc :ssl? true) + client-secret (assoc :client-secret client-secret))))))))) (comment (oauth-info "https://mcp.atlassian.com/v1/sse") diff --git a/test/eca/features/chat_test.clj b/test/eca/features/chat_test.clj index eead848d7..c2d87f00e 100644 --- a/test/eca/features/chat_test.clj +++ b/test/eca/features/chat_test.clj @@ -7,6 +7,8 @@ [eca.db :as db] [eca.features.chat :as f.chat] [eca.features.chat.lifecycle :as lifecycle] + [eca.features.context :as f.context] + [eca.features.index :as f.index] [eca.features.prompt :as f.prompt] [eca.features.tools :as f.tools] [eca.features.tools.mcp :as f.mcp] @@ -867,6 +869,60 @@ (is (string/includes? (turn-text 2) "> @user-msgs* first first :content (keep :text) (string/join "\n"))] + (is (string/includes? turn-text "> @user-msgs* first first :content (keep :text) (string/join "\n")) + "> @user-msgs* second first :content (keep :text) (string/join "\n"))] + (is (string/includes? turn-text "refined (fn [contexts _db] contexts) + f.prompt/build-chat-instructions + (fn [& args] + (swap! build-calls* inc) + (apply real-build args))] + (let [mocks {:all-tools-mock (constantly []) + :api-mock (fn [{:keys [on-message-received]}] + (on-message-received {:type :finish}))} + ctx-v1 {:type :file :path "foo.clj" :content "v1"} + ctx-v2 {:type :file :path "foo.clj" :content "v2"} + {:keys [chat-id]} (prompt! {:message "Hi" :contexts [ctx-v1]} mocks)] + (is (= 1 @build-calls*)) + (h/reset-messenger!) + (prompt! {:message "Again" :chat-id chat-id :contexts [ctx-v1]} mocks) + (is (= 1 @build-calls*) "Same stable context should reuse cached static instructions") + (h/reset-messenger!) + (prompt! {:message "Changed" :chat-id chat-id :contexts [ctx-v2]} mocks) + (is (= 2 @build-calls*) "Changed stable context should rebuild static instructions")))))) + +(deftest prompt-cache-repo-map-content-intentionally-does-not-change-test + (testing "Changing only repoMap content does not rebuild static instructions because repoMap is an expensive cached snapshot" + (h/reset-components!) + (let [build-calls* (atom 0) + repo-map-calls* (atom 0) + real-build f.prompt/build-chat-instructions] + (with-redefs [f.context/agents-file-contexts (constantly []) + f.context/raw-contexts->refined (fn [contexts _db] contexts) + f.index/repo-map (fn [& _] + (str "TREE-" (swap! repo-map-calls* inc))) + f.prompt/build-chat-instructions + (fn [& args] + (swap! build-calls* inc) + (apply real-build args))] + (let [mocks {:all-tools-mock (constantly []) + :api-mock (fn [{:keys [on-message-received]}] + (on-message-received {:type :finish}))} + ctx {:type :repoMap} + {:keys [chat-id]} (prompt! {:message "Hi" :contexts [ctx]} mocks)] + (is (= 1 @build-calls*)) + (is (= 1 @repo-map-calls*) "Initial cache miss renders the repoMap snapshot") + (h/reset-messenger!) + (prompt! {:message "Again" :chat-id chat-id :contexts [ctx]} mocks) + (is (= 1 @build-calls*) "repoMap presence, not repoMap content, is part of cache identity") + (is (= 1 @repo-map-calls*) "Cache hit must not rebuild the expensive repoMap")))))) + +(deftest prompt-cache-absolute-prompt-file-change-test + (testing "Static prompt cache is rebuilt when an absolute systemPromptFile changes" + (h/reset-components!) + (let [build-calls* (atom 0) + real-build f.prompt/build-chat-instructions + prompt-file (fs/create-temp-file {:prefix "eca-prompt" :suffix ".md"})] + (try + (spit (str prompt-file) "Prompt v1") + (h/config! {:prompts {:chat nil} + :agent {"code" {:prompts {:chat nil} + :systemPromptFile (str prompt-file)}}}) + (with-redefs [f.prompt/build-chat-instructions + (fn [& args] + (swap! build-calls* inc) + (apply real-build args))] + (let [mocks {:all-tools-mock (constantly []) + :api-mock (fn [{:keys [on-message-received]}] + (on-message-received {:type :finish}))} + {:keys [chat-id]} (prompt! {:message "Hi" :agent "code"} mocks)] + (is (= 1 @build-calls*)) + (h/reset-messenger!) + (prompt! {:message "Again" :chat-id chat-id :agent "code"} mocks) + (is (= 1 @build-calls*) "Unchanged prompt file should reuse cached static instructions") + (spit (str prompt-file) "Prompt v2") + (h/reset-messenger!) + (prompt! {:message "Changed" :chat-id chat-id :agent "code"} mocks) + (is (= 2 @build-calls*) "Changed prompt file content should rebuild static instructions"))) + (finally + (fs/delete-if-exists prompt-file)))))) + (deftest prompt-cache-key-includes-agent-test (testing "sync-or-async-prompt! receives prompt-cache-key scoped by active agent" (h/reset-components!) @@ -1644,10 +1781,11 @@ :agent "plan" :model "openai/gpt-4.1"}) (is (= 3 @build-calls*) "Changing model should rebuild instructions") - (is (= {:static "static-3" - :agent "plan" - :model "openai/gpt-4.1"} - (get-in (h/db) [:chats chat-id :prompt-cache])))))) + (is (match? {:static "static-3" + :static-signature (m/pred #(re-matches #"[0-9a-f]{64}" %)) + :agent "plan" + :model "openai/gpt-4.1"} + (get-in (h/db) [:chats chat-id :prompt-cache])))))) (deftest message-content->chat-content-image-test (testing "image_generation_call role replays as a single :image ChatContent under assistant" diff --git a/test/eca/features/commands_test.clj b/test/eca/features/commands_test.clj index 1f7b92cfe..0b9699e36 100644 --- a/test/eca/features/commands_test.clj +++ b/test/eca/features/commands_test.clj @@ -10,6 +10,19 @@ (h/reset-components-before-test) +(deftest prompt-show-text-test + (testing "uses compact section headings and omits the /prompt-show command itself" + (let [result (#'f.commands/prompt-show-text + {:static "system prompt"} + [{:role "user" + :content [{:type :text :text "/prompt-show"} + {:type :text :text ""}]}])] + (is (string/includes? result "────────────────────────────────────────\n# Instructions (System prompt)\n────────────────────────────────────────\nsystem prompt")) + (is (string/includes? result "system prompt\n\n────────────────────────────────────────\n# Chat (User prompt)")) + (is (string/includes? result "────────────────────────────────────────\n# Chat (User prompt)\n────────────────────────────────────────\n")) + (is (string/includes? result "_Tool schemas are sent separately and are not included in this text dump._")) + (is (not (string/includes? result "/prompt-show")))))) + (deftest get-custom-command-tests (testing "returns nil when command not found" (is (nil? (#'f.commands/get-custom-command "nope" [] [])))) diff --git a/test/eca/features/prompt_test.clj b/test/eca/features/prompt_test.clj index f118a2a29..f4cf3a2a9 100644 --- a/test/eca/features/prompt_test.clj +++ b/test/eca/features/prompt_test.clj @@ -7,7 +7,26 @@ (defn ^:private build-instructions [refined-contexts static-rules path-scoped-rules skills repo-map* agent-name config chat-id all-tools db] - (prompt/build-chat-instructions refined-contexts static-rules path-scoped-rules skills repo-map* agent-name config chat-id all-tools db)) + (let [db (cond-> db + (empty? (:workspace-folders db)) + (assoc :workspace-folders [{:uri "file:///workspace"}]))] + (prompt/build-chat-instructions refined-contexts static-rules path-scoped-rules skills repo-map* agent-name config chat-id all-tools db))) + +(deftest contexts-attribute-formatting-test + (testing "keeps copy-sensitive attribute values raw while preserving quote delimiters" + (let [result (prompt/contexts-str [{:type :file + :path "/tmp/a&b.clj" + :content "content"} + {:type :file + :path "/tmp/has\"quote.clj" + :content "content"} + {:type :file + :path "/tmp/has\"both'quotes.clj" + :content "content"}] + nil nil)] + (is (string/includes? result "path=\"/tmp/a&b.clj\"")) + (is (string/includes? result "path='/tmp/has\"quote.clj'")) + (is (string/includes? result "path=\"/tmp/has"both'quotes.clj\""))))) (deftest build-instructions-test (testing "Should return a map with :static and :dynamic keys" @@ -26,6 +45,7 @@ path-scoped-rules [{:id "/workspace/a/.eca/rules/format.md" :name "format.md" :scope :project :workspace-root "/workspace/a" :paths ["**/*.clj"]} {:id "/home/user/.config/eca/rules/no-network.md" :name "no-network.md" :scope :global :paths ["src/**/*.clj"]}] skills [{:name "review-pr" :description "Review a PR"} + {:name "grill-me" :description "Use when user mentions \"grill me\"."} {:name "lint-fix" :description "Fix a lint"}] fake-repo-map (delay "TREE") agent-name "code" @@ -33,8 +53,8 @@ {:keys [static dynamic]} (build-instructions refined-contexts static-rules path-scoped-rules skills fake-repo-map agent-name config nil [{:full-name "eca__fetch_rule"}] (h/db))] (is (string/includes? static "You are ECA")) (is (string/includes? static "")) - (is (string/includes? static "")) - (is (string/includes? static "")) + (is (string/includes? static "")) + (is (string/includes? static "")) (is (string/includes? static "First rule")) (is (string/includes? static "Second rule")) (is (string/includes? static "")) @@ -43,12 +63,18 @@ (is (string/includes? static "")) (is (string/includes? static "")) (is (string/includes? static "")) + (is (string/includes? static "")) (is (string/includes? static "")) - (is (string/includes? static "")) + (is (string/includes? static "")) (is (string/includes? static "(ns foo)")) - (is (string/includes? static "(def a 1)")) - (is (string/includes? static "TREE")) + (is (string/includes? static "(def a 1)")) + (is (string/includes? static "TREE")) (is (string/includes? static "")) + (is (string/includes? static "## Workspace Roots")) + (is (string/includes? static "")) + (is (string/includes? static "")) (is (string/includes? static "")) (is (string/includes? static "")) - (is (string/includes? static "")) + (is (string/includes? static "")) (is (string/includes? static "(ns foo)")) - (is (string/includes? static "(def a 1)")) - (is (string/includes? static "TREE")) + (is (string/includes? static "(def a 1)")) + (is (string/includes? static "TREE")) (is (string/includes? static "")) + (is (string/includes? static "## Workspace Roots")) + (is (string/includes? static "")) + (is (string/includes? static "")) - (is (string/includes? static "\ninjected by chatStart\n")))) + (is (string/includes? static "")) + (is (string/includes? static "\ninjected by chatStart\n")))) (testing "no Contexts section when there are neither stable contexts nor startup-context" (let [{:keys [static]} (build-instructions [] [] [] [] (delay "TREE") "code" {} "chat-1" [] (h/db))] @@ -93,7 +132,7 @@ (testing "omits empty global rules section when only project-scoped rules render" (let [static-rules [{:name "rule1" :content "Only project rule" :scope :project}] {:keys [static]} (build-instructions [] static-rules [] [] (delay "TREE") "code" {} nil [] (h/db))] - (is (string/includes? static "")) + (is (string/includes? static "")) (is (not (string/includes? static "")) (is (string/includes? result "str-test - (testing "flattens map with both parts to joined string" - (is (= "static\ndynamic" (prompt/instructions->str {:static "static" :dynamic "dynamic"})))) + (testing "flattens map with both parts to joined string separated by a blank line" + (is (= "static\n\ndynamic" (prompt/instructions->str {:static "static" :dynamic "dynamic"})))) (testing "flattens map with nil dynamic to just static" (is (= "static" (prompt/instructions->str {:static "static" :dynamic nil}))))) (deftest build-instructions-context-sections-ordering-test - (testing "Static contexts are labeled and rendered after Environment Context" + (testing "Context section is labeled and rendered after Environment, workspace, and path sections" (let [refined-contexts [{:type :file :path "foo.clj" :content "(ns foo)"}] {:keys [static]} (build-instructions refined-contexts [] [] [] (delay "TREE") "code" {} nil [] (h/db))] - (is (string/includes? static "## Static Contexts")) + (is (string/includes? static "## Context")) (is (string/includes? static "## Environment Context")) + (is (string/includes? static "## Workspace Roots")) + (is (string/includes? static "## Path Resolution")) (is (< (string/index-of static "## Environment Context") - (string/index-of static "## Static Contexts"))))) - (testing "Static contexts come before Dynamic contexts in the flattened prompt" + (string/index-of static "## Workspace Roots") + (string/index-of static "## Path Resolution") + (string/index-of static "\n## Context"))))) + (testing "Static context comes before MCP resources in the flattened prompt" (let [refined-contexts [{:type :file :path "foo.clj" :content "(ns foo)"} {:type :mcpResource :uri "custom://my-resource" :content "volatile-content"}] flat (prompt/instructions->str (build-instructions refined-contexts [] [] [] (delay "TREE") "code" {} nil [] (h/db)))] - (is (< (string/index-of flat "## Static Contexts") - (string/index-of flat "## Dynamic Contexts")))))) + (is (< (string/index-of flat "\n## Context") + (string/index-of flat "## MCP Resources")))))) + +(deftest instructions->str-no-double-blank-line-test + (testing "static-to-dynamic boundary has exactly one blank line, not two, when no Context section" + (let [refined-contexts [{:type :mcpResource :uri "custom://my-resource" :content "volatile-content"}] + flat (prompt/instructions->str + (build-instructions refined-contexts [] [] [] (delay "TREE") "code" {} nil [] (h/db)))] + (is (string/includes? flat "## Path Resolution")) + (is (string/includes? flat "## MCP Resources")) + (is (not (string/includes? flat "\n\n\n## MCP Resources")) + "there should be exactly one blank line before ## MCP Resources, not two")))) (deftest build-instructions-rule-condition-test (testing "renders rule content using Selmer condition variables"