Consolidate bounded pane context with GetPaneContext - #884
Consolidate bounded pane context with GetPaneContext#884Kai Tao (vanzue) wants to merge 2 commits into
Conversation
Add GetPaneContext protocol support, shared planner and Autofix collection, bounded capture coverage, and reproducible same-build performance benchmarks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77616472-cc8d-4566-9050-60ab09248076
There was a problem hiding this comment.
🟡 Changes recommended
wtcli get-pane-context currently fails silently on an invalid --target GUID (non-zero exit without an error message), which should be fixed for debuggability and script reliability.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR implements the consolidated, bounded pane-context operation for #838 by appending GetPaneContext to the existing Terminal Protocol COM interface (protocol 2.3), exposing it via wtcli get-pane-context, and routing WTA planner/manual-fix/autofix context collection through the single consolidated request with bounded capture and accurate truncation reporting.
Changes:
- Add protocol 2.3
GetPaneContextend-to-end (IDL/COM server/TerminalPage/ControlCore) with bounded marked-command capture and bounded buffer-tail fallback. - Add
wtcli get-pane-contextwith protocol/capability negotiation and max-lines/max-chars validation; add WTA channel support (get_pane_context) and migrate ACP prompt context collection to use it (with legacy fallback for unsupported servers). - Add E2E coverage for pane-context scenarios and a PowerShell benchmark harness + selftests + documentation + release checklist mapping.
File summaries
| File | Description |
|---|---|
| tools/wta/src/shell/wt_channel/cli_channel.rs | Map get_pane_context to a single wtcli get-pane-context invocation with bounds validation. |
| tools/wta/src/shell/shell_manager.rs | Add wt_get_pane_context helper routing through the WT channel. |
| tools/wta/src/protocol/acp/prompt_context.rs | Switch planner/autofix context capture to consolidated pane-context request; retain capability-gated legacy fallback. |
| tools/wta/src/protocol/acp/prompt_builder.rs | Remove direct shell manager field from ContextRequest and adapt tests/mocks to consolidated capture. |
| tools/wta/src/protocol/acp/mock_agent_tests.rs | Update blocking/mock WT channel behavior to serve consolidated pane-context payload shape. |
| test/e2e/tools/PaneContextBenchmark.ps1 | Add shared benchmark/statistics + UTF-16-surrogate-safe scalar counting and prompt truncation helpers. |
| test/e2e/tests/Feature.PaneContext.Tests.ps1 | Add packaged E2E tests covering marked/unmarked capture, isolation, missing panes, metadata-only, Unicode/truncation, and agent-focus resolution. |
| test/e2e/selftests/PaneContextBenchmark.Unit.Tests.ps1 | Add hermetic Pester selftests for benchmark math, Unicode handling, and request fidelity. |
| test/e2e/README.md | Document the new pane-context benchmark workflow and add the new PaneContext feature suite to the table. |
| test/e2e/Measure-PaneContext.ps1 | Add the benchmark driver script that validates stability and compares legacy vs consolidated collectors on the same deployed build. |
| src/tools/wtcli/main.cpp | Add get-pane-context subcommand with negotiation and validation; call ITerminalProtocol::GetPaneContext. |
| src/host/proxy/ITerminalProtocol.idl | Append COM GetPaneContext without reordering existing vtable slots (protocol 2.3). |
| src/cascadia/WindowsTerminal/TerminalProtocolComServer.h | Declare COM server implementation of GetPaneContext. |
| src/cascadia/WindowsTerminal/TerminalProtocolComServer.cpp | Advertise protocol 2.3 + capability; implement GetPaneContext routing to TerminalPage::GetProtocolPaneContext. |
| src/cascadia/ut_app/ProtocolParsingTests.cpp | Add unit tests for bounded UTF-8/line handling in command and buffer-tail builders. |
| src/cascadia/UnitTests_Control/ControlCoreTests.cpp | Add unit tests for ControlCore::ReadBufferTail behavior, bounds, and Unicode handling. |
| src/cascadia/TerminalProtocol/TerminalProtocol.idl | Add PaneContext struct and GetPaneContext WinRT method to the Terminal Protocol surface. |
| src/cascadia/TerminalProtocol/ProtocolParsing.h | Add bounded UTF-8-aware command and buffer-tail text builders used by pane-context capture. |
| src/cascadia/TerminalControl/TermControl.idl | Add ReadBufferTail and ReadLastPromptBounded to the TermControl API. |
| src/cascadia/TerminalControl/TermControl.h | Declare TermControl wrappers for new bounded read methods. |
| src/cascadia/TerminalControl/TermControl.cpp | Implement TermControl wrappers delegating to ControlCore. |
| src/cascadia/TerminalControl/ControlCore.idl | Add ReadBufferTail and ReadLastPromptBounded to ControlCore WinRT surface. |
| src/cascadia/TerminalControl/ControlCore.h | Declare new bounded read APIs on ControlCore. |
| src/cascadia/TerminalControl/ControlCore.cpp | Implement bounded buffer-tail reading and bounded mark-based last-prompt reading with Unicode-safe iteration. |
| src/cascadia/TerminalApp/TerminalPage.Protocol.cpp | Implement GetProtocolPaneContext resolving explicit/effective source pane and performing bounded capture/fallback. |
| src/cascadia/TerminalApp/TerminalPage.idl | Add GetProtocolPaneContext to TerminalPage IDL. |
| src/cascadia/TerminalApp/TerminalPage.h | Add GetProtocolPaneContext declaration. |
| doc/release-check-list.md | Add/reassign release checklist items C296–C303 for pane-context and routing coverage. |
Review details
- Files reviewed: 28/28 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| GUID source{}; | ||
| const auto hasExplicitSource = !paneContextTarget.empty(); | ||
| if (hasExplicitSource) | ||
| { | ||
| source = GuidFromString(paneContextTarget); | ||
| if (InlineIsEqualGUID(source, GUID{})) | ||
| { | ||
| exitCode = 1; | ||
| return; | ||
| } | ||
| } |
This comment has been minimized.
This comment has been minimized.
Preserve consolidated context dispatch alongside upstream hook listener changes and reconcile release checklist IDs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77616472-cc8d-4566-9050-60ab09248076
There was a problem hiding this comment.
🔵 Needs a closer look
It spans COM ABI changes, cross-process protocol negotiation, and UI-thread-adjacent context capture logic that merits final human validation.
Review details
- Files reviewed: 28/28 changed files
- Comments generated: 1
- Review effort level: Lite
| const auto lastCommand = termControl.ReadLastPromptBounded(maxLines + 1, maxCharacters + 1); | ||
| if (!lastCommand.empty()) | ||
| { | ||
| const auto bounded = ProtocolParsing::BuildBoundedCommand( | ||
| winrt::to_string(lastCommand), | ||
| maxLines, | ||
| maxCharacters); | ||
| result.Content = winrt::to_hstring(bounded.content); | ||
| result.OutputSource = L"last_command"; | ||
| result.LineCount = bounded.lineCount; | ||
| result.Truncated = bounded.truncated; | ||
| result.HasMarks = true; | ||
| co_return result; | ||
| } |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| VERIFY_IS_TRUE(result.truncated); | ||
|
|
||
| const auto lines = BuildBoundedCommand("command\r\nfirst\r\nsecond\r\n", 2, 100); | ||
| VERIFY_ARE_EQUAL("command\nfirst", lines.content); |
| VERIFY_ARE_EQUAL(2, blankLineLookahead.lineCount); | ||
| VERIFY_IS_TRUE(blankLineLookahead.truncated); | ||
|
|
||
| const auto leadingBlankLines = BuildBoundedCommand("\n\ncommand\n", 10, 100); |
| VERIFY_IS_TRUE(blankLineLookahead.truncated); | ||
|
|
||
| const auto leadingBlankLines = BuildBoundedCommand("\n\ncommand\n", 10, 100); | ||
| VERIFY_ARE_EQUAL("\n\ncommand\n", leadingBlankLines.content); |
| void ProtocolParsingTests::BoundedBufferTailAppliesLineAndCharacterLimits() | ||
| { | ||
| const auto byLines = BuildBoundedBufferTail("first\r\nsecond\r\nthird\r\n", 2, 100); | ||
| VERIFY_ARE_EQUAL("second\nthird", byLines.content); |
| VERIFY_IS_TRUE(byLines.truncated); | ||
|
|
||
| const auto byCharacters = BuildBoundedBufferTail("one\r\ntwo\r\nthree\r\n", 3, 6); | ||
| VERIFY_ARE_EQUAL("\nthree", byCharacters.content); |
check-spelling-bot Report
|
| Dictionary | Entries | Covers | Uniquely |
|---|---|---|---|
| cspell:csharp/csharp.txt | 32 | 2 | 2 |
| cspell:aws/aws.txt | 232 | 2 | 2 |
| cspell:fonts/fonts.txt | 536 | 1 | 1 |
Consider adding to the extra_dictionaries array (in the .github/actions/spelling/config.json file):
"cspell:csharp/csharp.txt",
"cspell:aws/aws.txt",
"cspell:fonts/fonts.txt",
To stop checking additional dictionaries, put (in the .github/actions/spelling/config.json file):
"check_extra_dictionaries": []Pattern suggestions ✂️ (1)
You could add these patterns to .github/actions/spelling/patterns/71516d3bb5164939437b76ea6e830eac9ab39fbd.txt:
# Automatically suggested patterns
# hit-count: 1 file-count: 1
# container images
image: [-\w./:@]+
Alternatively, if a pattern suggestion doesn't make sense for this project, add a # to the beginning of the line in the candidates file with the pattern to stop suggesting it.
Warnings and Notices ⚠️ (2)
See the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.
| Count | |
|---|---|
| ℹ️ candidate-pattern | 1 |
| 54 |
See
✏️ Contributor please read this
By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
- ... misspelled, then please correct them instead of using the command.
- ... names, please add them to
.github/actions/spelling/allow/names.txt. - ... APIs, you can add them to a file in
.github/actions/spelling/allow/. - ... just things you're using, please add them to an appropriate file in
.github/actions/spelling/expect/. - ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in
.github/actions/spelling/patterns/.
See the README.md in each directory for more information.
🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
Summary
Implements the consolidated, bounded context operation for #838, named
GetPaneContextas agreed during implementation.GetPaneContextto the existing COM protocol without changing earlier vtable slots; advertise protocol 2.3 andget_pane_context.wtcli get-pane-context, with version/capability negotiation before calling the appended method and validation of targets and capture budgets./fix, and error-triggered Autofix context collection through WTA -> wtcli -> the existing COM server. No direct Rust COM client and no new COM server.The Autofix event-delivery and cross-tab routing fixes were isolated and already merged in #880. They are part of this branch's main baseline, not new fixes in this PR.
Debug benchmark results
Important: this is a same-build comparison of two context-collection paths, not an A/B comparison of two separately built branches.
Both paths ran against the same locally deployed x64 Debug package built from
dev/vanzue/get-context, based on main commitdb609f8061f81c2eb9a4bdaf3e0666392596bce4, with the feature changes applied:IntelligentTerminal_0.8.0.2_x64__rd9vj3e6a2mbr.src\cascadia\CascadiaPackage\bin\x64\Debug\AppX.wtcli.exeand the same running Terminal/server. Deployed WTA and WindowsTerminal hashes matched the build outputs.active-panecall, followed by window/tab/pane enumeration and output capture. There is no artificially added unsupported-capability probe.wtcli get-pane-contextprocess captures the context. Authentication and capability negotiation still happen within wtcli; one process does not mean only one raw COM call./fix/fix/fixIn these measurements, median context-collection time decreased by 47.9%-88.9%, saving approximately 80-685 ms per collection. The new path's p50 stayed at approximately 84-87 ms.
Interpretation and limits
Reproduction and artifacts
The PR includes
test\e2e\Measure-PaneContext.ps1, its shared helpers, 16 hermetic selftests, and usage documentation intest\e2e\README.md.Example, with an already-running Dev package and a stable existing pane:
For Planner or ManualFix, focus the expected source pane first; the script does not change focus, send input, change settings, or start/stop Terminal. Each run exports
samples.csv,requests.csv,metadata.json, andsummary.json, including binary hashes, source provenance, and timing definitions.Local measurements are retained under
test\e2e\artifacts\pane-context-benchmark\, with the combined table indebug-comparison.csv. These ignored local artifacts are not committed or uploaded; the measured summary is reproduced above.Coverage and validation
e90ab68c4: 2,082 passed, 1 ignored.get_pane_contextcapability and metadata-only request succeeded.e90ab68c4, PaneContext uses C300-C306; Autofix routing uses C299. This preserves main's hook cases at C296-C298 and removes the merged-main C295 collision with Ctrl+wheel coverage.Remaining validation