fix: tolerate unknown hook types in .NET and Go SDKs#1013
fix: tolerate unknown hook types in .NET and Go SDKs#1013
Conversation
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>
|
@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. |
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>
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
Agent-Logs-Url: https://github.com/github/copilot-sdk/sessions/5a0ffe17-78e7-4008-a90a-67b439ccd210 Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>
Added in 24c28e8. Since |
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.