diff --git a/.copilot-schema-version b/.copilot-schema-version index 56d0dad..a700f32 100644 --- a/.copilot-schema-version +++ b/.copilot-schema-version @@ -1 +1 @@ -1.0.48 +1.0.49-1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0918531..6dc0ff7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,65 @@ All notable changes to this project will be documented in this file. This change ## [Unreleased] ### Added (post-v1.0.0-beta.4 sync) +- **`:session-id` on hook input maps** — `:on-hook-invoke` handlers now receive + a `:session-id` key on the input map. When the upstream wire payload includes + a `sessionId` (sub-agent hooks), the wire-provided value is preserved; + otherwise the SDK fills in the parent session id as a convenience. + (upstream PR #1290) +- **`:cloud` session config option (create only)** — `create-session` accepts + an optional `:cloud` map for creating a remote cloud session. Shape: + `{:repository {:owner "octocat" :name "hello-world" :branch "main"}}` — `:owner` + and `:name` are required non-blank strings; `:branch` is optional. Forwarded on + the wire as `cloud.repository.*`. Matches upstream's `CloudSessionOptions` / + `CloudSessionRepository`. Not accepted on `resume-session` / `join-session`, + matching upstream `ResumeSessionConfig` (Pick excludes `cloud`). + (upstream PR #1306) +- **Optional permission and tool callbacks (manual pending RPCs)** — Following + upstream PR #1308, `:on-permission-request` is now **optional** on + `create-session` and `resume-session`, and `:handler` is optional on tools + built via `tools/define-tool`. When omitted, the runtime no longer + auto-responds to permission requests or tool calls. Applications can resolve + these requests asynchronously via the new public functions: + - `sdk/handle-pending-tool-call!` / `sdk/wire (:cloud config))) (:model-capabilities config) (assoc :model-capabilities (util/clj->wire (:model-capabilities config))) true (assoc :include-sub-agent-streaming-events @@ -1609,8 +1618,13 @@ (defn create-session "Create a new conversation session. - Config options (`:on-permission-request` is **required**): - - :on-permission-request - Permission handler function (**required**, e.g. `approve-all`) + Config options (`:on-permission-request` is **optional** since upstream + PR #1308 — omit it to leave permission requests pending for manual + resolution via `copilot/handle-pending-permission-request!`): + - :on-permission-request - Permission handler function (optional, e.g. `approve-all`). + When omitted, permission requests are surfaced as + `:copilot/permission.requested` events and remain + pending until resolved by the application. - :session-id - Custom session ID - :client-name - Client name to identify the application (included in User-Agent header) - :model - Model to use (e.g., \"gpt-5.4\") @@ -1658,6 +1672,11 @@ - :remote-session - Keyword. Per-session Mission Control remote mode: :off, :export, or :on. Forwarded as `remoteSession` on session.create. When omitted, the CLI applies its default. (upstream PR #1295, CLI 1.0.48) + - :cloud - Map. Creates a remote session in the cloud instead of a local session. + Optional `:repository` associates the cloud session with a GitHub + repository: `{:repository {:owner \"...\" :name \"...\" :branch \"...\"}}`. + `:branch` is optional. Forwarded as `cloud` on session.create. + (upstream PR #1306) Returns a CopilotSession." [client config] @@ -1694,8 +1713,13 @@ (defn resume-session "Resume an existing session by ID. - Config options (`:on-permission-request` is **required**): - - :on-permission-request - Permission handler function (**required**, e.g. `approve-all`) + Config options (`:on-permission-request` is **optional** since upstream + PR #1308 — omit it to leave permission requests pending for manual + resolution via `copilot/handle-pending-permission-request!`): + - :on-permission-request - Permission handler function (optional, e.g. `approve-all`). + When omitted, permission requests are surfaced as + `:copilot/permission.requested` events and remain + pending until resolved by the application. - :client-name - Client name to identify the application (included in User-Agent header) - :model - Change the model for the resumed session - :tools - Tools exposed to the CLI server @@ -1734,11 +1758,6 @@ Returns a CopilotSession." [client session-id config] - (when-not (:on-permission-request config) - (throw (ex-info (str "An :on-permission-request handler is required when resuming a session. " - "For example, to allow all permissions, use " - "{:on-permission-request copilot/approve-all}.") - {:config config}))) (when-not (s/valid? ::specs/resume-session-config config) (throw (ex-info "Invalid resume session config" {:config config @@ -1775,7 +1794,8 @@ (defn ! >!! !! :token)) - tool-handlers (into {} (map (fn [t] [(:tool-name t) (:tool-handler t)]) tools)) + ;; Upstream PR #1308: declaration-only tools (no :tool-handler) are + ;; left out of the handler map — they're distinguished from tools with + ;; handlers, and unhandled invocations are left pending for manual + ;; resolution via handle-pending-tool-call!. + tool-handlers (into {} (keep (fn [t] + (when-let [h (:tool-handler t)] + [(:tool-name t) h])) + tools)) command-handlers (into {} (map (fn [c] [(:name c) (:command-handler c)]) commands))] ;; Store session state and IO in client's atom (swap! (:state client) @@ -631,11 +639,18 @@ "sessionEnd" :on-session-end "errorOccurred" :on-error-occurred nil) - handler (when handler-key (get hooks handler-key))] - (if-not handler - {:result nil} - (try - (let [result (handler input {:session-id session-id}) + handler (when handler-key (get hooks handler-key))] + (if-not handler + {:result nil} + (try + (let [;; Upstream PR #1290: BaseHookInput.sessionId. Preserve the + ;; wire-provided :session-id when present (it may identify a + ;; sub-agent session distinct from the outer session-id); + ;; otherwise fall back to the outer session-id. + input (cond-> input + (not (contains? input :session-id)) + (assoc :session-id session-id)) + result (handler input {:session-id session-id}) ;; If handler returns a channel, await it result (if (channel? result) ( result) + - :handler - Function (fn [args invocation] -> result). + **Optional** since upstream PR #1308: when + omitted, the tool is declaration-only and + the runtime emits a + `:copilot/external_tool.requested` event + whenever the LLM calls it. Applications + resolve the pending call by reading the + `:request-id` from the event data and + calling `copilot/handle-pending-tool-call!` + (or the async ``copilot/ {:tool-name name :tool-description description - :tool-parameters parameters - :tool-handler handler} + :tool-parameters parameters} + ;; Upstream PR #1308: handler is optional. Declaration-only tools (no + ;; handler) are surfaced as external_tool.requested events; consumers + ;; resolve them via handle-pending-tool-call!. + (some? handler) + (assoc :tool-handler handler) (some? overrides-built-in-tool) (assoc :overrides-built-in-tool overrides-built-in-tool))) @@ -56,10 +79,22 @@ - opts map: - :description - Tool description - :spec - A clojure.spec for the arguments - - :handler - Function (fn [args invocation] -> result) + - :handler - Function (fn [args invocation] -> result). + **Optional** since upstream PR #1308: when + omitted, no `:tool-handler` is installed + and the tool is declaration-only. The + runtime emits a + `:copilot/external_tool.requested` event + on call; applications resolve it via + `copilot/handle-pending-tool-call!` + (or ``copilot/ {:tool-name name :tool-description description - :tool-parameters nil ; User should provide JSON schema if needed - :tool-handler (fn [args invocation] + :tool-parameters nil} ; User should provide JSON schema if needed + ;; Upstream PR #1308: handler is optional. Declaration-only tools (no + ;; handler) are surfaced as external_tool.requested events; consumers + ;; resolve them via handle-pending-tool-call!. + (some? handler) + (assoc :tool-handler (fn [args invocation] (if (and spec (not (s/valid? spec args))) {:text-result-for-llm (str "Invalid arguments: " (s/explain-str spec args)) :result-type "failure" :error "spec validation failed"} - (handler args invocation)))} + (handler args invocation)))) (some? overrides-built-in-tool) (assoc :overrides-built-in-tool overrides-built-in-tool))) diff --git a/test/github/copilot_sdk/integration_test.clj b/test/github/copilot_sdk/integration_test.clj index 723a814..2712f0d 100644 --- a/test/github/copilot_sdk/integration_test.clj +++ b/test/github/copilot_sdk/integration_test.clj @@ -1192,6 +1192,65 @@ {:on-permission-request sdk/approve-all :remote-session :bogus}))))) +(deftest test-cloud-session-config-forwarded-on-wire + (testing "cloud {:repository {...}} is forwarded as cloud.repository on session.create (upstream PR #1306)" + (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 + :cloud {:repository {:owner "octocat" + :name "hello-world" + :branch "main"}}}) + create-params (get @seen "session.create")] + (is (= {:owner "octocat" :name "hello-world" :branch "main"} + (get-in create-params [:cloud :repository]))) + (is (not (contains? create-params :cloud-repository))))) + + (testing "cloud :repository may omit :branch" + (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 + :cloud {:repository {:owner "octocat" + :name "hello-world"}}}) + create-params (get @seen "session.create")] + (is (= {:owner "octocat" :name "hello-world"} + (get-in create-params [:cloud :repository]))))) + + (testing "cloud {} (empty options) is forwarded as empty map" + (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 + :cloud {}}) + create-params (get @seen "session.create")] + (is (= {} (:cloud create-params))))) + + (testing "cloud is omitted from wire when not set" + (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}) + create-params (get @seen "session.create")] + (is (not (contains? create-params :cloud))))) + + (testing "cloud config with invalid :repository shape is rejected by spec" + (is (thrown? Exception + (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :cloud {:repository {:branch "main"}}}))) ; missing :owner and :name + (is (thrown? Exception + (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :cloud {:repository "octocat/hello-world"}}))))) + (deftest test-agent-forwarded-on-wire (testing "agent is forwarded in session.create when set (upstream PR #722)" (let [seen (atom {}) @@ -1304,23 +1363,16 @@ (is (= {:kind :approve-once} (sdk/approve-all {:permission-kind kind} {:session-id "s1"})))))) -(deftest test-permission-handler-required - (testing "create-session throws without :on-permission-request" - (is (thrown-with-msg? clojure.lang.ExceptionInfo - #"on-permission-request handler is required" - (sdk/create-session *test-client* {})))) +(deftest test-permission-handler-now-optional + ;; Upstream PR #1308 made :on-permission-request optional. Previously this + ;; test asserted that create-session/resume-session threw without it. + (testing "create-session succeeds without :on-permission-request" + (is (some? (sdk/create-session *test-client* {})))) - (testing "create-session throws with nil handler" - (is (thrown-with-msg? clojure.lang.ExceptionInfo - #"on-permission-request handler is required" - (sdk/create-session *test-client* {:on-permission-request nil})))) - - (testing "resume-session throws without :on-permission-request" - (let [session (sdk/create-session *test-client* {:on-permission-request sdk/approve-all}) - session-id (sdk/session-id session)] - (is (thrown-with-msg? clojure.lang.ExceptionInfo - #"on-permission-request handler is required" - (sdk/resume-session *test-client* session-id {})))))) + (testing "resume-session succeeds without :on-permission-request" + (let [_ (sdk/create-session *test-client* {:on-permission-request sdk/approve-all}) + session-id (sdk/get-last-session-id *test-client*)] + (is (some? (sdk/resume-session *test-client* session-id {})))))) (deftest test-permission-denied-with-deny-handler (testing "Permission requests are denied when handler denies" @@ -2702,6 +2754,70 @@ :cwd "/workspace"}})] (is (nil? (:result response)))))) +(deftest test-hooks-input-exposes-session-id + (testing "hook input includes :session-id (upstream PR #1290 — BaseHookInput.sessionId)" + (let [handler-called (atom nil) + session (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :hooks {:on-pre-tool-use + (fn [input _ctx] + (reset! handler-called input) + nil)}}) + session-id (sdk/session-id session) + _ (mock/send-rpc-request! *mock-server* + "hooks.invoke" + {:sessionId session-id + :hookType "preToolUse" + :input {:toolName "bash" + :toolArgs {} + :sessionId session-id + :timestamp 12345 + :cwd "/workspace"}})] + (is (some? @handler-called)) + (is (= session-id (:session-id @handler-called))))) + + (testing "hook input :session-id preserves wire-provided value (sub-agent case)" + (let [handler-called (atom nil) + session (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :hooks {:on-pre-tool-use + (fn [input _ctx] + (reset! handler-called input) + nil)}}) + parent-session-id (sdk/session-id session) + sub-agent-session-id "sub-agent-session-xyz" + _ (mock/send-rpc-request! *mock-server* + "hooks.invoke" + {:sessionId parent-session-id + :hookType "preToolUse" + :input {:toolName "bash" + :toolArgs {} + :sessionId sub-agent-session-id + :timestamp 12345 + :cwd "/workspace"}})] + (is (some? @handler-called)) + (is (= sub-agent-session-id (:session-id @handler-called))))) + + (testing "hook input :session-id falls back to outer session-id when wire omits it" + (let [handler-called (atom nil) + session (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :hooks {:on-pre-tool-use + (fn [input _ctx] + (reset! handler-called input) + nil)}}) + session-id (sdk/session-id session) + _ (mock/send-rpc-request! *mock-server* + "hooks.invoke" + {:sessionId session-id + :hookType "preToolUse" + :input {:toolName "bash" + :toolArgs {} + :timestamp 12345 + :cwd "/workspace"}})] + (is (some? @handler-called)) + (is (= session-id (:session-id @handler-called)))))) + ;; ----------------------------------------------------------------------------- ;; User Input Handler Tests (server→client RPC) ;; ----------------------------------------------------------------------------- @@ -4042,3 +4158,238 @@ {:tool-call-id "tc-1" :agent-name "rubber-duck" :agent-display-name "Rubber Duck"})))) + +;; ----------------------------------------------------------------------------- +;; Optional callbacks (upstream PR #1308) +;; ----------------------------------------------------------------------------- + +(deftest test-create-session-without-permission-handler + (testing "create-session accepts omission of :on-permission-request (upstream PR #1308)" + (let [requests (atom []) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (swap! requests conj {:method method :params params}))) + session (sdk/create-session *test-client* {}) + create-rpcs (filter #(= "session.create" (:method %)) @requests)] + (is (= 1 (count create-rpcs)) "session.create still issued") + (is (some? (sdk/session-id session)))))) + +(deftest test-resume-session-without-permission-handler + (testing "resume-session accepts omission of :on-permission-request" + (let [requests (atom []) + _ (sdk/create-session *test-client* {}) + session-id (sdk/get-last-session-id *test-client*) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (swap! requests conj {:method method :params params}))) + resumed (sdk/resume-session *test-client* session-id {}) + resume-rpcs (filter #(= "session.resume" (:method %)) @requests)] + (is (= 1 (count resume-rpcs))) + (is (some? (sdk/session-id resumed)))))) + +(deftest test-define-tool-without-handler + (testing "define-tool accepts omission of :handler (declaration-only tool, upstream PR #1308)" + (let [tool (tools/define-tool "manual_lookup" + {:description "Look up status manually" + :parameters {:type "object" + :properties {:id {:type "string"}} + :required ["id"]}})] + (is (= "manual_lookup" (:tool-name tool))) + (is (not (contains? tool :tool-handler)) + "tool map must NOT contain :tool-handler key when handler omitted") + ;; Must pass tool spec validation (handler now optional) + (is (s/valid? :github.copilot-sdk.specs/tool tool)))) + + (testing "define-tool-from-spec accepts omission of :handler (upstream PR #1308)" + (let [tool (tools/define-tool-from-spec "manual_spec_tool" + {:description "Declaration-only spec tool"})] + (is (= "manual_spec_tool" (:tool-name tool))) + (is (not (contains? tool :tool-handler)) + "define-tool-from-spec must NOT install a wrapper handler when :handler omitted") + (is (s/valid? :github.copilot-sdk.specs/tool tool))))) + +(deftest test-declaration-only-tool-not-stored-in-handler-map + (testing "declaration-only tools don't populate :tool-handlers" + (let [decl-tool (tools/define-tool "decl_only" {:description "decl-only"}) + handled-tool (tools/define-tool "with_handler" + {:description "has handler" + :handler (fn [_ _] "ok")}) + session (sdk/create-session *test-client* + {:on-permission-request sdk/approve-all + :tools [decl-tool handled-tool]}) + session-id (sdk/session-id session) + tool-handlers (get-in @(:state *test-client*) + [:sessions session-id :tool-handlers])] + (is (not (contains? tool-handlers "decl_only")) + "declaration-only tools must not be in :tool-handlers") + (is (contains? tool-handlers "with_handler") + "tools with handlers must remain in :tool-handlers")))) + +(deftest test-handle-pending-permission-request!-sends-rpc + (testing "handle-pending-permission-request! issues session.permissions.handlePendingPermissionRequest" + (let [requests (atom []) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (swap! requests conj {:method method :params params}))) + session (sdk/create-session *test-client* {})] + (sdk/handle-pending-permission-request! session + {:request-id "req-42" + :result {:kind :approve-once}}) + (let [pending-rpcs (filter #(= "session.permissions.handlePendingPermissionRequest" + (:method %)) + @requests)] + (is (= 1 (count pending-rpcs))) + (is (= "req-42" (get-in (first pending-rpcs) [:params :requestId]))) + ;; Result is wire-serialized via clj->wire: keyword :kind → "approve-once" + (is (= "approve-once" (get-in (first pending-rpcs) + [:params :result :kind])))))) + + (testing "handle-pending-permission-request! accepts :user-not-available" + (let [requests (atom []) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (swap! requests conj {:method method :params params}))) + session (sdk/create-session *test-client* {})] + (sdk/handle-pending-permission-request! session + {:request-id "req-una" + :result {:kind :user-not-available}}) + (let [pending-rpcs (filter #(= "session.permissions.handlePendingPermissionRequest" + (:method %)) + @requests)] + (is (= 1 (count pending-rpcs))) + (is (= "user-not-available" (get-in (first pending-rpcs) + [:params :result :kind])))))) + + (testing "handle-pending-permission-request! accepts :approve-permanently" + (let [requests (atom []) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (swap! requests conj {:method method :params params}))) + session (sdk/create-session *test-client* {})] + (sdk/handle-pending-permission-request! session + {:request-id "req-perm" + :result {:kind :approve-permanently}}) + (let [pending-rpcs (filter #(= "session.permissions.handlePendingPermissionRequest" + (:method %)) + @requests)] + (is (= 1 (count pending-rpcs))) + (is (= "approve-permanently" (get-in (first pending-rpcs) + [:params :result :kind])))))) + + (testing "handle-pending-permission-request! rejects :no-result" + (let [session (sdk/create-session *test-client* {})] + (is (thrown? Exception + (sdk/handle-pending-permission-request! + session + {:request-id "req-1" :result {:kind :no-result}}))))) + + (testing "handle-pending-permission-request! rejects malformed result" + (let [session (sdk/create-session *test-client* {})] + (is (thrown? Exception + (sdk/handle-pending-permission-request! + session + {:request-id "req-1" :result "not-a-map"}))))) + + (testing "handle-pending-permission-request! rejects blank :request-id" + (let [session (sdk/create-session *test-client* {})] + (is (thrown-with-msg? Exception #":request-id must be a non-blank string" + (sdk/handle-pending-permission-request! + session + {:request-id "" :result {:kind :approve-once}}))) + (is (thrown-with-msg? Exception #":request-id must be a non-blank string" + (sdk/handle-pending-permission-request! + session + {:request-id " " :result {:kind :approve-once}}))))) + + (testing "handle-pending-permission-request! rejects non-keyword :kind" + (let [session (sdk/create-session *test-client* {})] + (is (thrown-with-msg? Exception #":kind must be a keyword" + (sdk/handle-pending-permission-request! + session + {:request-id "req-1" :result {:kind "approve-once"}}))) + (is (thrown-with-msg? Exception #":kind must be a keyword" + (sdk/handle-pending-permission-request! + session + {:request-id "req-1" :result {:kind 42}}))))) + + (testing "handle-pending-permission-request! rejects unsupported :kind" + (let [session (sdk/create-session *test-client* {})] + (is (thrown-with-msg? Exception #"not a supported permission decision" + (sdk/handle-pending-permission-request! + session + {:request-id "req-1" :result {:kind :totally-made-up}})))))) + +(deftest test-handle-pending-tool-call!-sends-rpc + (testing "handle-pending-tool-call! with :result issues session.tools.handlePendingToolCall" + (let [requests (atom []) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (swap! requests conj {:method method :params params}))) + session (sdk/create-session *test-client* {})] + (sdk/handle-pending-tool-call! session + {:request-id "tool-req-7" + :result "MANUAL_STATUS_OK"}) + (let [pending (filter #(= "session.tools.handlePendingToolCall" (:method %)) + @requests)] + (is (= 1 (count pending))) + (is (= "tool-req-7" (get-in (first pending) [:params :requestId]))) + ;; String result is normalized through normalize-tool-result + (is (= "MANUAL_STATUS_OK" + (get-in (first pending) [:params :result :textResultForLlm])))))) + + (testing "handle-pending-tool-call! with :error forwards error string" + (let [requests (atom []) + _ (mock/set-request-hook! *mock-server* + (fn [method params] + (swap! requests conj {:method method :params params}))) + session (sdk/create-session *test-client* {})] + (sdk/handle-pending-tool-call! session + {:request-id "tool-req-8" + :error "manual failure"}) + (let [pending (first (filter #(= "session.tools.handlePendingToolCall" (:method %)) + @requests))] + (is (= "manual failure" (get-in pending [:params :error]))) + (is (not (contains? (:params pending) :result)))))) + + (testing "handle-pending-tool-call! rejects :result and :error together" + (let [session (sdk/create-session *test-client* {})] + (is (thrown? Exception + (sdk/handle-pending-tool-call! + session + {:request-id "x" :result "a" :error "b"}))))) + + (testing "handle-pending-tool-call! rejects non-string :error" + (let [session (sdk/create-session *test-client* {})] + (is (thrown-with-msg? Exception #":error must be a string" + (sdk/handle-pending-tool-call! + session + {:request-id "x" :error {:code 1}}))) + (is (thrown-with-msg? Exception #":error must be a string" + (sdk/handle-pending-tool-call! + session + {:request-id "x" :error 42}))))) + + (testing "handle-pending-tool-call! requires :result or :error" + (let [session (sdk/create-session *test-client* {})] + (is (thrown-with-msg? Exception #"exactly one of :result or :error" + (sdk/handle-pending-tool-call! + session + {:request-id "x"}))))) + + (testing "