Port upstream PRs #664, #737: session pre-registration, on-event, join-session by krukow · Pull Request #52 · copilot-community-sdk/copilot-sdk-clojure

krukow · 2026-03-12T10:52:19Z

Summary

Ports upstream changes from PR #664 and PR #737 to the Clojure SDK.

Session Pre-Registration (upstream PR #664)

Problem: Events emitted during session.create RPC (e.g. session.start) were dropped because the session was only registered in client state after the RPC completed.

Fix: Sessions are now registered before the RPC call. Session IDs are generated client-side (UUID) when not provided. On RPC failure, pre-registered state is cleaned up automatically.

`:on-event` Handler (upstream PR #664)

New optional config for create-session and resume-session:

(copilot/create-session client
  {:on-permission-request copilot/approve-all
   :on-event (fn [event]
               (println "Event:" (:type event)))})
;; Guaranteed to receive session.start and other early events

Runs on async/thread (not go-loop) so user handlers can safely perform blocking I/O.

`join-session` (upstream PR #737)

Convenience function for extensions running as child processes of the Copilot CLI:

(let [{:keys [client session]} (copilot/join-session
                                 {:on-permission-request copilot/approve-all})]
  ;; session is already connected to parent CLI
  (copilot/send-and-wait! session {:prompt "Hello"})
  (copilot/stop! client))

`:copilot/system.notification` Event (upstream PR #737)

New event type with :kind discriminator (agent_completed, shell_completed, shell_detached_completed).

Robustness Fixes (from code review)

Async session functions (<create-session, <resume-session) clean up pre-registered sessions when RPC channel closes without a value
join-session cleans up client on resume failure (prevents resource leaks)
on-event handler runs on async/thread instead of go-loop to avoid starving the core.async dispatch pool

Upstream PRs Reviewed but Not Ported

PR	Reason
#787	`cliPath`/`cliUrl` mutual exclusion — our validation already matches upstream behavior
20 other commits	C#/.NET-specific, docs-only, or CI/workflow changes

Documentation

README.md: expanded examples table (5 → 17), added new features
API.md: documented :on-event, join-session, system.notification, evt, client-info
All other docs updated (getting-started, index, examples/README, CHANGELOG)

Testing

✅ 93 tests, 317 assertions, 0 failures (unit + integration + E2E)
✅ All 15 examples pass
✅ Doc validation: 12 files, 0 warnings
✅ Dual code review (Claude Opus 4.6 + GPT-5.4) — all findings addressed

…n-session Session pre-registration (upstream PR #664): - Sessions are now created and registered in client state BEFORE the session.create/session.resume RPC call, preventing early events (e.g. session.start) from being dropped - Session IDs are generated client-side via java.util.UUID/randomUUID when not explicitly provided - On RPC failure, pre-registered sessions are automatically cleaned up - CopilotSession record no longer includes workspace-path as a field; use (workspace-path session) accessor which reads from mutable state :on-event handler (upstream PR #664): - New optional :on-event config option for create-session and resume-session — a 1-arity function receiving event maps - Registered before the RPC call, guaranteeing early events like session.start are not missed - Runs on async/thread (not go-loop) to avoid blocking the core.async dispatch thread pool with user handlers join-session (upstream PR #737): - New convenience function for extensions running as child processes - Reads SESSION_ID from environment, creates child-process client, resumes session with :disable-resume? true - Returns {:client c :session s}; cleans up client on failure system.notification event (upstream PR #737): - Added :copilot/system.notification event type with :kind discriminator (agent_completed, shell_completed, shell_detached_completed) Async robustness fixes (from code review): - Async session functions (<create-session, <resume-session) now clean up pre-registered sessions when RPC channel closes without a value - join-session wraps resume-session in try/catch, calls stop! on the client if resume fails to prevent resource leaks Documentation: - Updated README.md examples table (5 → 17 examples), added new features to key features list - Updated doc/reference/API.md with :on-event, join-session, system.notification, evt, client-info - Updated doc/index.md example count (11 → 17) - Updated doc/getting-started.md Key Concepts table - Updated examples/README.md with ask_user_failure walkthrough - Updated CHANGELOG.md [Unreleased] section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Ports upstream Copilot SDK changes into the Clojure implementation to prevent early session events from being dropped, add an :on-event callback, and introduce join-session for child-process extensions.

Changes:

Pre-register sessions before session.create / session.resume RPCs and generate UUID session IDs client-side when not provided.
Add :on-event session config and wire it through session creation/resume so early events (e.g. :copilot/session.start) can be observed.
Add join-session plus new :copilot/system.notification event type, with broad docs/example updates.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/github/copilot_sdk/integration_test.clj	Updates integration coverage for UUID session IDs, custom session IDs, and `:on-event` delivery.
src/github/copilot_sdk/specs.clj	Adds `:on-event` to session/resume configs and extends `::event-type` with `:copilot/system.notification`.
src/github/copilot_sdk/session.clj	Removes `workspace-path` from `CopilotSession` record fields, adds `:on-event` forwarding, and adds workspace-path/state helpers for pre-registration flow.
src/github/copilot_sdk/instrument.clj	Adds instrumentation for `join-session` and includes it in (un)instrument lists.
src/github/copilot_sdk/client.clj	Implements session pre-registration + cleanup on failure, client-side session-id generation, async cleanup on closed RPC channels, and adds `join-session`.
src/github/copilot_sdk.clj	Exposes `join-session` in the public API and adds `:copilot/system.notification` to the public event type set.
examples/README.md	Adds the “Ask user failure” example and expands example documentation.
doc/reference/API.md	Documents `:on-event`, `join-session`, `evt`, `client-info`, and `system.notification`.
doc/index.md	Updates the examples count and link description.
doc/getting-started.md	Adds “On-Event” to the conceptual overview table.
README.md	Updates feature list and expands the examples table and run commands.
CHANGELOG.md	Adds Unreleased entries describing pre-registration, `:on-event`, `join-session`, event type changes, and the `CopilotSession` record change.

Copilot · 2026-03-12T10:57:37Z

src/github/copilot_sdk/session.clj

+    ;; If an on-event handler is provided, tap and forward events to it.
+    ;; Uses async/thread to avoid blocking core.async dispatch threads,
+    ;; since user handlers may perform blocking I/O.
+    (when on-event
+      (let [handler-ch (chan (async/sliding-buffer 1024))]
+        (tap event-mult handler-ch)


The :on-event handler buffer uses sliding-buffer, which will silently drop events if the handler can’t keep up. That contradicts the docstring wording that the handler is invoked “for each event”, and makes drops harder to diagnose. Consider either documenting the drop behavior explicitly (similar to subscribe-events), or using a strategy that preserves backpressure/observability (e.g., non-sliding buffer + explicit timeout/drop logging).

Copilot · 2026-03-12T10:57:38Z

src/github/copilot_sdk/session.clj

+                (try
+                  (on-event event)
+                  (catch Throwable t
+                    (log/warn "on-event handler threw: " (ex-message t))))


When the :on-event handler throws, the log line only includes (ex-message t), which drops the stack trace and the throwable itself. Logging the throwable (and ideally session-id / event type) would make debugging user handler failures significantly easier.

Suggested change

(log/warn "on-event handler threw: " (ex-message t))))

(log/warn t

"on-event handler threw"

{:session-id session-id

:event-type (:type event)})))

Copilot · 2026-03-12T10:57:38Z

src/github/copilot_sdk/specs.clj

 (s/def ::disable-resume? boolean?)

+;; Event handler (1-arity fn receiving event map)
+(s/def ::on-event fn?)


::on-event is documented here as a “1-arity fn receiving event map”, but the spec is only fn?, so spec instrumentation won’t catch arity mismatches (which will fail later at runtime with an ArityException). Either tighten the spec to validate the expected arity/shape, or relax the comment/documentation so it matches what is actually enforced.

Suggested change

(s/def ::on-event fn?)

(s/def ::on-event

(s/fspec

:args (s/cat :event map?)

:ret any?))

Good suggestion, but all handler specs in this codebase (::on-permission-request, ::on-user-input-request) use fn? rather than fspec. Using fspec here alone would be inconsistent. Updated the comment to clarify this design choice. If we tighten handler specs in the future, we should do it across the board.

Copilot · 2026-03-12T10:57:38Z

test/github/copilot_sdk/integration_test.clj

+          session (sdk/create-session *test-client*
+                                      {:on-permission-request sdk/approve-all
+                                       :on-event (fn [evt] (swap! events conj evt))})]
+      ;; Give the on-event handler time to receive the session.start event
+      (Thread/sleep 200)


This assertion depends on a fixed Thread/sleep to give the :on-event thread time to run, making the test timing-dependent and potentially flaky on slower CI machines. Prefer waiting deterministically (e.g., have the handler signal via a promise/chan and then wait with a timeout) so the test is reliable and fails fast if the event never arrives.

Suggested change

session (sdk/create-session *test-client*

{:on-permission-request sdk/approve-all

:on-event (fn [evt] (swap! events conj evt))})]

;; Give the on-event handler time to receive the session.start event

(Thread/sleep 200)

event-ch (chan 1)

session (sdk/create-session *test-client*

{:on-permission-request sdk/approve-all

:on-event (fn [evt]

(swap! events conj evt)

(when (= :copilot/session.start (:type evt))

(async/put! event-ch true)))})

[v ch] (alts!! [event-ch (timeout 1000)])]

(is (= event-ch ch)

"on-event handler should receive session.start event within timeout")

(is v)

- Document sliding-buffer drop behavior in on-event handler comment - Improve on-event error logging: include throwable, session-id, event-type - Replace Thread/sleep in on-event test with promise + deref timeout - Clarify ::on-event spec comment re: fn? consistency with other handlers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-12T11:33:35Z

src/github/copilot_sdk/session.clj

-   Initializes session state in client's atom and returns a CopilotSession handle."
-  [client session-id {:keys [tools on-permission-request on-user-input-request hooks workspace-path config]}]
+   Initializes session state in client's atom and returns a CopilotSession handle.
+   If :on-event is provided, taps a subscriber that invokes the handler for each event."


The create-session docstring says the :on-event handler is invoked “for each event”, but the implementation uses a tapped channel with a sliding buffer, so events may be dropped if the handler can’t keep up. Consider rewording the docstring to reflect the best-effort / may-drop delivery semantics so expectations match runtime behavior.

Suggested change

If :on-event is provided, taps a subscriber that invokes the handler for each event."

If :on-event is provided, taps a subscriber that forwards events to the handler on a best-effort basis;

events may be dropped if the handler cannot keep up with the event stream."

Address Copilot review: docstring now mentions best-effort delivery and that events may be dropped if handler cannot keep up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-12T11:40:44Z

src/github/copilot_sdk/session.clj

 (defn dispatch-event!
  "Dispatch an event to all subscribers via the mult. Called by client notification router.


dispatch-event! docstring says events are dropped “with warning” when the session event buffer is full, but the session event channel is created with a sliding buffer, so “buffer full” isn’t really a detectable condition (oldest events are silently dropped instead). As a result, the offer!/warning logic in this function will only trigger when the channel is closed, not when it is saturated. Consider updating the docstring/logging to reflect the actual drop/closed-channel behavior, or switch the event channel to a fixed buffer if you want buffer-full drop detection.

Copilot AI review requested due to automatic review settings March 12, 2026 10:52

Copilot started reviewing on behalf of krukow March 12, 2026 10:52 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

krukow requested a review from Copilot March 12, 2026 11:29

Copilot started reviewing on behalf of krukow March 12, 2026 11:30 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Update create-session docstring to reflect sliding-buffer drop semantics

9063a97

Address Copilot review: docstring now mentions best-effort delivery and that events may be dropped if handler cannot keep up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

krukow requested a review from Copilot March 12, 2026 11:35

Copilot started reviewing on behalf of krukow March 12, 2026 11:36 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

krukow merged commit 11154f0 into main Mar 12, 2026
5 checks passed

krukow deleted the upstream-sync/v0.1.32-pr664-pr737 branch March 12, 2026 11:46

github-actions bot mentioned this pull request Mar 12, 2026

[aw] No-Op Runs #43

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port upstream PRs #664, #737: session pre-registration, on-event, join-session#52

Port upstream PRs #664, #737: session pre-registration, on-event, join-session#52
krukow merged 3 commits intomainfrom
upstream-sync/v0.1.32-pr664-pr737

krukow commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

krukow Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-                    (log/warn "on-event handler threw: " (ex-message t))))
+                    (log/warn t
+                              "on-event handler threw"
+                              {:session-id session-id
+                               :event-type (:type event)})))

-(s/def ::on-event fn?)
+(s/def ::on-event
+  (s/fspec
+    :args (s/cat :event map?)
+    :ret any?))

	If :on-event is provided, taps a subscriber that invokes the handler for each event."
	If :on-event is provided, taps a subscriber that forwards events to the handler on a best-effort basis;
	events may be dropped if the handler cannot keep up with the event stream."

		(defn dispatch-event!
		"Dispatch an event to all subscribers via the mult. Called by client notification router.

Conversation

krukow commented Mar 12, 2026

Summary

Session Pre-Registration (upstream PR #664)

:on-event Handler (upstream PR #664)

join-session (upstream PR #737)

:copilot/system.notification Event (upstream PR #737)

Robustness Fixes (from code review)

Upstream PRs Reviewed but Not Ported

Documentation

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

krukow Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`:on-event` Handler (upstream PR #664)

`join-session` (upstream PR #737)

`:copilot/system.notification` Event (upstream PR #737)