Conversation
Upstream commits ported (10 total, 6 PORT / 4 SKIP): PORT: - PR #1049: convert-mcp-call-tool-result function (tools.clj) - PR #1051: MCP config type renames Local→Stdio, Remote→HTTP (specs.clj) - PR #1056: requestPermission false on resume with default handler (client.clj) - PR #995: Per-agent skills field on CustomAgentConfig (specs.clj) - PR #1055 (CLI 1.0.22): New RPCs (mcp.discover, usage.getMetrics) + memory event specs - PR #1076 (CLI 1.0.26): session.name.get/set + workspaces.getWorkspace RPCs SKIP: - PR #1048: Export ProviderConfig type (already have spec) - PR #1046: Clear CLI startup timeout on stop (Clojure handles cleanup) - PR #1043: Codegen naming improvements (internal) - PR #1062: Codegen $ref support (codegen only) Changes: - New public function: convert-mcp-call-tool-result in tools namespace - New permission handler: default-join-session-permission-handler - resume-session now sends requestPermission:false with default handler - join-session uses default-join-session-permission-handler (not anonymous fn) - MCP spec aliases: ::mcp-stdio-server, ::mcp-http-server (backward compat) - Per-agent ::agent-skills field on ::custom-agent spec - Memory permission fields on ::permission-request spec - 5 new experimental RPC wrappers in session namespace - 18 new integration tests, all fdefs in instrument.clj - Updated API reference docs, CHANGELOG, and codox HTML Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Ports 6 post-v0.2.2 upstream commits into the Clojure Copilot SDK, adding MCP tool-result conversion, permission-handling parity for resume/join flows, and new experimental session RPC wrappers/spec updates.
Changes:
- Add
convert-mcp-call-tool-resultand re-export it from the publicgithub.copilot-sdknamespace. - Add
default-join-session-permission-handlerand updateresume-sessionwire behavior to sendrequestPermission: falsewhen using it. - Add/rename specs (MCP stdio/http aliases, per-agent skills, memory permission fields) plus experimental session RPC wrappers and corresponding tests/docs.
Show a summary per file
| File | Description |
|---|---|
| test/github/copilot_sdk/mock_server.clj | Adds default mock responses for new RPC method names used by tests. |
| test/github/copilot_sdk/integration_test.clj | Adds integration coverage for MCP conversion, spec changes, requestPermission behavior, and new RPC wrappers. |
| src/github/copilot_sdk/tools.clj | Implements convert-mcp-call-tool-result. |
| src/github/copilot_sdk/specs.clj | Adds MCP alias specs, ::agent-skills, and memory permission fields on ::permission-request. |
| src/github/copilot_sdk/session.clj | Adds experimental wrappers for session name/workspace/mcp discovery/usage metrics. |
| src/github/copilot_sdk/instrument.clj | Adds fdefs + instrumentation wiring for the new public/experimental functions. |
| src/github/copilot_sdk/client.clj | Adds default-join-session-permission-handler and updates resume param building for requestPermission. |
| src/github/copilot_sdk.clj | Re-exports new public API vars. |
| doc/reference/API.md | Documents new APIs, behavioral change, and new experimental wrappers/specs. |
| doc/api/github.copilot-sdk.tools.html | Regenerated API docs including the new tools var. |
| doc/api/github.copilot-sdk.session.html | Regenerated API docs including new experimental session wrappers. |
| doc/api/github.copilot-sdk.html | Regenerated top-level API docs including new re-exports. |
| doc/api/github.copilot-sdk.client.html | Regenerated client API docs including default join permission handler. |
| doc/api/API.html | Regenerated rendered API reference HTML with new sections/fields. |
| CHANGELOG.md | Adds Unreleased entries describing the sync additions/changes. |
Copilot's findings
Comments suppressed due to low confidence (4)
src/github/copilot_sdk/session.clj:1313
- Same as above: this response is already wire-normalized by
protocol/normalize-incomingbeforesend-request!returns it, so the extrautil/wire->cljcall is redundant.
(let [{:keys [session-id client]} session
conn (connection-io client)]
(util/wire->clj
(proto/send-request! conn "session.name.set"
{:session-id session-id :name name}))))
src/github/copilot_sdk/session.clj:1324
- This
util/wire->cljwrapping is redundant becauseproto/send-request!returns a result from a message that has already been run throughutil/wire->cljin the protocol layer. Returningproto/send-request!directly would avoid double conversion.
(let [{:keys [session-id client]} session
conn (connection-io client)]
(util/wire->clj
(proto/send-request! conn "session.workspaces.getWorkspace"
{:session-id session-id}))))
src/github/copilot_sdk/session.clj:1339
- Redundant double conversion:
proto/send-request!returns normalized results already, soutil/wire->cljis unnecessary here. Dropping it would reduce overhead (and match the fact that params are already converted inprotocol/send-request).
(let [{:keys [session-id client]} session
conn (connection-io client)]
(util/wire->clj
(proto/send-request! conn "mcp.discover"
(cond-> {}
(:working-directory opts)
(assoc :working-directory (:working-directory opts))))))))
src/github/copilot_sdk/session.clj:1350
- Same redundancy: results from
proto/send-request!are already in Clojure key format; the extrautil/wire->cljconversion is unnecessary.
(let [{:keys [session-id client]} session
conn (connection-io client)]
(util/wire->clj
(proto/send-request! conn "session.usage.getMetrics"
{:session-id session-id}))))
- Files reviewed: 15/15 changed files
- Comments generated: 2
| ;; convert-mcp-call-tool-result function spec (upstream PR #1049) | ||
| (s/fdef github.copilot-sdk.tools/convert-mcp-call-tool-result | ||
| :args (s/cat :call-result map?) | ||
| :ret ::specs/tool-result-object) |
There was a problem hiding this comment.
The return spec for convert-mcp-call-tool-result is ::specs/tool-result-object, but ::tool-result-object references ::binary-results-for-llm which currently has no s/def in specs.clj. Defining ::binary-results-for-llm (and the per-item shape) would make instrumentation actually validate the new binary result output instead of effectively treating it as any?.
There was a problem hiding this comment.
Valid finding. Added ::binary-results-for-llm spec definition ((s/coll-of map?)) so the ::tool-result-object spec now validates this field. Used map? for items because the binary result item shape (:data, :type, :mime-type) conflicts with the existing ::type spec (which is ::attachment-type / #{:image :text}). A more precise item spec would require namespace-qualifying these keys to avoid the conflict — tracked as a follow-up improvement.
| (let [{:keys [session-id client]} session | ||
| conn (connection-io client)] | ||
| (util/wire->clj | ||
| (proto/send-request! conn "session.name.get" {:session-id session-id})))) |
There was a problem hiding this comment.
proto/send-request! already returns a result from a normalized (wire->clj) incoming message, so wrapping it in util/wire->clj here is redundant. Consider returning the result directly to avoid double key-conversion overhead and potential subtle key munging.
This issue also appears in the following locations of the same file:
- line 1309
- line 1320
- line 1333
- line 1346
There was a problem hiding this comment.
Valid observation, but this is a pre-existing pattern — all 15+ existing RPC wrappers in session.clj use the same (util/wire->clj (proto/send-request! ...)) pattern. Since wire->clj is idempotent (kebab keys stay kebab), it's harmless but redundant. Removing it only from the new functions would create an inconsistency. I'll track removing the redundant wire->clj calls across all session RPC wrappers as a separate cleanup PR.
Address Copilot Code Review feedback: the ::tool-result-object spec referenced ::binary-results-for-llm but it had no s/def. Added as (s/coll-of map?) — item-level spec uses map? to avoid conflicts with the existing ::type spec (::attachment-type). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Ports 6 unreleased upstream commits from
github/copilot-sdk(post-v0.2.2 tag) to the Clojure SDK.Upstream Commits Ported
convertMcpCallToolResultfunctionrequestPermission: falseon resume with default handlerskillsfield on CustomAgentConfigSkipped: #1048 (already have spec), #1046 (Clojure handles cleanup), #1043/#1062 (codegen only).
Changes
New Public APIs
convert-mcp-call-tool-result— Converts MCP CallToolResult to SDK ToolResultObject (text, image, resource content types)default-join-session-permission-handler— Returns{:kind :no-result}, sendsrequestPermission: falseon resumeBehavioral Changes
resume-sessionnow sendsrequestPermission: falsewhen usingdefault-join-session-permission-handler,truefor any other handler (e.g.approve-all)join-sessionupdated to usedefault-join-session-permission-handler(was anonymous fn)New Specs
::mcp-stdio-server/::mcp-http-server(backward-compat aliases)::agent-skillson::custom-agent::memory-action,::memory-direction,::memory-reasonon::permission-requestNew Experimental RPC Wrappers
Validation
run-all-examples.sh)bb validate-docs)Multi-Model Code Review
join-sessionused anonymous fn instead ofdefault-join-session-permission-handler::permission-request