fix: support bracketed IPv6 runtime URLs - #2200
Conversation
|
The underlying gap is worth fixing, but I think this PR should be narrower so it adds IPv6 without changing the existing endpoint grammar or taking on general URL-validation policy. A minimal version would be:
The intended new cases are simply: RuntimeConnection.forUri("[::1]:4321");
RuntimeConnection.forUri("http://[2001:db8::1]:4321");Tests should cover those forms in each URL-based SDK, plus one Python connector-level test proving the connection path is not forced to I would leave path/query/fragment rejection, whitespace normalization, arbitrary-scheme handling, userinfo policy, and unrelated port-validation changes out of this PR. Moving wholesale to general-purpose URL parsers changes which previously accepted inputs succeed or fail and introduces cross-language differences that then require us to define and maintain a much larger URL contract. A narrow bracketed-IPv6 branch fixes the demonstrated user problem, preserves compatibility, and avoids pulling those edge cases into scope. |
|
@xianjianlf2 Does that sound reasonable? I'll mark this PR as draft but please remark as ready to review as appropriate. |
Fixes #1137
Summary
Validation
cd go && go test .cd python && uv run pytest test_client.py -qcd nodejs && npx vitest run test/client.test.ts -t "URL parsing"cd dotnet && dotnet test test/GitHub.Copilot.SDK.Test.csproj --filter RuntimeConnectionUrlParsingTests(dotnetis not installed in this environment:zsh:1: command not found: dotnet)