Port upstream v0.1.33: OTel, skipPermission, setModel opts, event specs#58
Port upstream v0.1.33: OTel, skipPermission, setModel opts, event specs#58
Conversation
There was a problem hiding this comment.
Pull request overview
Ports upstream Copilot SDK v0.1.33 features into the Clojure SDK, covering OpenTelemetry configuration + trace propagation, tool permission skipping, updated event/data specs, and deprecation of auto-restart while aligning session/join-session APIs.
Changes:
- Add OTel support via
:telemetryclient option and W3C trace context propagation via:on-get-trace-context. - Add
:skip-permission?on tool definitions and makejoin-session’s:on-permission-requestoptional (defaulting to{:kind :no-result}). - Update session model switching APIs to accept optional
{:reasoning-effort ...}and expand event/data specs (resume/model_change/blob attachments).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/github/copilot_sdk_test.clj | Adjusts unit test expectations for set-model! delegating to switch-model! with the new arity. |
| test/github/copilot_sdk/integration_test.clj | Updates integration tests to assert deprecated auto-restart no longer triggers on disconnect/process-exit. |
| src/github/copilot_sdk/specs.clj | Adds specs for telemetry, :skip-permission?, join-session config, and new/expanded event & attachment specs. |
| src/github/copilot_sdk/session.clj | Extends tool invocation context with trace fields; adds trace context to send!; adds :reasoning-effort opts to model switching. |
| src/github/copilot_sdk/process.clj | Sets OTel-related environment variables when spawning the CLI process with telemetry configured. |
| src/github/copilot_sdk/instrument.clj | Updates function specs for join-session, switch-model!, and set-model! to reflect new optionals. |
| src/github/copilot_sdk/client.clj | Deprecates auto-restart (no-op), adds trace context helper and merges trace context into create/resume params; supports skipPermission in tool wire defs; join-session default permission handler. |
| src/github/copilot_sdk.clj | Updates public API docs and adds optional opts arities for switch-model! / set-model!. |
| CHANGELOG.md | Documents the upstream-sync additions/changes under [Unreleased]. |
…vent specs
Ports 6 upstream PRs from github/copilot-sdk into a single batch:
- **Event data specs** (upstream #796): Add blob attachment type, session.resume
and session.model_change event data specs, new session.start fields
- **skipPermission** (upstream #808): Tool definitions support :skip-permission?
to bypass permission prompts
- **Deprecate autoRestart** (upstream #803): :auto-restart? is now deprecated
and has no effect; maybe-reconnect! is a no-op
- **Optional permission in join-session** (upstream #802): :on-permission-request
is optional in join-session, defaults to returning {:kind :no-result}
optional opts map with :reasoning-effort
- **OpenTelemetry support** (upstream #785): :telemetry client option configures
OTEL env vars on CLI process; :on-get-trace-context injects W3C Trace Context
into session RPCs and tool invocations
Critical review improvements over auto-generated PRs #55 and #57:
- join-session-config reuses resume-session-config-keys (avoids key drift)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cdc7692 to
08cf955
Compare
There was a problem hiding this comment.
Pull request overview
Ports upstream @github/copilot-sdk v0.1.33 features into the Clojure SDK, expanding telemetry + trace propagation support, updating session/tool APIs, and aligning event/spec definitions with upstream schema.
Changes:
- Add OpenTelemetry support via
:telemetry(env var wiring) and:on-get-trace-context(W3C Trace Context propagation into RPCs + tool invocations). - Add
:skip-permission?tool option, makejoin-sessionpermission handler optional (default{:kind :no-result}), and deprecate:auto-restart?(no-op). - Extend specs/event data (new session resume/model_change data, blob attachments) and add reasoning-effort option to
switch-model!/set-model!.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/github/copilot_sdk_test.clj | Adjust set-model alias test to tolerate updated arity. |
| test/github/copilot_sdk/integration_test.clj | Update auto-restart tests to assert deprecated/no-op behavior. |
| src/github/copilot_sdk/specs.clj | Add telemetry/tool/join-session specs; extend attachment + event-data specs. |
| src/github/copilot_sdk/session.clj | Propagate trace context into session.send and tool invocation context; add reasoning-effort to model switch. |
| src/github/copilot_sdk/process.clj | Set OTel-related environment variables when telemetry is configured. |
| src/github/copilot_sdk/instrument.clj | Update fdefs for join-session config and model switch/set optional opts. |
| src/github/copilot_sdk/client.clj | Add trace-context plumbing, skipPermission wiring, join-session default permission handler, and deprecate auto-restart logic. |
| src/github/copilot_sdk.clj | Public API docs updated for telemetry + model switching opts; reflect auto-restart deprecation. |
| CHANGELOG.md | Document upstream v0.1.33 sync additions/changes. |
- Validate map? in get-trace-context to prevent ClassCastException from bad providers
- Separate outbound (::attachment) from inbound (::inbound-attachment) specs;
::blob excluded from send-options to prevent invalid wire payload
- Use s/keys for ::blob-attachment with :req-un [::data ::mime-type] and :opt-un [::display-name]
- Fix join-session docstring to show actual return shape {:kind :no-result}
- Strengthen set-model! test to assert full 3-arity delegation
- Add map? validation to trace-ctx in both send! and send-async* paths
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Ports upstream @github/copilot-sdk v0.1.33 changes into the Clojure SDK, adding OpenTelemetry wiring, new tool/session options, and updated specs to maintain upstream parity.
Changes:
- Add OpenTelemetry support via
:telemetryclient option (process env var wiring) and:on-get-trace-contextpropagation intosession.create/session.resume/session.send. - Add
:skip-permission?for tool definitions and makejoin-session’s:on-permission-requestoptional (defaults to{:kind :no-result}). - Deprecate
:auto-restart?(no-op), addswitch-model!/set-model!optional opts, and extend event-related specs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/github/copilot_sdk/specs.clj |
Adds telemetry/tool/join-session specs and new event/attachment specs (includes issues around blob + ::data). |
src/github/copilot_sdk/client.clj |
Deprecates auto-restart, adds trace-context plumbing into create/resume and tool invocation context. |
src/github/copilot_sdk/session.clj |
Adds trace-context propagation into session.send flows and adds opts to model switching APIs. |
src/github/copilot_sdk/process.clj |
Exports telemetry configuration into CLI process environment variables. |
src/github/copilot_sdk/instrument.clj |
Updates instrumentation specs for join-session and model switching arities/return types. |
src/github/copilot_sdk.clj |
Updates public wrapper docs/signatures for telemetry + model switching opts. |
test/github/copilot_sdk_test.clj |
Tightens delegation test for set-model! → switch-model! optional opts. |
test/github/copilot_sdk/integration_test.clj |
Updates integration tests to reflect deprecated no-op auto-restart behavior. |
CHANGELOG.md |
Documents v0.1.33 sync additions/changes. |
Comments suppressed due to low confidence (1)
src/github/copilot_sdk/session.clj:558
- Same as
send!: trace context is merged directly into thesession.sendparams insend-async*, so an invalid provider map could override required request fields. Consider filtering to:traceparent/:tracestatebefore merging to keep request construction robust.
params (cond-> {:session-id session-id
:prompt (:prompt opts)}
trace-ctx (merge trace-ctx)
wire-attachments (assoc :attachments wire-attachments)
src/github/copilot_sdk/client.clj
Outdated
| is configured or returns a non-map value." | ||
| [provider] | ||
| (if-not provider | ||
| {} | ||
| (try | ||
| (let [ctx (provider)] | ||
| (if (map? ctx) ctx {})) |
There was a problem hiding this comment.
Valid, fixed in a52c7af. get-trace-context now uses (select-keys ctx [:traceparent :tracestate]) to restrict the returned map, preventing any unexpected keys from overriding RPC params.
- Fix ::data spec conflict: blob-attachment uses manual predicates instead of s/keys to avoid redefining ::data (which is a map in ::session-event) - Restrict get-trace-context to select-keys [:traceparent :tracestate] to prevent provider from accidentally overriding RPC params like :session-id - Apply same select-keys restriction in send! and send-async* inline paths - Add ::inbound-attachments spec and use it in ::user.message-data so blob attachments in events pass validation - Remove unused ::inbound-attachment-type spec Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Batch port of upstream github/copilot-sdk v0.1.33 changes into this Clojure SDK, covering OpenTelemetry wiring, permission/extension ergonomics, model-switch options, and updated event/tool specs.
Changes:
- Add OpenTelemetry client options (
:telemetry,:on-get-trace-context) and propagate W3C trace context into key RPCs/tool invocations. - Add
:skip-permission?on tool definitions, makejoin-sessionpermission handler optional, and deprecate:auto-restart?(no-op). - Expand/adjust specs for new event data fields and inbound blob attachments; update tests accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/github/copilot_sdk/specs.clj |
Adds telemetry/tool/session config options and expands event + inbound attachment specs (including blob inbound-only). |
src/github/copilot_sdk/client.clj |
Implements trace-context injection for create/resume, adds tool skipPermission wiring, and removes auto-restart logic (no-op). |
src/github/copilot_sdk/session.clj |
Propagates trace context into session.send paths, passes trace context into tool invocation context, and adds :reasoning-effort opts to model switching. |
src/github/copilot_sdk/process.clj |
Sets OTel-related environment variables when spawning the CLI process. |
src/github/copilot_sdk/instrument.clj |
Updates fdefs for optional join-session permission handler and optional opts on switch-model!/set-model!. |
src/github/copilot_sdk.clj |
Updates public wrapper arities/docs for model switching and documents new telemetry options + auto-restart deprecation. |
test/github/copilot_sdk_test.clj |
Strengthens delegation/arity assertions for set-model! → switch-model!. |
test/github/copilot_sdk/integration_test.clj |
Updates auto-restart integration tests to assert deprecated/no-op behavior. |
CHANGELOG.md |
Documents v0.1.33 sync additions/changes (OTel, skip-permission, join-session change, auto-restart deprecation, event/spec updates). |
Comments suppressed due to low confidence (1)
src/github/copilot_sdk/client.clj:438
watch-process-exit!docstring still says it "Trigger[s] auto-restart" butmaybe-reconnect!is now an explicit no-op (auto-restart deprecated). Updating this docstring (and any related wording) would prevent confusion about whether a process exit should still cause reconnection behavior.
(defn- watch-process-exit!
"Trigger auto-restart when the managed CLI process exits."
[client mp]
Document features merged in PR #58 (upstream v0.1.33 sync): - Mark :auto-restart? as deprecated in create-client options - Add :telemetry and :on-get-trace-context client options (OTel support) - Note :on-permission-request is optional in create-session and join-session - Update join-session docs with example showing optional permission handler - Add :reasoning-effort opts to switch-model! and set-model! - Document :skip-permission? on tool definitions - Update event reference with data fields for session.start, session.resume, session.model_change events - Add :blob attachment type to send! attachments table Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Batch port of 6 upstream PRs from
github/copilot-sdkinto a single cohesive change.Supersedes auto-generated PRs #55 and #57 with critical review improvements.
Upstream PRs Ported
:skip-permission?on tool definitions:auto-restart?(no-op):on-permission-requestinjoin-session:reasoning-effortinswitch-model!/set-model!:telemetry,:on-get-trace-context)Critical Review Improvements over #55 / #57
join-session-configreusesresume-session-config-keys— PR [upstream-sync] Port upstream PRs #808, #803, #802: skipPermission, deprecate autoRestart, optional join-session permission handler #57 duplicated all key names, risking driftmark-restarting!and:restarting?from initial-state cleaned up after auto-restart deprecationswitch-model!/set-model!with optional opts parameterCode Review Fixes (GPT-5.4 + Opus 4.6 + Copilot Code Review)
get-trace-contextvalidatesmap?to preventClassCastExceptionfrom bad providers::attachmentfrom inbound::inbound-attachment— blob excluded from::send-options::blob-attachmentusess/keyswith proper:req-un/:opt-unincluding:display-name:skip-permission?usessome?+ pass-through value (matchesoverrides-built-in-toolpattern)send-async*):no-resultadded to::permission-result-kindspecNot Ported (and why)
Testing