Commit dd42d42
authored
Close Language Gaps for Commands + Dialogs/Elicitations (#960)
* Close Language Gaps for Commands + Dialogs/Elicitations
* Fix code quality review feedback and formatting issues
- Python: fix ruff formatting, add comments to empty except blocks, remove unused imports
- .NET: simplify boolean expressions, combine nested ifs, narrow generic catch clause
- Go: fix struct field alignment for go fmt compliance
* Fix Python ruff lint errors: unused imports, import sort order, line length
* Fix Python type checker errors: remove unused type-ignore comments, fix capabilities type
* Fix Go struct field alignment for go fmt compliance
* Fix Python E2E tests: use correct snapshot directory 'multi_client'
* Skip flaky Python E2E disconnect test: force_stop() doesn't trigger capabilities.changed reliably in replay proxy
* fix: close makefile wrapper in Python force_stop() to trigger TCP disconnect
Python's socket.makefile() holds its own reference to the socket.
Calling socket.close() alone won't release the OS-level resource
until the makefile wrapper is also closed. This meant force_stop()
wasn't actually closing the TCP connection, so the server never
detected the disconnect and never sent capabilities.changed events
to other clients.
Fix: close the file wrapper before the socket in SocketWrapper.terminate().
Unskip test_capabilities_changed_when_elicitation_provider_disconnects.
* fix: address remaining code review feedback
- Narrow generic catch clauses in .NET command/elicitation handlers
with 'when (ex is not OperationCanceledException)' filter
- Remove redundant null-conditional (val?.ToString -> val.ToString)
in SelectAsync and InputAsync switch expressions
- Add explanatory comments to Python empty except blocks
* fix: use socket.shutdown() in Python force_stop() for reliable disconnect
socket.close() and file wrapper close don't reliably interrupt a
blocking readline() on another thread in Python. socket.shutdown(SHUT_RDWR)
sends TCP FIN to the server immediately (triggering server-side
disconnect detection) and interrupts any pending blocking reads
across threads — matching Node.js socket.destroy() and Go conn.Close()
behavior.
* chore: remove working markdown files from PR
* fix: pass full elicitation schema in Go, add schema tests across SDKs
Go was only passing RequestedSchema.Properties to the elicitation
handler, dropping the 'type' and 'required' fields. This meant
handlers couldn't reconstruct the full JSON Schema. Now passes a
complete map with type, properties, and required.
Also replaces custom containsString/searchSubstring helpers in Go
tests with strings.Contains, and adds tests in Go and Python that
verify the full schema is passed through to elicitation handlers.
* fix: Go test compilation errors for schema extraction test
Use direct schema extraction logic test instead of dispatching
through session event machinery, avoiding need for RPC mocks.
Fixes undefined SessionEventData and handleEvent references.
* fix: resolve staticcheck SA4031 lint in Go schema test
* test: add Go command error, unknown command, and elicitation handler tests
- Command handler error propagation: verifies handler error is returned
- Unknown command: verifies getCommandHandler returns false for unknown
- Elicitation handler error: verifies error propagation from handler
- Elicitation handler success: verifies result with action and content
* fix: remove redundant nil check flagged by staticcheck SA4031
* docs: promote Commands and UI Elicitation to top-level sections in .NET README
Matches Go and Python README structure where these are ## (h2) sections
rather than ### (h3) subsections. Closes documentation gap flagged
by SDK Consistency Review Agent.
* fix: address human review feedback
.NET:
- Cache SessionUiApiImpl instance via Lazy<> instead of allocating on
every .Ui access
- Await command/elicitation handler calls instead of redundant
fire-and-forget (outer caller already fire-and-forgets)
- Use ElicitationRequestedDataMode enum for Mode instead of string
Go:
- Handle SessionEventTypeCapabilitiesChanged in handleBroadcastEvent
to update session capabilities when other clients join/leave with
elicitation handlers
- Add test verifying capabilities.changed event updates session
* refactor: merge ElicitationRequest + ElicitationInvocation into ElicitationContext
Combines the two-argument elicitation handler pattern into a single
ElicitationContext type across all three SDKs, matching the existing
CommandContext pattern. The context now includes SessionId alongside
the request fields (Message, RequestedSchema, Mode, etc.).
Changes per language:
- .NET: ElicitationContext class, single-arg delegate, Lazy<> cached Ui
- Go: ElicitationContext struct, single-arg handler func
- Python: ElicitationContext TypedDict, single-arg callable
All tests, READMEs, and E2E tests updated.
* refactor: apply ElicitationContext rename to Node.js SDK
Consistent with Python, Go, and .NET — ElicitationRequest is now
ElicitationContext with sessionId included. Handler takes single arg.
Completes the cross-SDK consistency change.
* style: fix formatting (prettier, ruff, trailing newlines)
* style: fix Python import sort order in __init__.py
* fix: simplify Ui auto-property and remove empty snapshot files
- Replace Lazy<ISessionUiApi> with simple auto-property initialized
in constructor, per reviewer feedback
- Delete 14 empty snapshot YAML files (commands, elicitation,
multi_client) that had no conversation data
* fix: rename misleading command test names
Renamed to accurately reflect what they verify:
- Forwards_Commands_In_Session_Create -> Session_With_Commands_Creates_Successfully
- Forwards_Commands_In_Session_Resume -> Session_With_Commands_Resumes_Successfully
Actual forwarding verification is in the multi-client test
Client_Receives_Commands_Changed_When_Another_Client_Joins_With_Commands
which proves the server received the commands by checking the
commands.changed event on another client.
* fix: remove leftover JSDoc from ElicitationRequest rename1 parent ad63b09 commit dd42d42
File tree
27 files changed
+5064
-88
lines changed- dotnet
- src
- test
- go
- internal/e2e
- nodejs
- src
- test
- python
- copilot
- e2e
27 files changed
+5064
-88
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
488 | 488 | | |
489 | 489 | | |
490 | 490 | | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
491 | 580 | | |
492 | 581 | | |
493 | 582 | | |
| |||
812 | 901 | | |
813 | 902 | | |
814 | 903 | | |
| 904 | + | |
| 905 | + | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
815 | 948 | | |
816 | 949 | | |
817 | 950 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
456 | 456 | | |
457 | 457 | | |
458 | 458 | | |
| 459 | + | |
| 460 | + | |
459 | 461 | | |
460 | 462 | | |
461 | 463 | | |
| |||
501 | 503 | | |
502 | 504 | | |
503 | 505 | | |
504 | | - | |
505 | | - | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
506 | 510 | | |
507 | 511 | | |
508 | 512 | | |
509 | 513 | | |
510 | 514 | | |
| 515 | + | |
511 | 516 | | |
512 | 517 | | |
513 | 518 | | |
| |||
570 | 575 | | |
571 | 576 | | |
572 | 577 | | |
| 578 | + | |
| 579 | + | |
573 | 580 | | |
574 | 581 | | |
575 | 582 | | |
| |||
616 | 623 | | |
617 | 624 | | |
618 | 625 | | |
619 | | - | |
620 | | - | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
621 | 630 | | |
622 | 631 | | |
623 | 632 | | |
624 | 633 | | |
625 | 634 | | |
| 635 | + | |
626 | 636 | | |
627 | 637 | | |
628 | 638 | | |
| |||
1592 | 1602 | | |
1593 | 1603 | | |
1594 | 1604 | | |
| 1605 | + | |
| 1606 | + | |
1595 | 1607 | | |
1596 | 1608 | | |
1597 | 1609 | | |
| |||
1614 | 1626 | | |
1615 | 1627 | | |
1616 | 1628 | | |
1617 | | - | |
| 1629 | + | |
| 1630 | + | |
1618 | 1631 | | |
1619 | 1632 | | |
1620 | 1633 | | |
| |||
1640 | 1653 | | |
1641 | 1654 | | |
1642 | 1655 | | |
| 1656 | + | |
| 1657 | + | |
1643 | 1658 | | |
1644 | 1659 | | |
1645 | 1660 | | |
1646 | 1661 | | |
1647 | 1662 | | |
1648 | | - | |
| 1663 | + | |
| 1664 | + | |
| 1665 | + | |
| 1666 | + | |
| 1667 | + | |
| 1668 | + | |
1649 | 1669 | | |
1650 | 1670 | | |
1651 | 1671 | | |
| |||
1782 | 1802 | | |
1783 | 1803 | | |
1784 | 1804 | | |
| 1805 | + | |
| 1806 | + | |
1785 | 1807 | | |
1786 | 1808 | | |
1787 | 1809 | | |
| 1810 | + | |
1788 | 1811 | | |
1789 | 1812 | | |
1790 | 1813 | | |
| |||
0 commit comments