diff --git a/doc/release-check-list.md b/doc/release-check-list.md index 688cb9ab8..dbc8d5e56 100644 --- a/doc/release-check-list.md +++ b/doc/release-check-list.md @@ -233,6 +233,8 @@ Net effect: UT shrinks the manual matrix to "did the wiring and UI connect", not ### Shell integration and detection +- [ ] `C295` `[new]` `[E2E]` **Detected Autofix clicks remain isolated between tabs:** With errors detected in two tabs, clicking one tab's diagnostics button submits exactly one prompt for that tab and preserves the other tab's pending opt-in. _(E2E: `Feature.AutofixRouting`.)_ + - [ ] `C087` `[E2E]` **PowerShell shell integration installed:** Supported PowerShell profiles emit command-finished events, including non-zero marks for PowerShell-level failures on Windows PowerShell 5.1. - [ ] `C219` `[new]` `[E2E]` **Bash / WSL shell integration installed:** Supported bash and WSL-bash profiles emit command-finished events, and the injected `PROMPT_COMMAND` is safe under `set -u` (no errors in strict-mode shells). _(#340.)_ - [ ] `C250` `[new]` `[E2E]` **Bash PROMPT_COMMAND rewrites preserve semantic prompt boundaries:** In Intelligent Terminal, a user hook that rebuilds `PS1` still produces one ordered `OSC 133;D/A/B` cycle per command; the same user-wide integration script stays inert in other terminals. _(#468; E2E: `Feature.BashPromptIntegration`.)_ @@ -465,4 +467,4 @@ Net effect: UT shrinks the manual matrix to "did the wiring and UI connect", not - Slash commands: `tools\wta\src\commands.rs`. - Session state model: `tools\wta\src\agent_sessions.rs`, `tools\wta\AGENTS.md`. - Multi-window agent pane architecture: `doc\specs\Multi-window-agent-pane.md`. -- Autofix flow, logging, and runtime layout: `AGENTS.md`. +- Autofix flow, logging, and runtime layout: `AGENTS.md`. diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index ec84ad1e0..5e6a2be88 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -3884,6 +3884,7 @@ namespace winrt::TerminalApp::implementation openAgentPaneForReview(); Json::Value params; params["pane_id"] = winrt::to_string(paneId); + params["tab_id"] = winrt::to_string(activeTab->StableId()); _RaiseProtocolEvent("autofix_execute_from_detected", params); break; } @@ -8061,6 +8062,32 @@ namespace winrt::TerminalApp::implementation return {}; } + std::string TerminalPage::_FindTabIdForSessionId(const std::string_view sessionId) + { + for (const auto& tab : _tabs) + { + const auto tabImpl = _GetTabImpl(tab); + if (!tabImpl) + { + continue; + } + const auto rootPane = tabImpl->GetRootPane(); + if (!rootPane) + { + continue; + } + const auto match = rootPane->WalkTree([&](const auto& p) -> std::shared_ptr { + const auto control = p->GetTerminalControl(); + return (control && _FindSessionIdForControl(control) == sessionId) ? p : nullptr; + }); + if (match) + { + return winrt::to_string(tabImpl->StableId()); + } + } + return {}; + } + void TerminalPage::_RegisterTerminalEvents(TermControl term) { term.RaiseNotice({ this, &TerminalPage::_ControlNoticeRaisedHandler }); @@ -8093,35 +8120,34 @@ namespace winrt::TerminalApp::implementation // Forward VT sequences and connection state changes to protocol clients. // This is unconditional — if no pipe client is listening, the event raise is a noop. // - // We capture a weak ref to the TermControl and resolve the connection SessionId - // at event-fire time, because at _RegisterTerminalEvents time the Pane hasn't - // been created yet (TermControl is set up before the Pane wraps it). + // Capture the connection SessionId now. It is stable for the control's + // lifetime and lets the background VT callback avoid carrying a + // TermControl weak reference across threads. // // VtSequenceReceived fires on the connection reader thread (background). - // The dispatched continuation calls `_FindTabIdForControl`, which walks + // The dispatched continuation calls `_FindTabIdForSessionId`, which walks // `_tabs` and has UI thread affinity, so the event raise has to run on // the UI thread. `_FindSessionIdForControl` itself is thread-safe - // (only reads `Connection().SessionId()`) and could be called inline, - // but the rest of the work in this handler is gated on `_FindTabIdForControl` - // and the protocol event raise, so we just defer the whole body. + // (only reads `Connection().SessionId()`). The tab lookup and protocol + // event raise remain on the UI thread. { winrt::weak_ref weakTerm{ term }; + const auto paneIdStr = _FindSessionIdForControl(term); term.VtSequenceReceived( - [weakThis = get_weak(), weakTerm](auto&&, const winrt::hstring& seq) { + [weakThis = get_weak(), paneIdStr](auto&&, const winrt::hstring& seq) { auto strongThis = weakThis.get(); - if (!strongThis) + if (!strongThis || paneIdStr.empty()) return; - // Dispatch to UI thread for the `_FindTabIdForControl` walk + // Dispatch to UI thread for the `_FindTabIdForSessionId` walk // of `_tabs` and the protocol event raise. Fire-and-forget — // don't block the connection reader thread. strongThis->Dispatcher().RunAsync( winrt::Windows::UI::Core::CoreDispatcherPriority::Normal, - [weakThis, weakTerm, seq]() { + [weakThis, paneIdStr, seq]() { auto page = weakThis.get(); - auto term2 = weakTerm.get(); - if (!page || !term2) + if (!page) return; // GPO-blocked gate: when administrator policy @@ -8161,10 +8187,7 @@ namespace winrt::TerminalApp::implementation return; } - const auto paneIdStr = page->_FindSessionIdForControl(term2); - if (paneIdStr.empty()) - return; - const auto tabIdStr = page->_FindTabIdForControl(term2); + const auto tabIdStr = page->_FindTabIdForSessionId(paneIdStr); if (isAgentEvent) { @@ -8180,13 +8203,12 @@ namespace winrt::TerminalApp::implementation { const auto eventName = agentParams["event"].asString(); const auto agentSessionId = agentParams.get("agent_session_id", "").asString(); - if (const auto connection = term2.Connection()) + if (const auto paneSessionId = _TryParsePaneSessionId(paneIdStr)) { // This event arrived in-band on this // pane's own VT stream, so the pane is // the origin by construction — there is // no reported `pane_id` to distrust. - const auto paneSessionId = connection.SessionId(); if ((eventName == "agent.session.started" || eventName == "agent.session.start") && !agentSessionId.empty() && !agentSessionId.starts_with("sidekick-")) @@ -8197,7 +8219,7 @@ namespace winrt::TerminalApp::implementation if (!resumeCommandline.empty()) { page->_paneAgentSessions.insert_or_assign( - paneSessionId, + *paneSessionId, _PaneAgentSession{ winrt::to_hstring(agentSessionId), winrt::to_hstring(agentParams.get("cli_source", "").asString()), diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index e2bad3cc1..450a2d41e 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -880,6 +880,7 @@ namespace winrt::TerminalApp::implementation void _RegisterTerminalEvents(Microsoft::Terminal::Control::TermControl term); std::string _FindSessionIdForControl(const Microsoft::Terminal::Control::TermControl& control); std::string _FindTabIdForControl(const Microsoft::Terminal::Control::TermControl& control); + std::string _FindTabIdForSessionId(std::string_view sessionId); void _RegisterTabEvents(Tab& hostingTab); void _DismissTabContextMenus(); diff --git a/test/e2e/ItE2E/Public/Agent.ps1 b/test/e2e/ItE2E/Public/Agent.ps1 index 1edc012fa..a68bc3a25 100644 --- a/test/e2e/ItE2E/Public/Agent.ps1 +++ b/test/e2e/ItE2E/Public/Agent.ps1 @@ -213,11 +213,13 @@ function Wait-TerminalActionProposal { recommendation card directly or first presents the provider's normal permission UI. With -ReturnOnPermission, the latter returns Mode=Permission without selecting an option; the caller must simulate an explicit user choice. + Use -PaneSessionId to pin the rendered MCP card when several tabs have helpers. #> [CmdletBinding()] param( [Parameter(Mandatory, ValueFromPipeline)]$App, [int]$TimeoutSec = 45, - [switch]$ReturnOnPermission + [switch]$ReturnOnPermission, + [string]$PaneSessionId ) process { Wait-Until -TimeoutSec $TimeoutSec -IntervalSec 0.5 -Because 'a pending terminal-action proposal' -Condition { @@ -229,7 +231,7 @@ function Wait-TerminalActionProposal { return Get-CimInstance Win32_Process -Filter "ProcessId = $($candidate.ProcessId)" -ErrorAction SilentlyContinue | Where-Object { $_.CommandLine -match '(?i)(?:^|\s)propose-terminal-actions(?:\s|$)' } } - $paneText = Get-AgentPaneText -App $App -MaxLines 60 + $paneText = Get-AgentPaneText -App $App -MaxLines 60 -PaneSessionId $PaneSessionId if ($paneText -match (Get-RecommendationCardRegex)) { return [pscustomobject]@{ Mode = 'Mcp'; Ready = $true } } diff --git a/test/e2e/README.md b/test/e2e/README.md index 9295ad0d1..ca5e9782d 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -30,8 +30,9 @@ authenticated ACP agents. Current status (run on the Store package): | `Feature.AgentSelectAll.Tests.ps1` | Plain Ctrl+A selects the current WTA-rendered frame; Ctrl+C copies through the existing clipboard path and clears selection without stale replay | 1 | | `Feature.PromptHistory.Tests.ps1` | PR #478: per-tab Up/Down prompt recall, draft restoration, and multiline preservation; PR #614: completed-turn collapse/expand rendering | 4 | | `Feature.CompletedTurnSelection.Tests.ps1` | Completed-turn Tab/Up/Down selection keeps focused history inside the chat viewport | 1 | -| `Feature.AutofixPane.Tests.ps1` | Direct Helper Autofix proposal card render/insert/run/reject/target/stashed + across layout | 10 | +| `Feature.AutofixPane.Tests.ps1` | Direct Helper Autofix proposal card render/insert/run/reject/target/stashed + across layout + WSL shell identity and Linux fixes | 12 (2 WSL-gated) | | `Feature.AutofixParser.Tests.ps1` | issue #474: PowerShell ParserError-to-Autofix pipeline + success/handled-error/blank-input negative controls | 4 | +| `Feature.AutofixRouting.Tests.ps1` | Two Detected tabs: real diagnostics clicks submit only to the selected tab's ACP session and preserve the other tab's opt-in | 1 | | `Feature.CommandResolution.Tests.ps1` | PR #418: packaged WTA resolves PowerShell profile-only aliases to their real targets | 1 | | `Feature.SessionList.Tests.ps1` | session view (button + `/sessions` slash), session states, view switching (incl. draft-preservation), focus/restore | 13 (+1 skip) | | `Feature.NonAsciiCwd.Tests.ps1` | issue #641: a non-ASCII starting directory survives `wtcli` argv → COM → `CreateProcessW`, so the resume launch path connects and starts in that directory | 2 | diff --git a/test/e2e/tests/Feature.AutofixPane.Tests.ps1 b/test/e2e/tests/Feature.AutofixPane.Tests.ps1 index 002993b27..023c06d03 100644 --- a/test/e2e/tests/Feature.AutofixPane.Tests.ps1 +++ b/test/e2e/tests/Feature.AutofixPane.Tests.ps1 @@ -225,7 +225,10 @@ Describe 'Feature: autofix in a WSL pane (OSC 9001;ShellType end-to-end)' -Tag ' # Agent pane on the (now active) WSL tab so autofix cards render here. Open-AgentPane -App $script:app | Out-Null - Wait-AgentReady -App $script:app -TimeoutSec 60 | Should -BeTrue -Because 'the agent pane must be connected for WSL autofix to render cards' + $script:wslAgent = Wait-Until -TimeoutSec 30 -Because 'the WSL tab helper identity' -Condition { + Get-AgentPaneSession -App $script:app -OwnerPaneSessionId $script:wslSid + } + Wait-AgentReady -App $script:app -PaneSessionId $script:wslAgent.PaneSessionId -TimeoutSec 60 | Should -BeTrue -Because 'the agent pane must be connected for WSL autofix to render cards' } catch { Write-ItLog -Level WARN -Message "WSL autofix setup failed (build without WSL-capable CreateTab / OSC 9001, or no WSL shell integration): $_" @@ -257,14 +260,14 @@ Describe 'Feature: autofix in a WSL pane (OSC 9001;ShellType end-to-end)' -Tag ' Invoke-FailingCommand -App $script:app -SessionId $script:wslSid -Command 'sl -la' | Out-Null Wait-Autofix -Listener $listener -TimeoutSec 45 | Out-Null } finally { Stop-WtEventListener -Listener $listener } - $pending = Wait-TerminalActionProposal -App $script:app -TimeoutSec 30 -ReturnOnPermission + $pending = Wait-TerminalActionProposal -App $script:app -PaneSessionId $script:wslAgent.PaneSessionId -TimeoutSec 30 -ReturnOnPermission $pending | Should -Not -BeNullOrEmpty -Because 'WSL Autofix must submit a Direct Helper Proposal' if ($pending.Mode -eq 'Permission') { # Explicit test-user selection of the provider's allow option. - Send-AgentKey -App $script:app -Key Y | Out-Null + Send-AgentKey -App $script:app -PaneSessionId $script:wslAgent.PaneSessionId -Key Y | Out-Null } $cardText = Wait-Until -TimeoutSec 60 -IntervalSec 1 -Because 'a visible WSL Autofix recommendation card' -Condition { - $text = Get-AgentPaneText -App $script:app -MaxLines 60 + $text = Get-AgentPaneText -App $script:app -PaneSessionId $script:wslAgent.PaneSessionId -MaxLines 60 if ($text -match (Get-RecommendationCardRegex)) { $text } } Assert-AI -Claim 'The suggested fix command uses Linux/bash shell syntax (e.g. ls, grep, cat, forward-slash paths). It is NOT a Windows PowerShell command (no Get-ChildItem / Select-String / cmdlet-style Verb-Noun).' -Context $cardText diff --git a/test/e2e/tests/Feature.AutofixParser.Tests.ps1 b/test/e2e/tests/Feature.AutofixParser.Tests.ps1 index eb22d1924..423293770 100644 --- a/test/e2e/tests/Feature.AutofixParser.Tests.ps1 +++ b/test/e2e/tests/Feature.AutofixParser.Tests.ps1 @@ -16,6 +16,25 @@ BeforeDiscovery { ) } +BeforeAll { + function Get-ParserAutofixEvidence { + param($App, [string]$PaneId, [string]$TabId, [string]$AcpSessionId) + + # Pending UI states can be re-emitted without submitting a turn. Count the + # dispatcher handoff and the master's actual receipt of session/prompt instead. + $dispatchPattern = 'autofix: sending auto-fix prompt\s+pane_id=' + + [regex]::Escape($PaneId) + '\s+tab_id=' + [regex]::Escape($TabId) + '(?=\s|$)' + $promptPattern = 'master: forwarding prompt to agent CLI \(non-blocking\).*op="prompt".*session_id=' + + [regex]::Escape(('SessionId("{0}")' -f $AcpSessionId)) + '(?=\s|$)' + [pscustomobject]@{ + Dispatches = @((Get-ItLogText -App $App -Name 'wta-main_helper-*.log' -SinceStart) -split '\r?\n' | + Where-Object { $_ -match $dispatchPattern }) + Prompts = @((Get-ItLogText -App $App -Name 'wta-main_master.log' -SinceStart) -split '\r?\n' | + Where-Object { $_ -match $promptPattern }) + } + } +} + Describe 'Feature: PowerShell parser errors trigger Autofix end-to-end' -Tag 'Feature' -Skip:(-not $script:Ready) { BeforeAll { Import-Module (Join-Path $PSScriptRoot '..\ItE2E\ItE2E.psd1') -Force @@ -27,7 +46,12 @@ Describe 'Feature: PowerShell parser errors trigger Autofix end-to-end' -Tag 'Fe Wait-AgentReady -App $script:app -TimeoutSec 60 | Should -BeTrue -Because 'Autofix requires a connected ACP session' $script:sid = (Get-ActivePane -App $script:app).session_id + $tabId = Resolve-AgentOwnerTabId -App $script:app -OwnerPaneSessionId $script:sid + $agent = Wait-NewAgentPaneSession -App $script:app -TabId $tabId + $agent.AcpSessionId | Should -Not -BeNullOrEmpty + $script:scope = @{ App = $script:app; PaneId = $script:sid; TabId = $tabId; AcpSessionId = $agent.AcpSessionId } } + BeforeEach { Initialize-LogOffsets -App $script:app | Out-Null } AfterAll { if ($script:app) { Stop-Terminal -App $script:app } } It 'PowerShell parser errors trigger exactly one Autofix prompt' { @@ -40,17 +64,17 @@ Describe 'Feature: PowerShell parser errors trigger Autofix end-to-end' -Tag 'Fe "$($failure.params.sequence)" | Should -Match '(?i)osc:133;D;(?!0(\b|;|$))' -Because 'the parser error must be corrected from stale exit code 0' - $autofix = Wait-WtEvent -Listener $listener -TimeoutSec 45 -Predicate { - $_.method -eq 'agent_event' -and - "$($_.params.payload.initial_prompt)" -match 'command failed|Diagnose the error' + "$($failure.params.tab_id)" | Should -Be $script:scope.TabId + $autofix = Wait-Until -TimeoutSec 45 -IntervalSec 0.4 -Because 'the parser Autofix prompt to cross the helper/master ACP boundary' -Condition { + $evidence = Get-ParserAutofixEvidence @script:scope + if ($evidence.Dispatches.Count -gt 0 -and $evidence.Prompts.Count -gt 0) { $evidence } } - $autofix | Should -Not -BeNullOrEmpty -Because 'the parser failure mark must reach the Autofix dispatcher' + $autofix | Should -Not -BeNullOrEmpty -Because 'a local dispatcher log alone does not prove ACP submission' Start-Sleep -Seconds 2 - @(Get-WtEvents -Listener $listener -Predicate { - $_.method -eq 'agent_event' -and - "$($_.params.payload.initial_prompt)" -match 'command failed|Diagnose the error' - }) | Should -HaveCount 1 -Because 'one malformed command must submit one Autofix turn' + $evidence = Get-ParserAutofixEvidence @script:scope + $evidence.Dispatches | Should -HaveCount 1 -Because 'one malformed command must dispatch one Autofix turn' + $evidence.Prompts | Should -HaveCount 1 -Because 'exactly one prompt must reach the owning ACP session' } finally { Stop-WtEventListener -Listener $listener } } @@ -60,6 +84,11 @@ Describe 'Feature: PowerShell parser errors trigger Autofix end-to-end' -Tag 'Fe try { Start-Sleep -Milliseconds 400 Send-WtKeys -App $script:app -SessionId $script:sid -Keys @('Enter') | Out-Null + Wait-WtEvent -Listener $listener -TimeoutSec 20 -Predicate { + $_.method -eq 'vt_sequence' -and + "$($_.params.pane_id)" -eq "$script:sid" -and + "$($_.params.sequence)" -match '(?i)osc:133;A(\b|;|$)' + } | Out-Null Start-Sleep -Seconds 3 @(Get-WtEvents -Listener $listener -Predicate { @@ -67,10 +96,9 @@ Describe 'Feature: PowerShell parser errors trigger Autofix end-to-end' -Tag 'Fe "$($_.params.pane_id)" -eq "$script:sid" -and "$($_.params.sequence)" -match '(?i)osc:133;D;(?!0(\b|;|$))' }) | Should -BeNullOrEmpty -Because 'blank input must not replay the previous parser failure' - @(Get-WtEvents -Listener $listener -Predicate { - $_.method -eq 'agent_event' -and - "$($_.params.payload.initial_prompt)" -match 'command failed|Diagnose the error' - }) | Should -BeNullOrEmpty -Because 'a prompt redraw must not submit another Autofix turn' + $evidence = Get-ParserAutofixEvidence @script:scope + $evidence.Dispatches | Should -BeNullOrEmpty -Because 'a prompt redraw must not dispatch another Autofix turn' + $evidence.Prompts | Should -BeNullOrEmpty -Because 'a prompt redraw must not submit another ACP prompt' } finally { Stop-WtEventListener -Listener $listener } } @@ -87,7 +115,12 @@ Describe 'Feature: successful PowerShell completion does not trigger Autofix' -T Wait-AgentReady -App $script:app -TimeoutSec 60 | Should -BeTrue -Because 'negative assertions require a connected Autofix pipeline' $script:sid = (Get-ActivePane -App $script:app).session_id + $tabId = Resolve-AgentOwnerTabId -App $script:app -OwnerPaneSessionId $script:sid + $agent = Wait-NewAgentPaneSession -App $script:app -TabId $tabId + $agent.AcpSessionId | Should -Not -BeNullOrEmpty + $script:scope = @{ App = $script:app; PaneId = $script:sid; TabId = $tabId; AcpSessionId = $agent.AcpSessionId } } + BeforeEach { Initialize-LogOffsets -App $script:app | Out-Null } AfterAll { if ($script:app) { Stop-Terminal -App $script:app } } It 'Successful PowerShell commands do not trigger Autofix' { @@ -101,16 +134,15 @@ Describe 'Feature: successful PowerShell completion does not trigger Autofix' -T "$($_.params.sequence)" -match '(?i)osc:133;D;0(\b|;|$)' } } | Should -Not -Throw Start-Sleep -Seconds 2 - @(Get-WtEvents -Listener $listener -Predicate { - $_.method -eq 'agent_event' -and - "$($_.params.payload.initial_prompt)" -match 'command failed|Diagnose the error' - }) | Should -BeNullOrEmpty + $evidence = Get-ParserAutofixEvidence @script:scope + $evidence.Dispatches | Should -BeNullOrEmpty -Because 'successful commands must not dispatch Autofix' + $evidence.Prompts | Should -BeNullOrEmpty -Because 'successful commands must not submit an ACP prompt' } finally { Stop-WtEventListener -Listener $listener } } It 'Handled non-terminating PowerShell errors do not trigger Autofix' { - $missingPath = Join-Path $env:TEMP "it-autofix-parser-missing-$([guid]::NewGuid())" + $missingPath = Join-Path $PSScriptRoot "it-autofix-parser-missing-$([guid]::NewGuid())" $escapedPath = $missingPath.Replace("'", "''") $listener = Start-WtEventListener -App $script:app try { @@ -122,10 +154,9 @@ Describe 'Feature: successful PowerShell completion does not trigger Autofix' -T "$($_.params.sequence)" -match '(?i)osc:133;D;0(\b|;|$)' } } | Should -Not -Throw Start-Sleep -Seconds 2 - @(Get-WtEvents -Listener $listener -Predicate { - $_.method -eq 'agent_event' -and - "$($_.params.payload.initial_prompt)" -match 'command failed|Diagnose the error' - }) | Should -BeNullOrEmpty -Because 'handled errors that finish successfully must remain distinct from parser failures' + $evidence = Get-ParserAutofixEvidence @script:scope + $evidence.Dispatches | Should -BeNullOrEmpty -Because 'handled errors that finish successfully must remain distinct from parser failures' + $evidence.Prompts | Should -BeNullOrEmpty -Because 'handled errors must not submit an ACP prompt' } finally { Stop-WtEventListener -Listener $listener } } diff --git a/test/e2e/tests/Feature.AutofixRouting.Tests.ps1 b/test/e2e/tests/Feature.AutofixRouting.Tests.ps1 new file mode 100644 index 000000000..ddf73f3c2 --- /dev/null +++ b/test/e2e/tests/Feature.AutofixRouting.Tests.ps1 @@ -0,0 +1,84 @@ +#Requires -Modules @{ ModuleName='Pester'; ModuleVersion='5.0.0' } +# Real failed commands and the diagnostics button cross WT -> helper -> master +# -> ACP. The fixture records requests without consuming provider quota. + +Describe 'Feature: Autofix detected action routing' -Tag 'Feature' { + BeforeAll { + Import-Module (Join-Path $PSScriptRoot '..\ItE2E\ItE2E.psd1') -Force + $fixture = (Resolve-Path (Join-Path $PSScriptRoot '..\fixtures\Mock-AcpInteractionAgent.ps1')).Path + $script:requestLog = Join-Path $env:TEMP ("ite2e-autofix-routing-{0}.log" -f [guid]::NewGuid().ToString('N')) + New-Item -ItemType File -Path $script:requestLog | Out-Null + $invocation = "& '$($fixture.Replace("'", "''"))' -LogPath '$($script:requestLog.Replace("'", "''"))'" + $encoded = [Convert]::ToBase64String([Text.Encoding]::Unicode.GetBytes($invocation)) + $command = "pwsh -NoProfile -EncodedCommand $encoded" + $script:app = Start-Terminal -Package (Get-ItTestPackage) -PassFre $true -Settings @{ + acpAgent = 'custom:autofix-routing-fixture' + acpCustomCommand = $command + acpModel = '' + autoErrorDetectionEnabled = $true + autoFixEnabled = $false + } + $script:targets = @() + foreach ($label in @('A', 'B')) { + $shell = New-WtTab -App $script:app -Command 'pwsh.exe -NoLogo -NoExit' -Title "autofix-routing-$label" + Set-WtPaneFocus -App $script:app -SessionId $shell.session_id + Open-AgentPane -App $script:app | Out-Null + $agent = Wait-Until -TimeoutSec 30 -Because "helper for tab $label" -Condition { + Get-AgentPaneSession -App $script:app -OwnerPaneSessionId $shell.session_id + } + Wait-AgentReady -App $script:app -PaneSessionId $agent.PaneSessionId -TimeoutSec 60 | + Should -BeTrue + $script:targets += [pscustomobject]@{ Shell = $shell; Agent = $agent } + } + + function Get-RoutingPromptCount($Target) { + $pattern = '\|session/prompt\|' + [regex]::Escape($Target.Agent.AcpSessionId) + '\|' + @(Select-String -LiteralPath $script:requestLog -Pattern $pattern).Count + } + } + AfterAll { + if ($script:app) { Stop-Terminal -App $script:app } + if ($script:requestLog -and (Test-Path -LiteralPath $script:requestLog)) { + Remove-Item -LiteralPath $script:requestLog + } + } + + It 'Detected Autofix clicks remain isolated between tabs' { + Initialize-LogOffsets -App $script:app | Out-Null + foreach ($target in $script:targets) { + Set-WtPaneFocus -App $script:app -SessionId $target.Shell.session_id + $listener = Start-WtEventListener -App $script:app + try { + Invoke-RunCommand -App $script:app -SessionId $target.Shell.session_id -Command "throw 'routing-$([guid]::NewGuid())'" | Out-Null + Wait-WtCommandFailure -Listener $listener -PaneId $target.Shell.session_id -TimeoutSec 20 | Out-Null + Wait-Until -TimeoutSec 20 -Because 'failure reaches this helper in Detected state' -Condition { + (Get-ItLogText -App $script:app -Name "wta-main_helper-$($target.Agent.HelperProcessId).log" -SinceStart) -match + ('surfacing Detected pill.*pane_id=' + [regex]::Escape($target.Shell.session_id)) + } | Out-Null + } + finally { Stop-WtEventListener -Listener $listener } + } + $a, $b = $script:targets + (Get-RoutingPromptCount $a) | Should -Be 0 + (Get-RoutingPromptCount $b) | Should -Be 0 + + # Both tabs are Detected. Clicking only B must leave A available for a + # separate opt-in, even though the protocol broadcasts to both helpers. + Invoke-UiElement -App $script:app -Selector 'DiagnosticsButton' | Out-Null + Wait-Until -TimeoutSec 30 -Because 'B submits its Autofix prompt to ACP' -Condition { + (Get-RoutingPromptCount $b) -gt 0 + } | Out-Null + Start-Sleep -Seconds 3 + (Get-RoutingPromptCount $b) | Should -Be 1 + (Get-RoutingPromptCount $a) | Should -Be 0 -Because 'another tab must not opt in to Autofix' + + Set-WtPaneFocus -App $script:app -SessionId $a.Shell.session_id + Invoke-UiElement -App $script:app -Selector 'DiagnosticsButton' | Out-Null + Wait-Until -TimeoutSec 30 -Because 'A retained its Detected state until explicitly clicked' -Condition { + (Get-RoutingPromptCount $a) -gt 0 + } | Out-Null + Start-Sleep -Seconds 3 + (Get-RoutingPromptCount $a) | Should -Be 1 + (Get-RoutingPromptCount $b) | Should -Be 1 + } +} diff --git a/tools/wta/src/app/autofix.rs b/tools/wta/src/app/autofix.rs index 629151a18..56d2ec75f 100644 --- a/tools/wta/src/app/autofix.rs +++ b/tools/wta/src/app/autofix.rs @@ -397,18 +397,39 @@ impl App { /// active tab's cached snapshot, synthesize a `WtNotification` from /// it, and replay through `trigger_autofix_inner` with `forced=true` /// so the auto-suggest off gate is bypassed and the LLM call fires. - pub(super) fn handle_autofix_execute_from_detected(&mut self) { + pub(super) fn handle_autofix_execute_from_detected( + &mut self, + requested_pane_id: &str, + requested_tab_id: Option<&str>, + ) { let active_tab = self.active_tab_key().to_string(); + if requested_pane_id.is_empty() + || requested_tab_id.is_some_and(|tab| tab != active_tab) + || self + .owner_tab_id + .as_deref() + .is_some_and(|owner| owner != active_tab) + { + tracing::debug!( + target: "autofix", + requested_pane_id, + requested_tab_id, + active_tab, + "ignoring detected Autofix action: target is missing or tab does not match" + ); + return; + } let snapshot = self.current_tab().autofix.bar_snapshot.clone(); let (pane_id, summary) = match snapshot { AutofixBarSnapshot::Detected { pane_id, summary, .. - } => (pane_id, summary), + } if pane_id == requested_pane_id => (pane_id, summary), other => { tracing::info!( target: "autofix", + requested_pane_id, state = ?other, - "autofix_execute_from_detected: bar not in Detected state — ignoring", + "autofix_execute_from_detected: no matching Detected pane — ignoring", ); return; } diff --git a/tools/wta/src/app_events.rs b/tools/wta/src/app_events.rs index 4763b9e3d..32113e02d 100644 --- a/tools/wta/src/app_events.rs +++ b/tools/wta/src/app_events.rs @@ -2283,7 +2283,7 @@ impl App { // User pressed the pill / hotkey in Detected state. // Replay the trigger as if auto-suggest were on, so // the LLM call fires and we transition to Pending. - self.handle_autofix_execute_from_detected(); + self.handle_autofix_execute_from_detected(&pane_id, tab_id.as_deref()); return; } diff --git a/tools/wta/src/autofix_tests.rs b/tools/wta/src/autofix_tests.rs index 0ff8e3143..73d9364ef 100644 --- a/tools/wta/src/autofix_tests.rs +++ b/tools/wta/src/autofix_tests.rs @@ -108,6 +108,89 @@ fn suggestion_off_emits_detected_without_submitting_turn() { ); } +fn detected_helper(tab: &str, pane: &str) -> App { + let mut app = test_app(); + app.state = ConnectionState::Connected; + app.autofix_enabled = false; + app.owner_tab_id = Some(tab.to_string()); + app.tab_id = Some(tab.to_string()); + app.maybe_trigger_autofix(&failure_notification(pane, Some(tab))); + assert!(matches!( + app.current_tab().autofix.bar_snapshot, + AutofixBarSnapshot::Detected { .. } + )); + app +} + +fn detected_action(pane: &str, tab: Option<&str>) -> AppEvent { + AppEvent::WtEvent { + method: "autofix_execute_from_detected".to_string(), + pane_id: pane.to_string(), + tab_id: tab.map(str::to_string), + params: serde_json::json!({}), + } +} + +#[test] +fn detected_action_only_submits_on_the_target_helper() { + let mut a = detected_helper("tab-a", "pane-a"); + let mut b = detected_helper("tab-b", "pane-b"); + for app in [&mut a, &mut b] { + app.handle_event(detected_action("pane-b", Some("tab-b"))); + } + assert!(a.current_tab().turn.is_idle()); + assert!(matches!( + a.current_tab().autofix.bar_snapshot, + AutofixBarSnapshot::Detected { .. } + )); + assert!(!b.current_tab().turn.is_idle()); + assert_eq!(b.current_tab().autofix.pane_id.as_deref(), Some("pane-b")); + let generation = b.current_tab().autofix.generation; + b.handle_event(detected_action("pane-b", Some("tab-b"))); + assert_eq!(b.current_tab().autofix.generation, generation); +} + +#[test] +fn detected_action_rejects_missing_stale_and_cross_tab_targets() { + for (pane, tab) in [ + ("", Some("tab-a")), + ("", None), + ("pane-old-split", Some("tab-a")), + ("pane-a", Some("tab-b")), + ("pane-a", Some("")), + ] { + let mut app = detected_helper("tab-a", "pane-a"); + app.handle_event(detected_action(pane, tab)); + assert!(app.current_tab().turn.is_idle(), "{pane:?} {tab:?}"); + assert!(matches!( + app.current_tab().autofix.bar_snapshot, + AutofixBarSnapshot::Detected { .. } + )); + } +} + +#[test] +fn legacy_detected_action_without_tab_still_requires_matching_pane() { + let mut a = detected_helper("tab-a", "pane-a"); + let mut b = detected_helper("tab-b", "pane-b"); + a.handle_event(detected_action("pane-b", None)); + b.handle_event(detected_action("pane-b", None)); + assert!(a.current_tab().turn.is_idle()); + assert!(!b.current_tab().turn.is_idle()); +} + +#[test] +fn detected_action_does_not_replay_after_source_pane_closes() { + let mut app = detected_helper("tab-a", "pane-a"); + app.handle_event(closed_event("pane-a", "tab-a")); + app.handle_event(detected_action("pane-a", Some("tab-a"))); + assert!(app.current_tab().turn.is_idle()); + assert!(matches!( + app.current_tab().autofix.bar_snapshot, + AutofixBarSnapshot::Idle + )); +} + /// Single-flight, same pane: re-triggering autofix for the *same* failing pane /// while a turn is already in flight must re-emit the bar state only — it must /// not bump the generation or submit a second turn (the agent is already