diff --git a/CHANGELOG.md b/CHANGELOG.md index 5edcd97..3ea7fa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,64 @@ All notable changes to this project will be documented in this file. This change ## [Unreleased] +### Added (Client Mode Empty — upstream PR #1428) +- **`:mode` client option** — `#{:copilot-cli :empty}`, default + `:copilot-cli`. Selects between historical CLI behavior and a hardened + multitenancy posture for SaaS hosts that must isolate sessions from + the local machine. Validated on `copilot/client`. +- **`:empty` mode constructor enforcement** — In `:empty` mode the + client requires at least one tenant-scoped storage root + (`:copilot-home`, `:session-fs`, `:cli-url`, or `:is-child-process?`) + so the CLI never falls back to the user's home directory, and forces + `COPILOT_DISABLE_KEYTAR=1` on the spawned CLI so the headless server + never touches the host keychain. +- **Required `:available-tools` in `:empty` mode** — + `create-session` / `resume-session` (sync and async) now reject + empty-mode sessions that don't supply a tool allow-list. An empty + vector `[]` is legitimate (it means "no tools") — the key just has + to be present so silently-empty filters can't happen. +- **9 mode-default session config fields** — In `:empty` mode the SDK + spreads safe defaults UNDER the caller's session config (caller + always wins): `:enable-session-telemetry? false`, + `:mcp-oauth-token-storage :in-memory`, `:skip-embedding-retrieval true`, + `:embedding-cache-storage :in-memory`, + `:enable-on-demand-instruction-discovery false`, + `:enable-file-hooks false`, `:enable-host-git-operations false`, + `:enable-session-store false`, `:enable-skills false`. +- **`session.options.update` plumbing** — After a successful + `session.create` / `session.resume`, the SDK now issues a follow-up + `session.options.update` RPC carrying the four overridable feature + flags (and, in `:empty` mode, `installedPlugins: []`): + - `:skip-custom-instructions` (default `true` in `:empty`) + - `:custom-agents-local-only` (default `true` in `:empty`) + - `:coauthor-enabled` (default `false` in `:empty`) + - `:manage-schedule-enabled` (default `false` in `:empty`) + In `:copilot-cli` mode only flags the caller explicitly set are + forwarded; if the patch ends up empty the RPC is skipped. On failure + the SDK disconnects and removes the half-configured session before + rethrowing. Wired into all four entry points (`create-session`, + `resume-session`, `:", +;; source is one of "builtin", "mcp", or "custom": +(tool-set/builtin "ask_user") ; => "builtin:ask_user" +(tool-set/builtin "*") ; => "builtin:*" (all built-ins) +(tool-set/mcp "*") ; => "mcp:*" (all MCP tools) +(tool-set/custom "my_tool") ; => "custom:my_tool" + +;; Vector of patterns: +(tool-set/builtins ["task" "skill"]) +;; => ["builtin:task" "builtin:skill"] + +;; The "Isolated" preset matches BuiltInTools.Isolated upstream — +;; every built-in that is safely session-bounded (no host I/O): +tool-set/isolated +;; => ["builtin:ask_user" "builtin:task_complete" "builtin:exit_plan_mode" ...] +``` + +The constructors enforce well-formed entries: a bare `"*"` is rejected (the SDK +also rejects it in `:available-tools` / `:excluded-tools` at the session +boundary — apps must explicitly opt into a source so an absent source can +never silently grant access to unexpected tools). The runtime always receives +`:tool-filter-precedence "excluded"` on `session.create` / `session.resume` +so the ordering between allow and deny lists is deterministic. + ### Tools Let the CLI call back into your process when the model needs capabilities you provide: diff --git a/examples/README.md b/examples/README.md index f8f4d01..65bc92e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -85,6 +85,9 @@ clojure -A:examples -X lifecycle-hooks/run # Reasoning effort clojure -A:examples -X reasoning-effort/run + +# Empty (multitenancy) mode +clojure -A:examples -X empty-mode/run ``` Or run all examples: @@ -93,7 +96,7 @@ Or run all examples: ``` > **Note:** `run-all-examples.sh` runs 16 examples that need only the Copilot CLI (examples 1–9, 12–16, 18, and 19). -> Examples 10 (BYOK) and 11 (MCP) require external dependencies (API keys, Node.js), and example 17 (ask-user-failure) is excluded for reliability. Run these manually. +> Examples 10 (BYOK), 11 (MCP), and 20 (empty-mode — uses BYOK) require external dependencies (API keys, Node.js), and example 17 (ask-user-failure) is excluded for reliability. Run these manually. With a custom CLI path: ```bash @@ -879,6 +882,39 @@ Commands are registered by passing them in the session config: --- +## Example 20: Empty (Multitenancy) Mode (`empty_mode.clj`) + +**Description**: Run a session under `:mode :empty` — the hardened +posture for SaaS hosts that run sessions on behalf of multiple users. + +**Features**: `:mode :empty`, `:copilot-home`, `:session-fs` with an +in-memory provider, `tool-set/isolated`, BYOK provider. + +Demonstrates the multitenancy hardening introduced in upstream PR #1428. +The example creates fresh temp directories for `:copilot-home`, the +session's `cwd`, and the session state path, supplies an in-memory +`:session-fs` provider, and runs a single query through a BYOK provider +(empty mode disables the local keychain, so the host must bring its own +auth). In `:empty` mode the SDK forces `COPILOT_DISABLE_KEYTAR=1` on the +spawned CLI, spreads safe session defaults (telemetry off, embeddings +in-memory, host-git off, skills off, ...), strips `environment_context` +from the system message, and sends a follow-up `session.options.update` +RPC turning off coauthor / manage-schedule and forcing +`installedPlugins []`. + +**Prerequisites**: Set `OPENAI_API_KEY` or `ANTHROPIC_API_KEY`. Excluded +from `run-all-examples.sh` because it requires an external API key. + +```bash +OPENAI_API_KEY=sk-... clojure -A:examples -X empty-mode/run +OPENAI_API_KEY=sk-... clojure -A:examples -X empty-mode/run :prompt '"What is Clojure?"' +``` + +See [`doc/reference/API.md`](../doc/reference/API.md#client-mode-empty) +for the full Client Mode reference. + +--- + ## Clojure vs JavaScript Comparison Here's how common patterns compare between the Clojure and JavaScript SDKs: diff --git a/examples/empty_mode.clj b/examples/empty_mode.clj new file mode 100644 index 0000000..c07b58a --- /dev/null +++ b/examples/empty_mode.clj @@ -0,0 +1,133 @@ +(ns empty-mode + "Demonstrates `:mode :empty` — the multitenancy posture for SaaS hosts + that run sessions on behalf of multiple users and must keep each + session isolated from the local machine. + + Compared to the default `:copilot-cli` mode, `:empty` mode: + + - Requires a tenant-scoped storage root (here we use a temp dir as + `:copilot-home` plus an in-memory `:session-fs`). + - Forces `COPILOT_DISABLE_KEYTAR=1` on the spawned CLI so the host + keychain is never touched. + - Spreads 9 safe defaults under each session's config (telemetry off, + host-git operations off, embeddings in-memory, skills off, ...). + - Strips the `environment_context` section from the system message. + - Sends a follow-up `session.options.update` RPC turning off + coauthor / manage-schedule and forcing `installedPlugins []`. + - Requires `:available-tools` on every session (use `[]` to opt into + no tools, or `tool-set/isolated` for the safely session-bounded + built-in preset). + + The session itself needs auth — since the local keychain is disabled, + this example uses a BYOK provider (OpenAI or Anthropic). Set ONE of: + + OPENAI_API_KEY — for OpenAI + ANTHROPIC_API_KEY — for Anthropic + + before running. The example is intentionally NOT in + `run-all-examples.sh` for this reason." + (:require [clojure.java.io :as io] + [github.copilot-sdk :as copilot] + [github.copilot-sdk.helpers :as h] + [github.copilot-sdk.tool-set :as tool-set])) + +(def defaults + {:prompt "What is 2+2? Answer in one sentence."}) + +(defn- temp-dir + "Create a fresh temp directory under java.io.tmpdir." + [prefix] + (let [path (java.nio.file.Files/createTempDirectory + prefix + (into-array java.nio.file.attribute.FileAttribute []))] + (.toString path))) + +(defn- in-memory-fs-provider + "Build a minimal in-memory filesystem provider for a single session. + The runtime routes session-scoped file I/O (event logs, large outputs, + checkpoints) through these callbacks." + [] + (let [store (atom {})] + {:read-file (fn [path] + (or (get @store path) + (throw (ex-info "missing file" {:code "ENOENT"})))) + :write-file (fn [path content _mode] + (swap! store assoc path content)) + :append-file (fn [path content _mode] + (swap! store update path (fnil str "") content)) + :exists (fn [path] (contains? @store path)) + :stat (fn [path] + {:is-file true + :is-directory false + :size (count (get @store path "")) + :mtime "2026-01-01T00:00:00Z" + :birthtime "2026-01-01T00:00:00Z"}) + :mkdir (fn [_path _recursive _mode] nil) + :readdir (fn [_path] []) + :readdir-with-types (fn [_path] []) + :rm (fn [path _recursive _force] (swap! store dissoc path)) + :rename (fn [src dest] + (swap! store (fn [s] (-> s + (assoc dest (get s src "")) + (dissoc src)))))})) + +(defn- byok-config + "Pick a BYOK provider config based on which API key is set." + [] + (let [openai (System/getenv "OPENAI_API_KEY") + anthropic (System/getenv "ANTHROPIC_API_KEY")] + (cond + openai {:model "gpt-5.4" + :provider {:provider-type :openai + :base-url "https://api.openai.com/v1" + :api-key openai}} + anthropic {:model "claude-sonnet-4" + :provider {:provider-type :anthropic + :base-url "https://api.anthropic.com" + :api-key anthropic}} + :else + (throw (ex-info + (str "Empty mode disables the local keychain — set " + "OPENAI_API_KEY or ANTHROPIC_API_KEY before running.") + {}))))) + +(defn run + "Issue a single query against a session created in `:empty` mode. + + Usage: + OPENAI_API_KEY=sk-xxx clojure -A:examples -X empty-mode/run + ANTHROPIC_API_KEY=sk-xxx clojure -A:examples -X empty-mode/run :prompt '\"What is Clojure?\"'" + [{:keys [prompt] :or {prompt (:prompt defaults)}}] + (let [{:keys [model provider]} (byok-config) + copilot-home (temp-dir "copilot-home-") + cwd (temp-dir "tenant-cwd-") + state (temp-dir "tenant-state-")] + (println "Tenant storage:") + (println " copilot-home:" copilot-home) + (println " cwd: " cwd) + (println " state: " state) + (println "Provider: " (:provider-type provider)) + (println "Model: " model) + (println) + (try + (copilot/with-client + [client {:mode :empty + :copilot-home copilot-home + :session-fs {:initial-cwd cwd + :session-state-path state + :conventions "posix"}}] + (copilot/with-session + [session client + {:on-permission-request copilot/approve-all + :model model + :provider provider + ;; Required in :empty mode. Use tool-set/isolated for the + ;; safely session-bounded built-in preset, or [] for none. + :available-tools tool-set/isolated + :create-session-fs-handler + (fn [_session] (in-memory-fs-provider))}] + (println "Q:" prompt) + (println "🤖:" (h/query prompt :session session)))) + (finally + (doseq [d [state cwd copilot-home]] + (try (.delete (io/file d)) (catch Exception _))))))) diff --git a/script/validate_docs.clj b/script/validate_docs.clj index f3fa1ec..daff6a3 100644 --- a/script/validate_docs.clj +++ b/script/validate_docs.clj @@ -42,6 +42,7 @@ "github.copilot-sdk.specs" "github.copilot-sdk.instrument" "github.copilot-sdk.tools" + "github.copilot-sdk.tool-set" "github.copilot-sdk.generated.event-specs" "github.copilot-sdk.generated.coerce"}) diff --git a/src/github/copilot_sdk/client.clj b/src/github/copilot_sdk/client.clj index 2fac01b..992cd79 100644 --- a/src/github/copilot_sdk/client.clj +++ b/src/github/copilot_sdk/client.clj @@ -144,7 +144,17 @@ - :remote? - **Experimental**. Enable remote session support (Mission Control). When true, the SDK appends `--remote` to the spawned CLI; sessions in a GitHub repository working directory become accessible from GitHub web and mobile. Ignored when - `:cli-url` is set. (upstream PR #1192)" + `:cli-url` is set. (upstream PR #1192) + - :mode - Client mode (upstream PR #1428). One of: + - `:copilot-cli` (default) — preserves historical CLI behavior. + - `:empty` — multitenancy hardening for hosts running sessions on behalf of + multiple users. Empty mode REQUIRES one of `:copilot-home`, `:session-fs`, + `:cli-url`, or `:is-child-process?` so the runtime has a tenant-scoped + storage root. It also forces `COPILOT_DISABLE_KEYTAR=1` on the spawned + CLI, spreads safe defaults under each session's config (telemetry off, + host-git operations disabled, ...), strips the `:environment-context` + section from system messages unless the caller already controls it, + and requires every session to specify `:available-tools`." ([] (client {})) ([opts] @@ -171,6 +181,19 @@ (when (and (some? (:tcp-connection-token opts)) (= true (:use-stdio? opts))) (throw (ex-info "tcp-connection-token cannot be used with use-stdio? true (stdio is pre-authenticated by transport)" {:tcp-connection-token "***" :use-stdio? true}))) + ;; Empty mode requires a tenant-scoped storage root (upstream PR #1428). + (when (and (= :empty (:mode opts)) + (not (or (:copilot-home opts) + (:session-fs opts) + (:cli-url opts) + (:is-child-process? opts)))) + (throw (ex-info + (str "Mode :empty requires one of :copilot-home, :session-fs, " + ":cli-url, or :is-child-process? so the spawned CLI has a " + "tenant-scoped storage root.") + {:mode :empty + :provided-storage-keys (select-keys opts [:copilot-home :session-fs + :cli-url :is-child-process?])}))) (when-not (s/valid? ::specs/client-options opts) (let [unknown (specs/unknown-keys opts specs/client-options-keys) explain (s/explain-data ::specs/client-options opts) @@ -1369,6 +1392,44 @@ (throw (ex-info "Invalid session config: :model is required when :provider (BYOK) is specified" {:config config})))) +(defn- validate-tool-filter-list! + "Reject bare `\"*\"` in a tool filter list (mirrors upstream + `validateToolFilterList`). The runtime treats a bare wildcard as a + literal name match for a tool whose name is the single character `*`, + which never matches anything — so a silent empty filter is worse than + an explicit error pointing the developer at the source-qualified + `tool-set/builtin`/`mcp`/`custom` helpers." + [field tools] + (when tools + (doseq [entry tools] + (when (= "*" entry) + (throw (ex-info + (format "Invalid %s entry '*': there is no bare wildcard. Use a source-qualified pattern like \"builtin:*\", \"mcp:*\", or \"custom:*\" (see github.copilot-sdk.tool-set)." + field) + {:field field :entry entry :tools tools})))))) + +(defn- validate-tool-filters! + "Run [[validate-tool-filter-list!]] on both `:available-tools` and + `:excluded-tools`." + [config] + (validate-tool-filter-list! "availableTools" (:available-tools config)) + (validate-tool-filter-list! "excludedTools" (:excluded-tools config))) + +(defn- validate-empty-mode-session-requirements! + "Empty mode requires every session to opt into its tools explicitly. The + caller may pass `:available-tools []` to mean \"no tools\", but the key + must be PRESENT — undefined is rejected so silently-empty filters can't + happen (upstream PR #1428, mirrors `resolveToolFilterOptions`)." + [client config] + (when (and (= :empty (:mode (options client))) + (not (contains? config :available-tools))) + (throw (ex-info + (str "Mode :empty requires every session to specify :available-tools. " + "Pass an explicit list of source-qualified patterns (e.g. " + "tool-set/isolated, or [\"builtin:*\"]) — or [] to opt into no tools.") + {:mode :empty + :session-config-keys (vec (keys config))})))) + ;; Section keyword → wire string mapping for system message customize mode. ;; Delegates to shared mapping in util.clj. @@ -1420,6 +1481,43 @@ ;; default: append mode {:mode "append" :content (:content sm)})) +(defn- apply-empty-mode-system-message + "In :empty mode, normalize the session config's :system-message so the + `environment_context` section is always stripped (CWD/OS/git-root leak) + unless the app has already taken control of it. No-op in :copilot-cli + mode. Mirrors upstream `getSystemMessageConfigForMode` (PR #1428). + + - No caller :system-message → emit `:customize` with + `:environment-context {:action :remove}`. + - Caller `:replace` → pass through (app fully replaces system message). + - Caller `:customize` → add `:environment-context {:action :remove}` to + :sections unless the app already specified an env-context override. + - Caller `:append` (or `:mode` absent) → promote to `:customize`, + preserve `:content` (still appended as additional instructions), + and strip `environment_context`." + [client config] + (if (not= :empty (:mode (options client))) + config + (let [sm (:system-message config) + ec-remove {:environment-context {:action :remove}}] + (cond + (nil? sm) + (assoc config :system-message {:mode :customize :sections ec-remove}) + + (= :replace (:mode sm)) + config + + (= :customize (:mode sm)) + (if (contains? (:sections sm) :environment-context) + config + (update-in config [:system-message :sections] + (fnil assoc {}) :environment-context {:action :remove})) + + :else + (assoc config :system-message + (cond-> {:mode :customize :sections ec-remove} + (:content sm) (assoc :content (:content sm)))))))) + (defn- provider->wire "Convert a Clojure ProviderConfig map to its JSON-RPC wire shape. @@ -1457,6 +1555,126 @@ (cond-> (dissoc lo :output-directory) dir (assoc :output-dir dir)))) +(defn- config-defaults-for-mode + "Mode-specific session config defaults spread UNDER the caller's config + (caller's values always win). Mirrors upstream `configDefaultsForMode` + in `client.ts` (upstream PR #1428). + + In `:empty` mode this returns the 9 safe defaults that enable + multitenancy hardening (no telemetry, in-memory token + embedding + storage, no host filesystem / git / skills). In `:copilot-cli` + mode this returns `{}` so the historical behavior is preserved." + [mode] + (if (= mode :empty) + {:enable-session-telemetry? false + :mcp-oauth-token-storage :in-memory + :skip-embedding-retrieval true + :embedding-cache-storage :in-memory + :enable-on-demand-instruction-discovery false + :enable-file-hooks false + :enable-host-git-operations false + :enable-session-store false + :enable-skills false} + {})) + +(defn- normalize-config-for-mode + "Apply mode-specific normalizations to session config (upstream PR #1428): + 1. Spread `config-defaults-for-mode` UNDER caller config (caller wins). + 2. In `:empty` mode, normalize `:system-message` so the + `environment_context` section is stripped unless the app has taken + control of it (see `apply-empty-mode-system-message`). + No-op in `:copilot-cli` mode beyond returning the caller config as-is." + [client config] + (->> config + (merge (config-defaults-for-mode (-> client options :mode))) + (apply-empty-mode-system-message client))) + +(defn- build-session-options-update-patch + "Compute the params for session.options.update (upstream PR #1428). + + In `:empty` mode, defaults the 4 overridable feature flags to safe values + (caller wins) and forces `installedPlugins: []` — apps that need custom + plugins must switch modes. In `:copilot-cli` mode, only forwards the 4 + flags that the caller explicitly set. + + Returns nil if the resulting patch is empty (no RPC needed)." + [client config] + (let [mode (-> client options :mode) + with-flag (fn [patch wire-key kw default-empty] + (cond + (contains? config kw) (assoc patch wire-key (get config kw)) + (= mode :empty) (assoc patch wire-key default-empty) + :else patch)) + patch (-> {} + (with-flag :skip-custom-instructions :skip-custom-instructions true) + (with-flag :custom-agents-local-only :custom-agents-local-only true) + (with-flag :coauthor-enabled :coauthor-enabled false) + (with-flag :manage-schedule-enabled :manage-schedule-enabled false)) + patch (cond-> patch + (= mode :empty) (assoc :installed-plugins []))] + (when (seq patch) patch))) + +(defn- cleanup-failed-options-update! + "Best-effort cleanup after a failed session.options.update. Disconnects + the runtime session (`session.destroy` RPC) and removes it from the + in-memory registry so the SDK doesn't leak a half-configured session." + [client session-id] + (try (session/disconnect! client session-id) + (catch Throwable t (log/debug "disconnect! during options.update cleanup failed: " (.getMessage t)))) + (try (session/remove-session! client session-id) + (catch Throwable t (log/debug "remove-session! during options.update cleanup failed: " (.getMessage t))))) + +(defn- apply-session-options-update! + "Sync: issue session.options.update with the mode-derived patch. No-op + when the patch is empty. On RPC failure, cleans up the half-configured + session and rethrows." + [client session config] + (when-let [patch (build-session-options-update-patch client config)] + (let [session-id (:session-id session) + {:keys [connection-io]} @(:state client) + params (assoc patch :session-id session-id)] + (try + (proto/send-request! connection-io "session.options.update" params 30000) + (catch Throwable t + (log/warn "session.options.update failed; cleaning up session " session-id) + (cleanup-failed-options-update! client session-id) + (throw t)))))) + +(defn- ! out err)) + (>! out :ok)) + (close! out)))) + (do (async/put! out :ok) + (close! out))) + out)) + (defn- build-create-session-params "Build wire params for session.create from config." [config] @@ -1502,6 +1720,12 @@ wire-sys-msg (assoc :system-message wire-sys-msg) (:available-tools config) (assoc :available-tools (:available-tools config)) (:excluded-tools config) (assoc :excluded-tools (:excluded-tools config)) + ;; SDK always sends `toolFilterPrecedence: "excluded"` so callers can + ;; compose include + exclude lists naturally (upstream PR #1428). + ;; Behavioral change for CLI mode: prior to PR #1428 the CLI default + ;; precedence was forwarded as-is. Allowlist-precedence is not exposed + ;; on the SDK boundary by design (mirrors upstream `resolveToolFilterOptions`). + true (assoc :tool-filter-precedence "excluded") wire-provider (assoc :provider wire-provider) true (assoc :request-permission true) (:streaming? config) (assoc :streaming (:streaming? config)) @@ -1612,6 +1836,9 @@ wire-sys-msg (assoc :system-message wire-sys-msg) (:available-tools config) (assoc :available-tools (:available-tools config)) (:excluded-tools config) (assoc :excluded-tools (:excluded-tools config)) + ;; SDK always sends `toolFilterPrecedence: "excluded"` (upstream PR #1428). + ;; See `build-create-session-params` for rationale. + true (assoc :tool-filter-precedence "excluded") wire-provider (assoc :provider wire-provider) true (assoc :request-permission (not (identical? (:on-permission-request config) @@ -1853,9 +2080,12 @@ [client config] (log/debug "Creating session with config: " (select-keys config [:model :session-id])) (validate-session-config! config) + (validate-tool-filters! config) + (validate-empty-mode-session-requirements! client config) (ensure-connected! client) (ensure-session-fs-handler-factory! client config) - (let [{:keys [connection-io]} @(:state client) + (let [config (normalize-config-for-mode client config) + {:keys [connection-io]} @(:state client) trace-ctx (get-trace-context (:on-get-trace-context client)) {:keys [transform-callbacks]} (extract-transform-callbacks (:system-message config)) cloud? (some? (:cloud config)) @@ -1887,6 +2117,8 @@ session-id (:session-id session)] (session/set-workspace-path! client session-id (:workspace-path result)) (session/set-capabilities! client session-id (:capabilities result)) + ;; Mode-specific post-create options patch (upstream PR #1428). + (apply-session-options-update! client session config) (log/info "Session created (cloud, server-assigned id): " session-id) session))) ;; Standard path: client supplies (or generates) the sessionId up front. @@ -1907,6 +2139,11 @@ {:requested session-id :returned returned-id}))) (session/set-workspace-path! client session-id (:workspace-path result)) (session/set-capabilities! client session-id (:capabilities result)) + ;; Mode-specific post-create options patch (upstream PR #1428). + ;; Helper handles its own cleanup on failure (disconnect! + + ;; remove-session!) before rethrowing; the outer catch's + ;; remove-session! is then a safe no-op. + (apply-session-options-update! client session config) (log/info "Session created: " session-id) session) (catch Throwable t @@ -1968,9 +2205,12 @@ (when (and (:provider config) (not (:model config))) (throw (ex-info "Invalid session config: :model is required when :provider (BYOK) is specified" {:config config}))) + (validate-tool-filters! config) + (validate-empty-mode-session-requirements! client config) (ensure-connected! client) (ensure-session-fs-handler-factory! client config) - (let [{:keys [connection-io]} @(:state client) + (let [config (normalize-config-for-mode client config) + {:keys [connection-io]} @(:state client) trace-ctx (get-trace-context (:on-get-trace-context client)) {:keys [transform-callbacks]} (extract-transform-callbacks (:system-message config)) params (merge trace-ctx (build-resume-session-params session-id config)) @@ -1983,6 +2223,8 @@ (let [result (proto/send-request! connection-io "session.resume" params)] (session/set-workspace-path! client session-id (:workspace-path result)) (session/set-capabilities! client session-id (:capabilities result)) + ;; Mode-specific post-resume options patch (upstream PR #1428). + (apply-session-options-update! client session config) session) (catch Throwable t (session/remove-session! client session-id) @@ -2016,9 +2258,12 @@ [client config] (log/debug "Creating session (async) with config: " (select-keys config [:model :session-id])) (validate-session-config! config) + (validate-tool-filters! config) + (validate-empty-mode-session-requirements! client config) (ensure-connected! client) (ensure-session-fs-handler-factory! client config) - (let [{:keys [connection-io]} @(:state client) + (let [config (normalize-config-for-mode client config) + {:keys [connection-io]} @(:state client) trace-ctx (get-trace-context (:on-get-trace-context client)) {:keys [transform-callbacks]} (extract-transform-callbacks (:system-message config)) cloud? (some? (:cloud config)) @@ -2068,8 +2313,11 @@ session-id (:session-id session)] (session/set-workspace-path! client session-id (:workspace-path result)) (session/set-capabilities! client session-id (:capabilities result)) - (log/info "Session created (async, cloud, server-assigned id): " session-id) - session))))))) + (let [r ( {} @@ -55,6 +55,11 @@ ;; tcpConnectionToken (upstream PR #1176) — required when server enforces a token tcp-connection-token (assoc "COPILOT_CONNECTION_TOKEN" tcp-connection-token) + ;; Client mode :empty disables the system keychain (upstream PR #1428). + ;; Applied in :overrides so the caller's `:env` cannot accidentally + ;; re-enable it under multitenancy hardening. + (= mode :empty) + (assoc "COPILOT_DISABLE_KEYTAR" "1") ;; OpenTelemetry (upstream PR #785) telemetry (as-> m @@ -88,6 +93,9 @@ - :tcp-connection-token - Connection token sent via COPILOT_CONNECTION_TOKEN (upstream PR #1176) - :remote? - When true, append `--remote` so the CLI exposes its session over a GitHub-hosted remote endpoint (upstream PR #1192) + - :mode - Client mode `:empty` or `:copilot-cli` (default). When `:empty`, + `COPILOT_DISABLE_KEYTAR=1` is forced into the spawned env so the + child CLI cannot reach the host keychain (upstream PR #1428). Returns a ManagedProcess record." [{:keys [cli-path cwd env use-stdio?] diff --git a/src/github/copilot_sdk/specs.clj b/src/github/copilot_sdk/specs.clj index 34faf96..c1c510e 100644 --- a/src/github/copilot_sdk/specs.clj +++ b/src/github/copilot_sdk/specs.clj @@ -186,23 +186,40 @@ ;; Factory fn: (session) → session-fs-handler (s/def ::create-session-fs-handler fn?) +;; Client mode (upstream PR #1428). `:copilot-cli` (the default) preserves +;; the historical behavior; `:empty` enables multitenancy hardening: +;; tenant-scoped storage is required, the system keychain is disabled, and +;; a set of safe session defaults is spread under the caller's config. +;; +;; Note: the unqualified key in client-options is `:mode`, but the existing +;; ::mode spec at line ~899 is already taken by message-options for +;; #{:enqueue :immediate}. We use a uniquely-named ::client-mode spec and +;; validate the unqualified `:mode` key via a predicate in ::client-options +;; (mirrors the ::remote-session-mode pattern further down). +(s/def ::client-mode #{:empty :copilot-cli}) + (def client-options-keys #{:cli-path :cli-args :cli-url :cwd :port :use-stdio? :log-level :auto-start? :auto-restart? :notification-queue-size :router-queue-size :tool-timeout-ms :env :github-token :use-logged-in-user? :is-child-process? :on-list-models :telemetry :on-get-trace-context - :session-fs :copilot-home :tcp-connection-token :remote?}) + :session-fs :copilot-home :tcp-connection-token :remote? + :mode}) (s/def ::client-options - (closed-keys - (s/keys :opt-un [::cli-path ::cli-args ::cli-url ::cwd ::port - ::use-stdio? ::log-level ::auto-start? ::auto-restart? - ::notification-queue-size ::router-queue-size - ::tool-timeout-ms ::env ::github-token ::use-logged-in-user? - ::is-child-process? ::on-list-models ::telemetry ::on-get-trace-context - ::session-fs ::copilot-home ::tcp-connection-token ::remote?]) - client-options-keys)) + (s/and + (closed-keys + (s/keys :opt-un [::cli-path ::cli-args ::cli-url ::cwd ::port + ::use-stdio? ::log-level ::auto-start? ::auto-restart? + ::notification-queue-size ::router-queue-size + ::tool-timeout-ms ::env ::github-token ::use-logged-in-user? + ::is-child-process? ::on-list-models ::telemetry ::on-get-trace-context + ::session-fs ::copilot-home ::tcp-connection-token ::remote?]) + client-options-keys) + (fn [m] + (or (not (contains? m :mode)) + (s/valid? ::client-mode (:mode m)))))) ;; ----------------------------------------------------------------------------- ;; Tool definitions @@ -601,7 +618,7 @@ ;; Multitenancy hardening flags (upstream PR #1474). All optional, plain ;; booleans/strings. Application-mode behavior is driven by Client Mode -;; (upstream PR #1428), which has been deferred to a dedicated future round. +;; (upstream PR #1428). (s/def ::skip-embedding-retrieval boolean?) (s/def ::organization-custom-instructions string?) (s/def ::enable-on-demand-instruction-discovery boolean?) @@ -610,6 +627,15 @@ (s/def ::enable-session-store boolean?) (s/def ::enable-skills boolean?) +;; Client-mode session option flags (upstream PR #1428). Sent via +;; session.options.update after session.create / session.resume. In empty +;; mode these get safe defaults applied beneath the caller's config; in +;; CLI mode they are only emitted when explicitly set. +(s/def ::skip-custom-instructions boolean?) +(s/def ::custom-agents-local-only boolean?) +(s/def ::coauthor-enabled boolean?) +(s/def ::manage-schedule-enabled boolean?) + ;; Reasoning summary mode (upstream PR #813 - pre-existing parity gap). ;; Wire enum: "none" | "concise" | "detailed". Mirrors upstream's ReasoningSummary type. (s/def ::reasoning-summary #{"none" "concise" "detailed"}) @@ -648,6 +674,10 @@ :enable-host-git-operations :enable-session-store :enable-skills + :skip-custom-instructions + :custom-agents-local-only + :coauthor-enabled + :manage-schedule-enabled :include-sub-agent-streaming-events?}) (s/def ::session-config @@ -682,6 +712,10 @@ ::enable-host-git-operations ::enable-session-store ::enable-skills + ::skip-custom-instructions + ::custom-agents-local-only + ::coauthor-enabled + ::manage-schedule-enabled ::include-sub-agent-streaming-events?]) session-config-keys)) @@ -710,6 +744,10 @@ :enable-host-git-operations :enable-session-store :enable-skills + :skip-custom-instructions + :custom-agents-local-only + :coauthor-enabled + :manage-schedule-enabled :include-sub-agent-streaming-events?}) (s/def ::resume-session-config @@ -741,6 +779,10 @@ ::enable-host-git-operations ::enable-session-store ::enable-skills + ::skip-custom-instructions + ::custom-agents-local-only + ::coauthor-enabled + ::manage-schedule-enabled ::include-sub-agent-streaming-events?]) resume-session-config-keys)) @@ -774,6 +816,10 @@ ::enable-host-git-operations ::enable-session-store ::enable-skills + ::skip-custom-instructions + ::custom-agents-local-only + ::coauthor-enabled + ::manage-schedule-enabled ::include-sub-agent-streaming-events?]) resume-session-config-keys)) diff --git a/src/github/copilot_sdk/tool_set.clj b/src/github/copilot_sdk/tool_set.clj new file mode 100644 index 0000000..bb4c037 --- /dev/null +++ b/src/github/copilot_sdk/tool_set.clj @@ -0,0 +1,91 @@ +(ns github.copilot-sdk.tool-set + "Helpers for building source-qualified tool filter patterns used by + `:available-tools` and `:excluded-tools` in session configs. + + The runtime matches each tool reference against patterns of the form + `\":\"` where source is one of `builtin`, `mcp`, or + `custom` and name is either a literal tool name or the wildcard + `\"*\"`. The bare wildcard `\"*\"` (no source) is intentionally + rejected by the SDK — apps must explicitly opt into a source so an + absent source can never silently grant access to unexpected tools. + + Mirrors upstream `ToolSet` and `BuiltInTools` from + `nodejs/src/toolSet.ts` (upstream PR #1428). + + Examples: + + (tool-set/builtin \"ask_user\") ;; => \"builtin:ask_user\" + (tool-set/builtin \"*\") ;; => \"builtin:*\" + (tool-set/mcp \"*\") ;; => \"mcp:*\" + (tool-set/builtins [\"task\" \"skill\"]) ;; => [\"builtin:task\" \"builtin:skill\"] + + ;; Ready-to-use: every built-in that is safely session-bounded. + tool-set/isolated ;; => [\"builtin:ask_user\" \"builtin:task_complete\" ...]") + +(def ^:private valid-name-regex + "Allowed characters in a tool name segment (mirrors upstream `nameRe`)." + #"^[a-zA-Z0-9_-]+$") + +(defn valid-name? + "True when `name` is a valid tool-name segment — alphanumeric plus + `_` and `-`, or the literal wildcard `\"*\"`. Returns false for any + non-string input." + [name] + (and (string? name) + (or (= "*" name) + (some? (re-matches valid-name-regex name))))) + +(defn- validate! [source name] + (when-not (valid-name? name) + (throw (ex-info (format "Invalid %s tool name: %s" source (pr-str name)) + {:source source :name name})))) + +(defn builtin + "Returns the filter pattern `\"builtin:\"`. `name` may be the + wildcard `\"*\"`. Throws on invalid names." + [name] + (validate! "builtin" name) + (str "builtin:" name)) + +(defn mcp + "Returns the filter pattern `\"mcp:\"`. `name` may be the + wildcard `\"*\"`. Throws on invalid names." + [name] + (validate! "mcp" name) + (str "mcp:" name)) + +(defn custom + "Returns the filter pattern `\"custom:\"`. `name` may be the + wildcard `\"*\"`. Throws on invalid names." + [name] + (validate! "custom" name) + (str "custom:" name)) + +(defn builtins + "Returns a vector of `\"builtin:\"` patterns, one per entry in + `names`. Throws on the first invalid name." + [names] + (mapv builtin names)) + +(def isolated-builtins + "Names of the built-in tools that are safely session-bounded (no host + I/O). Mirrors upstream `BuiltInTools.Isolated`. Use [[isolated]] for + the source-qualified form ready to drop into `:available-tools`." + ["ask_user" + "task_complete" + "exit_plan_mode" + "task" + "read_agent" + "write_agent" + "list_agents" + "send_inbox" + "context_board" + "skill"]) + +(def isolated + "Source-qualified `\"builtin:\"` patterns for every tool in + [[isolated-builtins]]. Drop directly into `:available-tools`: + + (copilot/create-session client + {:available-tools tool-set/isolated})" + (builtins isolated-builtins)) diff --git a/test/github/copilot_sdk/integration_test.clj b/test/github/copilot_sdk/integration_test.clj index 6ddf9f0..d9fc55c 100644 --- a/test/github/copilot_sdk/integration_test.clj +++ b/test/github/copilot_sdk/integration_test.clj @@ -5399,3 +5399,539 @@ (is (= custom-id (sdk/session-id session))) (sdk/destroy! session)))) +;; ----------------------------------------------------------------------------- +;; Client Mode (upstream PR #1428) — constructor + validation tests. +;; The session-time options.update orchestration and wire-shape parity tests +;; live further down once that plumbing lands; these are the pure-validation +;; checks (client construction + bare-`*` rejection + required :available-tools). +;; ----------------------------------------------------------------------------- + +(deftest test-empty-mode-requires-tenant-scoped-storage + (testing "construction without storage hook throws" + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Mode :empty requires" + (sdk/client {:mode :empty :auto-start? false})))) + (testing ":copilot-home satisfies the requirement" + (let [c (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-test" + :auto-start? false})] + (is (= :empty (get-in c [:options :mode]))) + (is (= "/tmp/empty-mode-test" (get-in c [:options :copilot-home]))))) + (testing ":session-fs satisfies the requirement" + (let [c (sdk/client {:mode :empty + :session-fs {:initial-cwd "/workspace" + :session-state-path "/state" + :conventions "posix"} + :auto-start? false})] + (is (= :empty (get-in c [:options :mode]))))) + (testing ":cli-url satisfies the requirement" + (let [c (sdk/client {:mode :empty + :cli-url "localhost:1234" + :auto-start? false})] + (is (= :empty (get-in c [:options :mode]))))) + (testing ":is-child-process? satisfies the requirement" + (let [c (sdk/client {:mode :empty + :is-child-process? true + :auto-start? false})] + (is (= :empty (get-in c [:options :mode])))))) + +(deftest test-empty-mode-default-and-cli-mode + (testing "default mode is :copilot-cli (no extra options needed)" + (let [c (sdk/client {:auto-start? false})] + ;; :mode is not auto-injected; absence is treated as :copilot-cli by + ;; downstream code. The point is that no extra options are required. + (is (nil? (get-in c [:options :mode])) + ":mode should be unset by default — downstream code treats nil as :copilot-cli"))) + (testing "explicit :copilot-cli mode imposes no extra requirements" + (let [c (sdk/client {:mode :copilot-cli :auto-start? false})] + (is (= :copilot-cli (get-in c [:options :mode])))))) + +(deftest test-empty-mode-spec-validation + (testing "an unknown :mode value is rejected by the spec" + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Invalid client options" + (sdk/client {:mode :bogus :copilot-home "/tmp/x" :auto-start? false}))))) + +(deftest test-bare-star-rejected-in-available-tools + (testing ":available-tools containing bare * is rejected at create-session" + (let [c (sdk/client {:auto-start? false})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Invalid availableTools entry '\*'" + (sdk/create-session c {:available-tools ["*"]})))))) + +(deftest test-bare-star-rejected-in-excluded-tools + (testing ":excluded-tools containing bare * is rejected at create-session" + (let [c (sdk/client {:auto-start? false})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Invalid excludedTools entry '\*'" + (sdk/create-session c {:excluded-tools ["*"]})))))) + +(deftest test-bare-star-rejected-in-resume-session + (testing ":available-tools containing bare * is rejected at resume-session" + (let [c (sdk/client {:auto-start? false})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Invalid availableTools entry '\*'" + (sdk/resume-session c "session-id" {:available-tools ["*"]})))))) + +(deftest test-source-qualified-tools-accepted + (testing "source-qualified patterns pass the bare-* validation" + ;; We can't fully exercise create-session without a started client, but + ;; the validation step throws BEFORE ensure-connected!, so a non-throw + ;; below means we made it past validate-tool-filters! (further work + ;; will hit the connection check). + (let [c (sdk/client {:auto-start? false})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Client is not started|not connected|Connection" + (sdk/create-session c {:available-tools ["builtin:*" "mcp:my_server"]})))))) + +(deftest test-empty-mode-requires-available-tools-on-create + (testing "empty mode rejects create-session without :available-tools" + (let [c (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-test" + :auto-start? false})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Mode :empty requires every session to specify :available-tools" + (sdk/create-session c {})))))) + +(deftest test-empty-mode-allows-empty-available-tools + (testing "empty mode accepts :available-tools [] as explicit opt-in to no tools" + ;; The validation passes; downstream will fail because client is not + ;; started. Distinguish those errors by message. + (let [c (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-test" + :auto-start? false})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Client is not started|not connected|Connection" + (sdk/create-session c {:available-tools []})))))) + +(deftest test-empty-mode-requires-available-tools-on-resume + (testing "empty mode rejects resume-session without :available-tools" + (let [c (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-test" + :auto-start? false})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Mode :empty requires every session to specify :available-tools" + (sdk/resume-session c "session-id" {})))))) + +(deftest test-cli-mode-does-not-require-available-tools + (testing "cli mode allows create-session without :available-tools" + (let [c (sdk/client {:auto-start? false})] + ;; Validation passes; downstream throws because not started. + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Client is not started|not connected|Connection" + (sdk/create-session c {})))))) + + +;; ----------------------------------------------------------------------------- +;; Client Mode (upstream PR #1428) — wire payload tests: +;; - `tool-filter-precedence` is always emitted as "excluded" (both modes). +;; - In `:empty` mode the 9 mode defaults are spread under caller config. +;; ----------------------------------------------------------------------------- + +(deftest test-tool-filter-precedence-always-excluded-on-create + (testing "session.create always sends toolFilterPrecedence=\"excluded\" (PR #1428)" + (let [seen (atom {}) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (when (= "session.create" method) + (swap! seen assoc method params)))) + _ (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :model "gpt-5.4"}) + create-params (get @seen "session.create")] + (is (= "excluded" (:toolFilterPrecedence create-params)) + "tool-filter-precedence must always be \"excluded\" in CLI mode")))) + +(deftest test-tool-filter-precedence-always-excluded-on-resume + (testing "session.resume always sends toolFilterPrecedence=\"excluded\" (PR #1428)" + (let [seen (atom {}) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (when (= "session.resume" method) + (swap! seen assoc method params)))) + _ (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all}) + session-id (sdk/get-last-session-id *test-client*) + _ (sdk/resume-session *test-client* session-id + {:on-permission-request sdk/approve-all}) + resume-params (get @seen "session.resume")] + (is (= "excluded" (:toolFilterPrecedence resume-params)) + "tool-filter-precedence must always be \"excluded\" on resume")))) + +(deftest test-empty-mode-spreads-config-defaults-under-caller + (testing "empty mode spreads the 9 safe defaults under caller config (PR #1428)" + (let [server (mock/create-mock-server) + _ (mock/start-mock-server! server) + seen (atom {}) + _ (mock/set-request-hook! server + (fn [method params] + (when (= "session.create" method) + (swap! seen assoc method params)))) + client (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-wire-test" + :auto-start? false}) + [in out] (mock/client-streams server)] + (try + (client/connect-with-streams! client in out) + (sdk/create-session client + {:on-permission-request sdk/approve-all + :available-tools ["builtin:think"]}) + (let [p (get @seen "session.create")] + (is (= false (:enableSessionTelemetry p))) + (is (= "in-memory" (:mcpOAuthTokenStorage p))) + (is (= true (:skipEmbeddingRetrieval p))) + (is (= "in-memory" (:embeddingCacheStorage p))) + (is (= false (:enableOnDemandInstructionDiscovery p))) + (is (= false (:enableFileHooks p))) + (is (= false (:enableHostGitOperations p))) + (is (= false (:enableSessionStore p))) + (is (= false (:enableSkills p))) + (is (= "excluded" (:toolFilterPrecedence p)))) + (finally + (try (sdk/stop! client) (catch Exception _)) + (Thread/sleep 50) + (mock/stop-mock-server! server)))))) + +(deftest test-empty-mode-caller-config-wins-over-mode-defaults + (testing "caller-provided config values always win over mode defaults (PR #1428)" + (let [server (mock/create-mock-server) + _ (mock/start-mock-server! server) + seen (atom {}) + _ (mock/set-request-hook! server + (fn [method params] + (when (= "session.create" method) + (swap! seen assoc method params)))) + client (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-override-test" + :auto-start? false}) + [in out] (mock/client-streams server)] + (try + (client/connect-with-streams! client in out) + ;; Caller overrides 2 of the 9 defaults — those values must win. + (sdk/create-session client + {:on-permission-request sdk/approve-all + :available-tools ["builtin:think"] + :enable-session-telemetry? true + :enable-skills true}) + (let [p (get @seen "session.create")] + (is (= true (:enableSessionTelemetry p)) + "caller-provided :enable-session-telemetry? must override the mode default") + (is (= true (:enableSkills p)) + "caller-provided :enable-skills must override the mode default") + ;; Other defaults still apply + (is (= true (:skipEmbeddingRetrieval p)))) + (finally + (try (sdk/stop! client) (catch Exception _)) + (Thread/sleep 50) + (mock/stop-mock-server! server)))))) + +(deftest test-cli-mode-does-not-spread-mode-defaults + (testing "CLI mode does NOT spread the empty-mode defaults (PR #1428)" + (let [seen (atom {}) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (when (= "session.create" method) + (swap! seen assoc method params)))) + _ (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all}) + p (get @seen "session.create")] + ;; None of the 9 mode-default flags should appear unless caller set them. + (is (not (contains? p :enableSessionTelemetry))) + (is (not (contains? p :skipEmbeddingRetrieval))) + (is (not (contains? p :enableFileHooks))) + (is (not (contains? p :enableSkills)))))) + +;; ----------------------------------------------------------------------------- +;; Client Mode (upstream PR #1428) — system message normalization tests. +;; In :empty mode, the SDK enforces that environment_context is stripped from +;; the system message (unless app has taken control of it). Mirrors upstream +;; `getSystemMessageConfigForMode`. +;; ----------------------------------------------------------------------------- + +(defn- empty-mode-create-with-system-message + "Spin up a fresh empty-mode client against a fresh mock server, invoke + create-session with the supplied :system-message, and return the + captured wire payload's :systemMessage field." + [system-message] + (let [server (mock/create-mock-server) + _ (mock/start-mock-server! server) + seen (atom {}) + _ (mock/set-request-hook! server + (fn [method params] + (when (= "session.create" method) + (swap! seen assoc method params)))) + client (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-sm-test" + :auto-start? false}) + [in out] (mock/client-streams server)] + (try + (client/connect-with-streams! client in out) + (sdk/create-session client + (cond-> {:on-permission-request sdk/approve-all + :available-tools []} + system-message (assoc :system-message system-message))) + (-> (get @seen "session.create") :systemMessage) + (finally + (try (sdk/stop! client) (catch Exception _)) + (Thread/sleep 50) + (mock/stop-mock-server! server))))) + +(deftest test-empty-mode-system-message-default + (testing "no caller :system-message → emit customize with env-context removed" + (let [sm (empty-mode-create-with-system-message nil)] + (is (= "customize" (:mode sm))) + (is (= {:action "remove"} (get-in sm [:sections :environment_context])))))) + +(deftest test-empty-mode-system-message-replace-passes-through + (testing ":replace mode is preserved unchanged in empty mode" + (let [sm (empty-mode-create-with-system-message + {:mode :replace :content "Replacement text."})] + (is (= "replace" (:mode sm))) + (is (= "Replacement text." (:content sm))) + (is (not (contains? sm :sections)))))) + +(deftest test-empty-mode-system-message-append-promoted-to-customize + (testing ":append mode is promoted to :customize, content preserved, env-context removed" + (let [sm (empty-mode-create-with-system-message + {:mode :append :content "Extra instructions."})] + (is (= "customize" (:mode sm))) + (is (= "Extra instructions." (:content sm)) + "caller :content must be preserved verbatim") + (is (= {:action "remove"} (get-in sm [:sections :environment_context])))))) + +(deftest test-empty-mode-system-message-customize-adds-env-context-remove + (testing ":customize without env-context override → SDK adds env-context remove" + (let [sm (empty-mode-create-with-system-message + {:mode :customize + :sections {:tone {:action :replace :content "Be terse."}}})] + (is (= "customize" (:mode sm))) + (is (= {:action "remove"} (get-in sm [:sections :environment_context])) + "env-context section must be removed") + (is (some? (get-in sm [:sections :tone])) + "caller's :tone section must be preserved")))) + +(deftest test-empty-mode-system-message-customize-no-sections-key + (testing ":customize with NO :sections key at all → SDK adds :sections with env-context remove" + (let [sm (empty-mode-create-with-system-message {:mode :customize})] + (is (= "customize" (:mode sm))) + (is (= {:action "remove"} (get-in sm [:sections :environment_context])) + "env-context section must be added even when caller omits :sections entirely")))) + +(deftest test-empty-mode-system-message-customize-respects-app-env-context + (testing ":customize with explicit env-context override → SDK does NOT touch it" + (let [sm (empty-mode-create-with-system-message + {:mode :customize + :sections {:environment-context {:action :replace :content "Custom env."}}})] + (is (= "customize" (:mode sm))) + (is (= {:action "replace" :content "Custom env."} + (get-in sm [:sections :environment_context])) + "app's env-context override must win unchanged")))) + +(deftest test-cli-mode-system-message-untouched + (testing "CLI mode does NOT normalize :system-message (preserve historical behavior)" + (let [seen (atom {}) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (when (= "session.create" method) + (swap! seen assoc method params)))) + _ (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :system-message {:mode :append + :content "Just append this."}}) + sm (-> (get @seen "session.create") :systemMessage)] + (is (= "append" (:mode sm)) "CLI mode keeps :append, no promotion") + (is (= "Just append this." (:content sm)))))) + +;; ----------------------------------------------------------------------------- +;; Client Mode (upstream PR #1428) — session.options.update RPC tests. +;; After session.create succeeds, the SDK issues a follow-up RPC to set the 4 +;; overridable feature flags and (in :empty mode) the installedPlugins list. +;; Mirrors upstream `updateSessionOptionsForMode`. +;; ----------------------------------------------------------------------------- + +(defn- empty-mode-capture-options-update + "Spin up a fresh empty-mode client, create a session with the given extra + config map, and return the captured session.options.update wire params + (or nil if the RPC was not issued)." + [extra-config] + (let [server (mock/create-mock-server) + _ (mock/start-mock-server! server) + seen (atom nil) + _ (mock/set-request-hook! server + (fn [method params] + (when (= "session.options.update" method) + (reset! seen params)))) + client (sdk/client {:mode :empty + :copilot-home "/tmp/empty-mode-opts-test" + :auto-start? false}) + [in out] (mock/client-streams server)] + (try + (client/connect-with-streams! client in out) + (sdk/create-session client + (merge {:on-permission-request sdk/approve-all + :available-tools []} + extra-config)) + @seen + (finally + (try (sdk/stop! client) (catch Exception _)) + (Thread/sleep 50) + (mock/stop-mock-server! server))))) + +(deftest test-empty-mode-options-update-defaults + (testing "empty mode sends session.options.update with safe flag defaults + empty plugins" + (let [p (empty-mode-capture-options-update {})] + (is (some? p) "session.options.update must be issued in :empty mode") + (is (= true (:skipCustomInstructions p))) + (is (= true (:customAgentsLocalOnly p))) + (is (= false (:coauthorEnabled p))) + (is (= false (:manageScheduleEnabled p))) + (is (= [] (:installedPlugins p))) + (is (string? (:sessionId p)) "session-id must accompany the patch")))) + +(deftest test-empty-mode-options-update-caller-overrides + (testing "caller-supplied flags override empty-mode defaults in options.update patch" + (let [p (empty-mode-capture-options-update + {:skip-custom-instructions false + :coauthor-enabled true})] + (is (= false (:skipCustomInstructions p)) "caller false must win over default true") + (is (= true (:coauthorEnabled p)) "caller true must win over default false") + (is (= true (:customAgentsLocalOnly p)) "untouched flags keep their defaults") + (is (= false (:manageScheduleEnabled p))) + (is (= [] (:installedPlugins p)) "installedPlugins always forced to [] in :empty mode")))) + +(deftest test-cli-mode-options-update-skipped-when-no-caller-flags + (testing "CLI mode does NOT issue session.options.update when caller sets no flags" + (let [seen (atom nil) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (when (= "session.options.update" method) + (reset! seen params)))) + _ (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all})] + (Thread/sleep 100) + (is (nil? @seen) + "no RPC should be sent when the patch would be empty")))) + +(deftest test-cli-mode-options-update-forwards-only-explicit-flags + (testing "CLI mode forwards ONLY caller-supplied flags via options.update (no defaults, no installedPlugins)" + (let [seen (atom nil) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (when (= "session.options.update" method) + (reset! seen params)))) + _ (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :manage-schedule-enabled true}) + p @seen] + (is (some? p) "RPC should be issued because caller set a flag") + (is (= true (:manageScheduleEnabled p))) + (is (not (contains? p :skipCustomInstructions)) "untouched flags must NOT appear") + (is (not (contains? p :customAgentsLocalOnly))) + (is (not (contains? p :coauthorEnabled))) + (is (not (contains? p :installedPlugins)) + "installedPlugins must NOT be forced in :copilot-cli mode")))) + +(deftest test-empty-mode-options-update-async-path + (testing "async