feat(protocol): typed daemon event schema v1#4217
Merged
Merged
Conversation
Contributor
📋 Review SummaryThis PR introduces a typed daemon event schema v1 for the TypeScript SDK, providing known event unions, runtime narrowing helpers, and a reducer for building session view state. The implementation is well-structured, maintains backward compatibility by preserving the raw 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
wenshao
requested changes
May 16, 2026
wenshao
reviewed
May 16, 2026
wenshao
requested changes
May 16, 2026
wenshao
requested changes
May 16, 2026
wenshao
reviewed
May 16, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Schema 与 packages/cli/src/serve/ 下 daemon 的实际 SSE 发射点对齐过(8 个事件类型逐个 grep 验证),LGTM with nits inline。最值得跟进的是严格校验路径下的静默降级行为 —— 在更多 adapter 依赖 typed view 之前最好补一个可观测信号。
3e15930 to
ea91731
Compare
Collaborator
Author
|
Review follow-up update:
Local validation passed:
I resolved the addressed review threads. The remaining CHANGES_REQUESTED state appears to be the previous review decision on the older commit; please re-review when convenient. |
wenshao
reviewed
May 17, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Follow-up review on top of the round addressed in ea917315d. Verified all 8 emit sites in httpAcpBridge.ts/eventBus.ts/server.ts match the validators; typecheck + vitest pass locally. 5 minor follow-ups inline — none are blocking, point 1 is the most worthwhile to address before merge.
wenshao
reviewed
May 17, 2026
wenshao
reviewed
May 17, 2026
wenshao
requested changes
May 17, 2026
ea91731 to
ab88f6f
Compare
Collaborator
Author
|
更新了一轮，已处理最新 review 线程并全部 resolve：
本地验证：
GitHub CI 已重新触发；当前 Classify PR 已通过，其余 CodeQL/Lint/三平台 Test 还在 pending。 |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
approved these changes
May 17, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Local tmux verification on ab88f6f (squashed HEAD on top of current main):
npx vitest runSDK schema + DaemonSessionClient suites: 20/20 passing.npm run typecheck(SDK),npm run build(SDK), full reponpm run build: all clean (only the pre-existing vscode-companion curly lint warning, as noted in PR description).npx eslinton the four touched files: 0 problems.- GitHub CI: Lint / CodeQL / Test (mac+ubuntu+win) / coverage — all green.
All eight items from my 2026-05-17 03:12Z round are addressed (verified via git diff ea91731..ab88f6f on events.ts):
lastEventIdis now monotonic viaadvanceLastEventId+Math.max— fixes SSE resume cursor regression on out-of-order ids.pendingPermissionscapped at 64 (matches server-sideDEFAULT_MAX_PENDING_PER_SESSIONin httpAcpBridge.ts), withdroppedPermissionRequestCount/lastDroppedPermissionRequestIdobservability counters.isPermissionRequestDatanow requiresisRecord(toolCall)instead of'toolCall' in value, rejectingtoolCall: null.permission_resolvedon unknown requestId is no longer silent:unmatchedPermissionResolutionCount+lastUnmatchedPermissionResolutionId.isOptionalNumber/isOptionalNumberOrNullrejectNaNviaisFiniteNumber.isRecordchecksObject.prototypeto rejectMap/Date/class instances.- Permission request data shallow-copied via
clonePermissionRequestData(data + options entries) before storing in reducer state. - Event type list extracted to a single
DAEMON_KNOWN_EVENT_TYPE_VALUESsource; reducerdefaultis now an exhaustivenessneverguard.
Test coverage extended for: toolCall: null, session_died bad-type optional fields, client_evicted.droppedAfter bad type, permission_resolved empty optionId, out-of-order id monotonicity, cancelled outcome, unmatched resolution, pendingPermissions cap, and the full terminal-event preference matrix (client↔stream + upgrade to session_died).
LGTM.
doudouOUC
added a commit
that referenced
this pull request
May 17, 2026
Follow-up to #4217 (`feat(protocol): add typed daemon event schema v1`, Wave 1 PR 4 of #4175), which landed the SDK-side typed schema + `KnownDaemonEvent` union + reducer but did not register a daemon-side capability tag for it. Without the tag, non-SDK clients (web debug UI, third-party adapters, channel/IDE backends not yet on `@qwen-code/sdk`) have no way to detect at the protocol envelope level that the daemon promises to emit only `KnownDaemonEvent`-shaped frames — they would either pin against SDK version, or pre-flight every frame defensively. Add `typed_event_schema: { since: 'v1' }` to `SERVE_CAPABILITY_REGISTRY`, inserted right after `session_events` (the route that delivers the frames whose schema this tag describes). The capability is purely informational — `narrowDaemonEvent`/`asKnownDaemonEvent` already fall back to "unknown" for older daemons that don't advertise the tag, so the SDK does not gate any behavior off the tag. Sync `EXPECTED_STAGE1_FEATURES` (server.test.ts) and the integration test array (qwen-serve-routes.test.ts) with the registry order, the same lockstep discipline #4214 codified. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
doudouOUC
added a commit
that referenced
this pull request
May 17, 2026
…monSessionClient docstring at it Two small follow-ups to #4217 (Wave 1 PR 4 of #4175). 1. Public-entry regression fence `@qwen-code/sdk` is a single-entry package: `package.json.exports` only exposes `.` (`dist/index.{cjs,mjs,d.ts}`), and the bundle is built from `src/index.ts`. Symbols re-exported only from `src/daemon/index.ts` are unreachable to consumers unless they are also forwarded by `src/index.ts`. #4217 forwards the typed event schema correctly today, but the two-layer chain has no compile-time test pinning it — a future daemon export that lands in `src/daemon/index.ts` but is missed by `src/index.ts` would ship invisibly. Add `test/unit/daemon-public-surface.test.ts` that imports `* as Public from '../../src/index.js'`, asserts at runtime that every PR 4 value is `typeof === 'function'` (or a primitive of the expected shape), round-trips a raw `DaemonEvent` through the public `asKnownDaemonEvent` to prove the wire-up actually works, and compile-imports every PR 4 type so any drift fails to build. 2. DaemonSessionClient docstring pointer The class docstring already deferred typed event consumption to "the protocol schema layer" without a concrete pointer. Now that #4217 has put `asKnownDaemonEvent` and `reduceDaemonSessionEvent` in `./events.js`, name them so future readers can find the typed surface without grepping. No code change. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Merged
11 tasks
wenshao
pushed a commit
that referenced
this pull request
May 17, 2026
…4175 PR 4 follow-up) (#4226) * feat(serve): advertise typed_event_schema capability Follow-up to #4217 (`feat(protocol): add typed daemon event schema v1`, Wave 1 PR 4 of #4175), which landed the SDK-side typed schema + `KnownDaemonEvent` union + reducer but did not register a daemon-side capability tag for it. Without the tag, non-SDK clients (web debug UI, third-party adapters, channel/IDE backends not yet on `@qwen-code/sdk`) have no way to detect at the protocol envelope level that the daemon promises to emit only `KnownDaemonEvent`-shaped frames — they would either pin against SDK version, or pre-flight every frame defensively. Add `typed_event_schema: { since: 'v1' }` to `SERVE_CAPABILITY_REGISTRY`, inserted right after `session_events` (the route that delivers the frames whose schema this tag describes). The capability is purely informational — `narrowDaemonEvent`/`asKnownDaemonEvent` already fall back to "unknown" for older daemons that don't advertise the tag, so the SDK does not gate any behavior off the tag. Sync `EXPECTED_STAGE1_FEATURES` (server.test.ts) and the integration test array (qwen-serve-routes.test.ts) with the registry order, the same lockstep discipline #4214 codified. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * test(sdk): pin typed event surface at the public SDK entry, point DaemonSessionClient docstring at it Two small follow-ups to #4217 (Wave 1 PR 4 of #4175). 1. Public-entry regression fence `@qwen-code/sdk` is a single-entry package: `package.json.exports` only exposes `.` (`dist/index.{cjs,mjs,d.ts}`), and the bundle is built from `src/index.ts`. Symbols re-exported only from `src/daemon/index.ts` are unreachable to consumers unless they are also forwarded by `src/index.ts`. #4217 forwards the typed event schema correctly today, but the two-layer chain has no compile-time test pinning it — a future daemon export that lands in `src/daemon/index.ts` but is missed by `src/index.ts` would ship invisibly. Add `test/unit/daemon-public-surface.test.ts` that imports `* as Public from '../../src/index.js'`, asserts at runtime that every PR 4 value is `typeof === 'function'` (or a primitive of the expected shape), round-trips a raw `DaemonEvent` through the public `asKnownDaemonEvent` to prove the wire-up actually works, and compile-imports every PR 4 type so any drift fails to build. 2. DaemonSessionClient docstring pointer The class docstring already deferred typed event consumption to "the protocol schema layer" without a concrete pointer. Now that #4217 has put `asKnownDaemonEvent` and `reduceDaemonSessionEvent` in `./events.js`, name them so future readers can find the typed surface without grepping. No code change. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
46 tasks
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
3 participants
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.
Summary
This is stacked on #4201 and targets
feat/daemon-session-clientintentionally. After #4201 lands, this branch can be rebased/retargeted tomain.Validation
Scope / Risk
Testing Matrix
Testing matrix notes:
Linked Issues / Bugs