Fix Ctrl+wheel zoom in agent panes - #877
Conversation
Prioritize vertical Ctrl+wheel zoom for agent TermControls while preserving plain and horizontal VT mouse input. Add focused unit coverage and physical packaged E2E coverage with release checklist mapping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new Win32 GetCursorPos P/Invoke is missing SetLastError=true, so failures can report incorrect Win32 error codes in the new wheel-injection helper.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes Ctrl+mouse-wheel zoom behavior in agent panes by letting agent TermControls prioritize zoom handling before VT mouse tracking consumes vertical Ctrl+wheel, while keeping plain wheel and horizontal wheel routed to the agent as before.
Changes:
- Add an agent-only “prioritize zoom on Ctrl+vertical wheel” path in
ControlInteractivity::MouseWheel, wired throughTermControlvia a new opt-in flag. - Enable this behavior for agent panes and extend Control unit tests to cover VT-mouse-enabled routing vs. zoom priority.
- Add ItE2E support for physical wheel injection and an E2E + release-checklist case validating Ctrl+wheel zoom + plain wheel chat scrolling.
File summaries
| File | Description |
|---|---|
| test/e2e/tests/Feature.AgentMouse.Tests.ps1 | Adds a physical wheel routing E2E validating plain wheel scroll + Ctrl+wheel zoom in agent panes. |
| test/e2e/README.md | Updates E2E suite table entry and coverage/test-status counts to include the new case. |
| test/e2e/ItE2E/Public/Ui.ps1 | Adds Win32 SendInput-based physical wheel injection helper (Invoke-WtWindowWheel). |
| test/e2e/ItE2E/ItE2E.psm1 | Exports Invoke-WtWindowWheel from the ItE2E module. |
| src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp | Adds a unit test ensuring agent-pane Ctrl+wheel zoom happens before VT mouse wheel routing. |
| src/cascadia/TerminalControl/TermControl.idl | Introduces EnableAgentMouseWheelZoom API to opt TermControl into the new behavior. |
| src/cascadia/TerminalControl/TermControl.h | Stores the per-control agent zoom-priority flag. |
| src/cascadia/TerminalControl/TermControl.cpp | Passes the new flag into interactivity mouse-wheel handling paths. |
| src/cascadia/TerminalControl/ControlInteractivity.h | Adds a MouseWheel(..., bool prioritizeZoom) overload. |
| src/cascadia/TerminalControl/ControlInteractivity.cpp | Implements the zoom-priority fast path ahead of VT mouse routing when enabled. |
| src/cascadia/TerminalApp/AgentPaneContent.cpp | Enables the agent zoom-priority flag on agent-pane TermControls. |
| doc/release-check-list.md | Adds release-checklist case C295 covering physical Ctrl+wheel zoom in agent panes. |
Review details
- Files reviewed: 12/12 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.
Mark the P/Invoke as SetLastError-aware so failed cursor queries report the correct Win32 error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The new physical-wheel E2E path should include the same foreground/interactive-desktop skip gating used elsewhere to avoid flakiness in unattended or focus-contended runs.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
test/e2e/tests/Feature.AgentMouse.Tests.ps1:213
- This physical wheel test currently fails hard when the WT window cannot take foreground (locked desktop / competing foreground app), which can make the suite flaky in unattended runs. Mirror the existing hotkey/physical-input suites by skipping when Test-WtWindowKeyFocusable fails before sending wheel input.
test/e2e/ItE2E/Public/Ui.ps1:124 - Avoid the raw 0x0800 magic number for mouse-wheel injection; define and use a named constant so the SendInput flags are self-describing and less error-prone to extend (e.g., if adding HWHEEL later).
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Lite
Skip the physical-wheel case when foreground focus is unavailable and name the Win32 wheel input flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The updated E2E coverage counts in test/e2e/README.md leave a nearby sentence still referring to the old “147 implemented checklist items,” creating an inconsistency that should be corrected.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
test/e2e/README.md:64
- The updated coverage/test-status counts (148/150 and 128 baseline cases) are followed by a sentence that still says "The 147 implemented checklist items...", which is now inconsistent and can confuse readers tracking coverage.
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Lite
Keep the implemented checklist total consistent with the updated 148-of-150 summary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The behavior change is narrowly gated to agent panes, and the PR adds both unit and physical E2E coverage plus a release-checklist item for the regression.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary of the Pull Request
Makes Ctrl+mouse-wheel zoom text in agent panes before VT mouse tracking can consume the event. Plain and horizontal wheel input still reaches the agent, while ordinary terminal and Ctrl+Shift opacity behavior remain unchanged.
References and Relevant Issues
Closes #790
Detailed Description of the Pull Request / Additional comments
Agent panes run a mouse-tracking TUI, so the existing VT-first wheel route consumed Ctrl+wheel as chat scrolling. Agent TermControls now opt into a vertical Ctrl+wheel priority path. The default route remains unchanged. The PR also adds physical HWND wheel injection to ItE2E and release-checklist case C295.
Validation Steps Performed
PR Checklist