Plan C Phase 7: mock server v3 broadcast event validation#95
Draft
krukow wants to merge 1 commit intoplan-c/codegen-pipelinefrom
Draft
Plan C Phase 7: mock server v3 broadcast event validation#95krukow wants to merge 1 commit intoplan-c/codegen-pipelinefrom
krukow wants to merge 1 commit intoplan-c/codegen-pipelinefrom
Conversation
Adds typo-protection at the test/mock boundary so that injecting an unknown event-type fails fast with a helpful message instead of silently emitting an unrecognised notification (the kind of bug that previously produced confusing "expected event never arrived" test failures). * `mock/send-session-event!` now validates the event-type against the SDK's canonical public `event-types` registry (`github.copilot-sdk/event-types`). Any event the SDK recognises can still be injected. * New `mock/send-v3-broadcast-event!` helper restricts injection to the five protocol v3 broadcast events (`external_tool.requested`, `permission.requested`, `command.execute`, `elicitation.requested`, `capabilities.changed`). A subset assertion at namespace-load time guards against drift in the hand-curated set. * Migrated the four v3 integration tests (13 call sites) from `send-session-event!` to `send-v3-broadcast-event!` to make protocol v3 intent explicit. * Negative tests that need to inject deliberately-unknown event types can use `send-notification!` to bypass validation (documented in the helper's docstring). 200 tests / 668 assertions still pass on `bb test`. 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
1 participant
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.
Plan C Phase 7 — mock server v3 broadcast event coverage
Stacks on #93 (codegen pipeline). Independent of #94 (instrument dedup).
What
Adds typo-protection at the test/mock boundary.
mock/send-session-event!now validates the event-type against the SDK's canonical publicevent-typesregistry; injecting an unknown type throws with a helpful message instead of silently producing a notification the client ignores.A dedicated
mock/send-v3-broadcast-event!helper restricts injection to the five protocol v3 broadcast events (external_tool.requested,permission.requested,command.execute,elicitation.requested,capabilities.changed) — RPC-replacement events the SDK intercepts inclient/handle-v3-broadcast-event!. A namespace-load-time assertion catches drift in the hand-curated set.The four v3 integration tests (13 call sites) have migrated to the dedicated helper to make protocol v3 intent explicit. Negative tests can still bypass validation via
send-notification!.Why
Previously the mock accepted any string as event-type. A typo like
"session.startt"produced a silently-ignored notification on the client side, and the test failed with a confusing "expected event never arrived" timeout. With Plan C giving us a canonical event registry, validating at the injection point is a free win.Design notes (from rubber-duck review)
The first attempt validated against the schema-derived set (
event-specs/event-types, 38 events). That was too strict — the SDK's publicevent-typesset is hand-curated and includes 74 events, including protocol-level events likepermission.requestedandmcp.oauth_requiredthat aren't in the upstream session-events schema. The rubber-duck flagged this and we switched to the SDK registry as the source of truth.We deliberately do NOT assert that
event-specs/event-types ⊆ known-event-types: there is real drift (e.g.session.import_legacyis in the upstream schema but not yet in the SDK's curated set). That drift is a known issue tracked separately by the codegen pipeline and out of scope here.Verification
bb test— 200 tests / 668 assertions pass.Closes nothing (Plan C tracking issue is internal).