feat: forward GitHub telemetry (upstream #1835) + schema 1.0.68 sync#145
Merged
Conversation
Sync the pinned @github/copilot schema to 1.0.68. The upstream bump adds metadata context-attribution / heaviest-messages RPC methods and slash-command input choices to generated rpc.ts only; none are surfaced on the public Node SDK, so no hand-written Clojure API changes are needed. Regenerated wire specs and coercions produced no diff (session events unchanged). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ream #1835) Add the experimental/internal :on-github-telemetry client callback. When (and only when) a handler is registered, the client stamps enableGitHubTelemetryForwarding: true onto the wire params of both session.create and session.resume; the flag is omitted entirely otherwise. The notification router gains a "gitHubTelemetry.event" branch that invokes the callback with the normalized notification params, wrapped in catch Throwable so a throwing consumer cannot unwind the go-loop and kill dispatch for all sessions. The runtime's three source-defined sub-maps (:properties, :metrics, :features) are opaque string maps whose keys must survive verbatim, so an escape hatch in protocol.clj re-assocs the raw pre-conversion values after wire->clj kebab-cases the rest of the event. Hand-curated idiom specs document the notification/event/client-info shapes as open maps so future upstream fields pass through. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add three integration tests exercising the :on-github-telemetry callback, the enableGitHubTelemetryForwarding wire flag on create and resume, and the opaque passthrough of :properties/:metrics/:features. Add a regression test asserting that a handler throwing a non-Exception Throwable does not kill the notification router. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the :on-github-telemetry client-option row and a GitHub telemetry forwarding subsection under Observability in the API reference, covering the callback signature, opt-in semantics, notification/event shapes, and the opaque-passthrough note for :properties/:metrics/:features. Record the sync in the changelog citing upstream #1835 and the schema 1.0.68 regen (#1886). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sync the version to the 1.0.6-preview.1 upstream pre-release (the sync point for #1835/#1886) with Clojure patch 0, and teach the build validators to accept preview qualifiers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Syncs the Clojure Copilot SDK with upstream changes by adding opt-in GitHub telemetry forwarding (experimental/internal) and bumping the pinned schema package version to 1.0.68, with accompanying tests and documentation updates.
Changes:
- Added client option
:on-github-telemetryand routing forgitHubTelemetry.eventnotifications, including an escape hatch to preserve opaque sub-maps verbatim. - Stamped
enableGitHubTelemetryForwarding: trueontosession.createandsession.resumewhen the handler is registered, plus integration tests covering wire params and callback behavior. - Synced schema pin to
1.0.68and updated changelog/docs/version strings accordingly.
Show a summary per file
| File | Description |
|---|---|
| test/github/copilot_sdk/integration_test.clj | Adds integration tests for telemetry opt-in flagging and notification callback behavior (including Throwable safety). |
| src/github/copilot_sdk/specs.clj | Introduces specs for :on-github-telemetry and idiom-shaped telemetry notification/event maps (open maps). |
| src/github/copilot_sdk/protocol.clj | Adds protocol normalization escape hatch to preserve telemetry opaque sub-maps’ keys verbatim. |
| src/github/copilot_sdk/client.clj | Wires client option, enables forwarding flag on create/resume, and dispatches telemetry notifications to the callback with Throwable protection. |
| schemas/README.md | Updates documented pinned schema version to 1.0.68. |
| schemas/api.schema.json | Regenerated schema content for 1.0.68. |
| README.md | Bumps example dependency version to 1.0.6-preview.1.0. |
| doc/reference/API.md | Documents the new :on-github-telemetry option and describes the notification/event shapes. |
| doc/getting-started.md | Updates dependency snippet version to 1.0.6-preview.1.0. |
| CHANGELOG.md | Adds Unreleased entry describing telemetry forwarding + schema 1.0.68 sync. |
| build.clj | Updates project version and expands upstream version regex/error message to include preview. |
| .copilot-schema-version | Bumps pinned schema version to 1.0.68. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Low
…y true The session.create/resume wire builders gated the forwarding flag on `some?`, which would leak `enableGitHubTelemetryForwarding: false` onto the wire if the config ever carried an explicit `false`. The documented contract (mirroring upstream #1835) is to omit the field entirely and never send `false`. Harden both builders to emit only for an explicit `true` config value; `false`/`nil` omit the key. Adds a direct-builder regression test covering true/false/absent for both the create and resume paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Syncs the Clojure SDK with two upstream
github/copilot-sdkchanges: the experimental GitHub telemetry forwarding callback (#1835) and the schema pin bump to1.0.68(#1886).GitHub telemetry forwarding (#1835)
Adds the
:on-github-telemetryclient option — a one-arg callback marked @experimental / Internal, mirroring the Node/Python SDKs'onGitHubTelemetry. It is opt-in and off by default.enableGitHubTelemetryForwarding: trueis stamped onto bothsession.createandsession.resumeparams, but only when a handler is registered. The flag is omitted entirely otherwise — we never sendfalse.gitHubTelemetry.eventnotifications (client-global, not per-session); each invokes the callback.Non-obvious bits a reviewer might wonder about:
catch Throwable, notcatch Exception. The callback runs inside the shared notification go-loop. A consumer throwing a non-ExceptionThrowable(e.g.AssertionError) would otherwise unwind the loop and permanently kill notification dispatch for all sessions. This matches the ~10 other critical-path catch sites. A RED→GREEN regression test pins the behavior.:properties,:metrics, and:featuressub-maps are source-defined string maps whose keys must survive verbatim.protocol.cljre-assocs the raw pre-conversion values afterwire->cljkebab-cases the rest of the event — the same escape-hatch pattern used forsession.event/ MCP args / SQL bind params.:enableGitHubTelemetryForwardingbecause camel-snake-kebab would lowercase theH(enableGithubTelemetryForwarding).s/keysfor documentation only; they are never validated against incoming notifications, so future upstream fields pass through untouched.Schema 1.0.68 (#1886)
Schema-regen only. The upstream bump adds metadata context-attribution / heaviest-messages RPC methods and slash-command input choices to generated
rpc.ts, but none are surfaced on the public Node SDK — so per the API-parity rule no hand-written Clojure API is added. Regenerated wire specs / coercions produced no diff (session events unchanged).Deliberate non-goals
session.lifecyclenotification handler has the same pre-existingcatch Exceptionpattern; it is out of scope for this PR and tracked separately.Validation
bb validate-docs: 17 files, 0 warnings.bb ci:full: jar build ✓,run-all-examples.shexit 0. (The E2Esession.idletimeout is a known benign flake.)Multi-model review (Phase 6)
catch Exception; a handler throwing a non-ExceptionThrowableescapes, unwinds the notification go-loop, and permanently kills dispatch for ALL sessionscatch Exception→catch Throwable. RED→GREEN regression test added.normalize-config-for-modedocstring's final line is stale — steps 1 and 3 fire in every mode; only step 2 is:empty-only:event/:clientdescribed as nested but not wired into:opt-un)::eventand wired it in.::clientcollides with the client-record spec keyword; left as open passthrough, comments made honest.