Upstream sync: post-v1.0.0-beta.4 round 2 (#1299, #1315) + schema 1.0.49#108
Merged
Conversation
Ports the SessionFs SQLite support (upstream PR #1299) and bumps the
pinned schema version from 1.0.49-1 to 1.0.49.
### SessionFs SQLite (upstream PR #1299)
- New optional `:sqlite {:query :exists}` sub-provider for the
provider-style `createSessionFsHandler` factory. Low-level handlers
expose flat `:sqlite-query` / `:sqlite-exists` keys; the adapter
bridges between them.
- Clients advertise support via `:capabilities {:sqlite true}` under
`:session-fs`; the value is forwarded on `sessionFs.setProvider` and
validated at session creation (throws when capabilities.sqlite is
declared but the provider lacks :sqlite).
- `queryType` is coerced from the wire string to an idiomatic
keyword (`#{:exec :query :run}`) before reaching the handler.
- SQL bind-parameter map keys (e.g. `$userId`) bypass kebab-case
conversion via a new escape hatch in `protocol/normalize-incoming`.
- Result row column-name keys (e.g. `:user_id`, `:created_at`)
round-trip verbatim on the outgoing wire path via a new
`preserve-outgoing-opaque-fields` escape hatch in `protocol/`,
matching upstream Node.js semantics where provider rows are forwarded
untouched. (review feedback)
- SQLite handler exceptions propagate as JSON-RPC errors (not wrapped
as SessionFsError result maps), matching upstream Node behavior.
### Schema 1.0.49
- `.copilot-schema-version` advanced from `1.0.49-1` to `1.0.49`
- Additive only: new named enum types, format annotations, and the
new sessionFs.sqliteQuery / sessionFs.sqliteExists RPC methods
- Regenerated event_specs.clj reflects the new enums
- (upstream PRs #1305, #1307, #1327, #1333)
### Other
- Updated `examples/permission_bash.clj` from deprecated `:approved`
to current `:approve-once` (carried from upstream PR #1315)
- Added integration tests covering the SQLite adapter, RPC dispatch,
capability forwarding, validation throw, opaque param keys, and
the new snake_case column-name round-trip regression test
### Validation
- bb test: 269 tests / 1275 assertions pass
- bb validate-docs: 0 warnings
- ./run-all-examples.sh: all examples pass
- E2E: 2 pre-existing failures (test-e2e-connection ping timestamp,
test-e2e-blob-attachment timeout) due to local CLI 1.0.51-2 drift
from the pinned 1.0.49 schema; unrelated to these changes
### Multi-model code review
Parallel reviews via Claude Opus 4.7 and GPT-5.5 both flagged the same
HIGH severity bug: outgoing SQL row column names were being mangled by
recursive kebab→camelCase conversion (`util/clj->wire`). Fixed by
adding `preserve-outgoing-opaque-fields` in protocol.clj, mirroring the
existing incoming escape hatch for bind params. Regression test added.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Ports upstream SessionFs SQLite support (PR #1299), updates an example per upstream (PR #1315), and bumps pinned Copilot schema assets to 1.0.49 to keep the Clojure SDK wire layer aligned with upstream.
Changes:
- Add SessionFs SQLite RPC support (
sessionFs.sqliteQuery,sessionFs.sqliteExists) including capability advertisement and handler/provider adaptation. - Add protocol escape hatches to preserve opaque SQL bind-parameter keys on incoming requests and preserve row column keys on outgoing sqlite results.
- Bump pinned schema version to
1.0.49and regenerate generated event specs; updatepermission_bashexample and changelog/docs accordingly.
Show a summary per file
| File | Description |
|---|---|
| test/github/copilot_sdk/mock_server.clj | Mock server now responds to sessionFs.setProvider for new capability forwarding tests. |
| test/github/copilot_sdk/integration_test.clj | Adds integration tests covering sqlite adapter behavior, RPC dispatch, capability forwarding, and opaque key preservation. |
| src/github/copilot_sdk/specs.clj | Extends specs with session-fs capabilities, optional sqlite handler keys, and sqlite query type/result shapes. |
| src/github/copilot_sdk/session.clj | Implements sqlite adapter wiring, queryType coercion, and capability validation on handler installation. |
| src/github/copilot_sdk/protocol.clj | Adds incoming/outgoing opaque-field preservation for sqlite bind params and result row keys. |
| src/github/copilot_sdk/generated/event_specs.clj | Regenerated specs from updated schemas (enum tightening, etc.). |
| src/github/copilot_sdk/client.clj | Routes new sqlite RPC methods and forwards :session-fs.capabilities on sessionFs.setProvider. |
| schemas/session-events.schema.json | Upstream schema refresh (formats/enums/refs) as part of 1.0.49 bump. |
| schemas/api.schema.json | Upstream API schema refresh adding sqlite RPCs and related type refactors as part of 1.0.49 bump. |
| schemas/README.md | Updates pinned schema version text to 1.0.49. |
| .copilot-schema-version | Bumps pinned schema version to 1.0.49. |
| examples/permission_bash.clj | Updates permission decision kind to :approve-once. |
| doc/reference/API.md | Documents optional sqlite support for SessionFs providers/handlers. |
| CHANGELOG.md | Adds Unreleased entries for sqlite support + schema bump + example fix. |
Copilot's findings
- Files reviewed: 13/14 changed files
- Comments generated: 2
- session.clj: capabilities.sqlite validation now requires BOTH :sqlite-query
and :sqlite-exists (previously only :sqlite-query). A low-level handler
declaring only :sqlite-query would have passed validation but failed at
runtime with an opaque "Unknown sessionFs method" error when
sessionFs.sqliteExists arrived. The throw now reports which handler
keys are missing.
- doc/reference/API.md: clarify that the low-level handler map requires
the 10 core FS operations and the two :sqlite-* keys are optional
(only required when :capabilities {:sqlite true} is advertised).
- integration_test.clj: regression test that create-session throws when
capabilities.sqlite=true and the low-level handler has :sqlite-query
but not :sqlite-exists.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
krukow
added a commit
that referenced
this pull request
May 21, 2026
Add explicit 'sync local main first' step in Phase 1 (Discovery) and Phase 8 (PR Creation) of the update-upstream skill. A stale local main caused this round-3 PR to be rebased post-hoc against origin/main after PR #108 (round 2) was merged. Also document the post-hoc remedy: when an already-squash-merged commit shows up during rebase, 'git rebase --skip' is the correct action and Git's 'patch contents already upstream' detection will handle subsequent duplicates automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
krukow
added a commit
that referenced
this pull request
May 21, 2026
…0) (#109) * Schema bump 1.0.49 → 1.0.51 + regenerate event specs Pin .copilot-schema-version to 1.0.51 (latest GA on npm), re-fetch schemas/, and regenerate src/github/copilot_sdk/generated/event_specs.clj via 'bb codegen'. Notable generated diffs: - :ttft-ms removed, :time-to-first-token-ms added in assistant.usage-data (wire field 'ttftMs' → 'timeToFirstTokenMs' upstream). - Many predicates change from number? → integer? (upstream PR #1329 uses 32-bit types for bounded schema integers). - Cosmetic gensym renumbering on anyOf-derived specs. No new event types, no new top-level event-data fields beyond the rename. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * MCP stdio: :mcp-args is now optional (upstream PR #1347) Upstream PR #1347 made MCPStdioServerConfig.args optional across all SDKs. Move ::mcp-args from :req-un to :opt-un on ::mcp-local-server (aliased as ::mcp-stdio-server). Add red-then-green test asserting {:mcp-command "true" :mcp-tools ["read"]} validates with no :mcp-args. Update doc/mcp/overview.md to mark :mcp-args optional and document the CLI 1.0.51 origin. Also extend ::assistant.usage-data to accept :time-to-first-token-ms (new wire name from CLI 1.0.51 schema rename of ttftMs → timeToFirstTokenMs) alongside legacy :ttft-ms, so events from older and newer CLIs both validate. Add a focused test for both shapes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ping: :timestamp is now an ISO 8601 string (upstream PR #1340) CLI 1.0.51 changed the 'ping' RPC result timestamp field from epoch millis number to an ISO 8601 date-time string. The SDK forwards the field verbatim, so callers of sdk/ping now receive a string :timestamp. - Update the mock test server to return (.toString (Instant/now)). - Update test-ping and ^:e2e test-e2e-connection assertions to expect a string, and also parse it as Instant to assert ISO 8601 shape. - Update docstrings on sdk/ping and client/ping to document the new shape and note that earlier CLI versions returned a number. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Bump version to 1.0.0-beta.4.1 + CHANGELOG round 3 Document the sync of CLI schema 1.0.49 → 1.0.51, including the three behavioural changes ported (PR #1347, the ttftMs rename, and the ping timestamp shape change) and the items deliberately tracked but not ported (PR #1316 packaging-only, PR #1327 ToolBinaryResult tighten). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * skill(update-upstream): sync local main before branching Add explicit 'sync local main first' step in Phase 1 (Discovery) and Phase 8 (PR Creation) of the update-upstream skill. A stale local main caused this round-3 PR to be rebased post-hoc against origin/main after PR #108 (round 2) was merged. Also document the post-hoc remedy: when an already-squash-merged commit shows up during rebase, 'git rebase --skip' is the correct action and Git's 'patch contents already upstream' detection will handle subsequent duplicates automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review: relax ::timestamp, fix typo, test legacy ttft-ms - ::specs/timestamp now accepts both ISO 8601 strings (CLI ≥ 1.0.51) and numeric epoch-millis (older CLIs), making the spec consistent with the ping docstring contract. The ping fdef return spec no longer fails instrumentation against older CLIs. - Fix "Reflecing" → "Reflecting" typo in update-upstream SKILL.md. - Add backward-compat assertion for legacy :ttft-ms key in test-assistant-usage-time-to-first-token-ms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review round 2: tighten timestamp spec, fix doc comments - ::timestamp epoch-ms branch is now nat-int? (epoch-millis are non-negative integers, not arbitrary numbers). - Fix misleading comment that implied event :timestamp is coerced to Instant at the coercion layer; actually only certain event-data fields are coerced. - Drop POSIX-specific 'true' example from MCP overview (not portable to Windows). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review round 3: clarify changelog ping entry Reword the round-3 'Changed' bullet to make clear that CLI 1.0.51 (not the SDK) changed the ping :timestamp field type, and that the SDK accepts both shapes via ::specs/timestamp so callers see a string on CLI ≥ 1.0.51 and a number on older CLIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review round 4: assert legacy epoch-ms timestamp validates Add explicit assertions that ::specs/timestamp accepts both an ISO 8601 string and a System/currentTimeMillis-sized long, plus boundary checks (negative rejected, fractional number rejected). Locks in backward compatibility with older CLIs that still return numeric :timestamp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review round 5: clarify ::timestamp doc; tighten E2E ping assertion - ::timestamp doc comment now explicitly mentions both Instant/parse for the string form and Instant/ofEpochMilli for the numeric form, so the comment matches the spec's two-branch nature. - E2E ping test now also asserts the :timestamp parses as an ISO 8601 instant (mirroring the integration test), so a stringified epoch-millis from a misbehaving CLI would not silently pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <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.
Ports the SessionFs SQLite support from upstream PR #1299, updates
examples/permission_bash.cljper upstream PR #1315, and bumps the pinned schema version from1.0.49-1to1.0.49.Changes
SessionFs SQLite support (upstream PR #1299)
:sqlite {:query :exists}sub-provider for the provider-stylecreateSessionFsHandlerfactory. Low-level handlers expose flat:sqlite-query/:sqlite-existskeys; the adapter bridges between them.:capabilities {:sqlite true}under:session-fs; the value is forwarded onsessionFs.setProviderand validated at session creation (throws whencapabilities.sqliteis declared but the provider lacks:sqlite).queryTypeis coerced from the wire string to an idiomatic keyword (#{:exec :query :run}) before reaching the handler.$userId) bypass kebab-case conversion via an escape hatch inprotocol/normalize-incoming.:user_id,:created_at) round-trip verbatim on the outgoing wire path via a newpreserve-outgoing-opaque-fieldsescape hatch inprotocol.clj, matching upstream Node.js semantics. (Found via multi-model code review.)Schema 1.0.49
.copilot-schema-versionadvanced from1.0.49-1to1.0.49formatannotations, and the new sqlite RPC methodsevent_specs.cljreflects the new enums (upstream PRs #1305, #1307, #1327, #1333)Examples
examples/permission_bash.cljfrom deprecated:approvedto current:approve-once(upstream PR #1315)Validation
bb testbb validate-docs./run-all-examples.shMulti-model code review
Parallel reviews via Claude Opus 4.7 and GPT-5.5 both independently identified a HIGH severity bug:
preserve-outgoing-opaque-fieldsescape hatch + regression testcapabilities.sqlitevalidation only checks:sqlite-querysession-fs-provider?already validates both arities; mirrors upstreamclient.ts:466await-session-fs-result::sqlite-rows/::sqlite-columnsused wrongseq?predicate(s/coll-of …)Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com