fix: tolerate unknown hook types in .NET and Go SDKs#1013
fix: tolerate unknown hook types in .NET and Go SDKs#1013SteveSandersonMS merged 4 commits intomainfrom
Conversation
|
@copilot Why did you add so much stuff to Go? Is a new otel dependency really needed for this tiny change? Filter this PR down to the actual minimal change. |
Those changes were picked up accidentally when the Go toolchain resolved transitive dependencies. Reverted in 2624819 — the PR now only touches |
|
@copilot Any reason there isn't a .NET test like the Go one? |
There was a problem hiding this comment.
Pull request overview
This PR improves forward compatibility of the .NET and Go SDKs when the Copilot CLI invokes a hook type the SDK doesn’t recognize, preventing JSON-RPC errors that can cause the CLI to terminate the session.
Changes:
- .NET: unknown hook types no longer throw; they return
null(ignored). - Go: unknown hook types no longer return an error; they return
(nil, nil)(ignored). - Go: adds a unit test to ensure unknown hook types are ignored even when other hooks are registered.
Show a summary per file
| File | Description |
|---|---|
| go/session.go | Stops treating unknown hook types as errors; ignores them instead. |
| go/session_test.go | Adds regression coverage for forward-compatible hook dispatch behavior. |
| dotnet/src/Session.cs | Stops throwing on unknown hook types; returns null so the RPC call succeeds. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 0
Added in 24c28e8. Since |
24c28e8 to
296e22e
Compare
Agent-Logs-Url: https://github.com/github/copilot-sdk/sessions/0ea3d153-24a4-4136-807f-cb4fd9223da0 Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/copilot-sdk/sessions/0ea3d153-24a4-4136-807f-cb4fd9223da0 Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/copilot-sdk/sessions/fd4a2cd1-7805-4214-8268-d71285f1ecc0 Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>
296e22e to
98a5159
Compare
Cross-SDK Consistency Review ✅This PR correctly fixes a forward-compatibility issue in the .NET and Go SDKs and achieves full cross-SDK parity. Here's the summary across all four implementations:
All four SDKs now silently ignore unknown hook types rather than treating them as fatal errors. No additional changes are needed to maintain consistency.
|
When the CLI invokes a hook type the SDK doesn't recognize (e.g.
postToolUseFailure), both the .NET and Go SDKs were returning an error over JSON-RPC. The CLI treated this as a fatal condition and shut down the session — appearing as a hang to the client.Changes
dotnet/src/Session.cs: Default switch arm changed fromthrow new ArgumentException(...)tonull— unknown hook types are silently ignoreddotnet/test/HookForwardCompatibilityTests.cs: Added unit tests using an in-process fake CLI server (connected viaCliUrl) covering both "hooks registered but unknown type received" and "no hooks registered" scenariosgo/session.go: Default switch case changed fromreturn nil, fmt.Errorf("unknown hook type: %s", ...)toreturn nil, nil— same behaviorgo/session_test.go: AddedTestSession_HookForwardCompatibilityunit tests covering both "hooks registered but unknown type received" and "no hooks registered" scenariosNode.js already handled this correctly (returns
undefinedfor unmapped hook types); this brings .NET and Go to parity.