Skip to content

Commit 431f52d

Browse files
krukowCopilot
andcommitted
fix: address round 2 code review feedback
- Sanitize ex-info data in is-child-process? validations to avoid leaking :github-token (only include relevant keys) - Flush wrapped OutputStream on close instead of pure no-op, ensuring buffered bytes are sent before disconnect - Tighten log! return spec from nilable to required ::event-id since the RPC contract guarantees eventId Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 0bda4f4 commit 431f52d

2 files changed

Lines changed: 9 additions & 6 deletions

File tree

src/github/copilot_sdk/client.clj

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,12 @@
137137
:use-logged-in-user? (:use-logged-in-user? opts)})))
138138
;; Validation: is-child-process? is mutually exclusive with cli-url
139139
(when (and (:is-child-process? opts) (:cli-url opts))
140-
(throw (ex-info "is-child-process? is mutually exclusive with cli-url" opts)))
140+
(throw (ex-info "is-child-process? is mutually exclusive with cli-url"
141+
{:is-child-process? true :cli-url (:cli-url opts)})))
141142
;; Validation: is-child-process? requires stdio transport
142143
(when (and (:is-child-process? opts) (= false (:use-stdio? opts)))
143-
(throw (ex-info "is-child-process? requires use-stdio? to be true (or unset)" opts)))
144+
(throw (ex-info "is-child-process? requires use-stdio? to be true (or unset)"
145+
{:is-child-process? true :use-stdio? false})))
144146
(when-not (s/valid? ::specs/client-options opts)
145147
(let [unknown (specs/unknown-keys opts specs/client-options-keys)
146148
explain (s/explain-data ::specs/client-options opts)
@@ -534,11 +536,12 @@
534536
(close [] nil)))
535537

536538
(defn- non-closing-output-stream
537-
"Wrap an OutputStream so that .close is a no-op.
538-
Prevents proto/disconnect from closing System/out."
539+
"Wrap an OutputStream so that .close flushes but does not close.
540+
Prevents proto/disconnect from closing System/out while ensuring
541+
buffered bytes are sent."
539542
^java.io.OutputStream [^java.io.OutputStream out]
540543
(proxy [java.io.FilterOutputStream] [out]
541-
(close [] nil)))
544+
(close [] (.flush ^java.io.OutputStream out))))
542545

543546
(defn- connect-parent-stdio!
544547
"Connect via own stdio to a parent Copilot CLI process (child process mode).

src/github/copilot_sdk/instrument.clj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@
225225
:args (s/cat :session ::specs/session
226226
:message string?
227227
:opts (s/? (s/nilable ::specs/log-options)))
228-
:ret (s/nilable ::specs/event-id))
228+
:ret ::specs/event-id)
229229

230230
;; -----------------------------------------------------------------------------
231231
;; Function specs for helpers namespace

0 commit comments

Comments
 (0)