Expand agent integration coverage and fix discovered regressions - #656
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
There was a problem hiding this comment.
Pull request overview
Expands Intelligent Terminal’s agent-focused E2E coverage by adding deterministic ACP fixtures and new end-to-end protocol experience tests, while tightening existing suites (prompt history, autofix, model lifecycle/sync, BYOK) and updating documentation/release checklist coverage to reflect the new automated cases.
Changes:
- Add a new deterministic ACP “protocol experience” E2E suite plus a stdio interaction agent fixture to validate transcripts/tools, clarification requests, session config, model title confirmation, tab action direction hints, and
/newsession replacement behavior. - Extend BYOK E2E to validate Credential Manager API key resolution (including authorization header verification via the OpenAI-compatible mock server).
- Reduce flakiness in existing E2E tests (prompt history uniqueness size, autofix timing, picker/title assertions) and document new automated checklist items.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/tests/Feature.PromptHistory.Tests.ps1 | Shortens per-test GUID markers to reduce UI/test fragility while keeping uniqueness. |
| test/e2e/tests/Feature.ByokProvider.Tests.ps1 | Adds Credential Manager injection via P/Invoke and asserts BYOK auth headers are passed end-to-end. |
| test/e2e/tests/Feature.AutofixPane.Tests.ps1 | Adjusts autofix waits/dismiss loop to reduce timing-related flakes. |
| test/e2e/tests/Feature.AgentProtocolExperience.Tests.ps1 | New end-to-end ACP protocol experience suite using a deterministic stdio fixture. |
| test/e2e/tests/Feature.AgentPopup.Tests.ps1 | Adds substring-filtering coverage for slash command search ranking/filtering. |
| test/e2e/tests/Feature.AgentModelSync.Tests.ps1 | Updates model picker assertion strategy for ACP model synchronization validation. |
| test/e2e/tests/Feature.AgentModelLifecycle.Tests.ps1 | Simplifies settings-driven model restart/reconnect coverage using the model-switch fixture. |
| test/e2e/README.md | Documents the new protocol experience suite in the E2E suite list. |
| test/e2e/fixtures/Mock-OpenAIChatServer.ps1 | Extends mock server to validate expected Authorization bearer token. |
| test/e2e/fixtures/Mock-AcpModelSwitchAgent.ps1 | Enhances fixture to track and report the current model via config option updates/results. |
| test/e2e/fixtures/Mock-AcpInteractionAgent.ps1 | New deterministic ACP interaction agent fixture (tools, input modal, config, MCP calls, session lifecycle). |
| doc/release-check-list.md | Adds new automated E2E/UT checklist items C278–C284 tied to the added coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
test/e2e/tests/Feature.AgentProtocolExperience.Tests.ps1:21
- $command embeds $script:fixture and $script:requestLog without quoting/escaping. If either path contains spaces or special characters, WTA’s process launch will fail and the test will be flaky. Prefer quoting the -File and -LogPath arguments (or using an encoded command like other fixtures do).
$command = "pwsh -NoProfile -File $script:fixture -LogPath $script:requestLog"
test/e2e/tests/Feature.AgentModelSync.Tests.ps1:37
- Hard-coding '(?m)^┌Model' makes the test depend on an English literal that may differ under localization and deviates from existing ItE2E conventions for localized UI assertions. Use Get-WtaLocalizedTextRegex('model_picker.title') (with the existing en-US fallback) to assert the picker is open, as done elsewhere (e.g. test/e2e/tests/Feature.AgentPaneInteraction.Tests.ps1:135-137).
Assert-AgentPaneText -App $script:app -Pattern '(?m)^┌Model' -TimeoutSec 10
Preserve and restore plugin registration when an active Copilot process locks the installed plugin directory. Keep FRE and OpenCode integration coverage aligned with the product installer, and make full-suite report capture robust to leaked Pester output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
test/e2e/tests/Feature.AgentModelSync.Tests.ps1:37
- This assertion hard-codes an English "Model" title using box-drawing characters, which makes the test locale-dependent and inconsistent with the surrounding E2E convention of matching localized strings via
Get-WtaLocalizedTextRegex(see Feature.AgentPaneInteraction.Tests.ps1).
Assert-AgentPaneText -App $script:app -Pattern '(?m)^┌Model' -TimeoutSec 10
Preserve click history when focusing the input so double-click word selection works. Harden physical-input integration tests against foreground loss, stale cross-test focus state, text-rectangle overshoot, and ambiguous redock actions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
test/e2e/tests/Feature.ByokProvider.Tests.ps1:65
- This test writes an API key into Windows Credential Manager with
Persist = 2(local-machine persistence). If the test run is aborted before cleanup, the credential can be left behind persistently. Using session persistence reduces the risk of leaving stray credentials on developer machines while still exercising CredWrite/CredRead.
Consider switching to Persist = 1 (session) for this fixture credential.
CredentialBlob = handle.AddrOfPinnedObject(),
Persist = 2,
UserName = "Intelligent Terminal"
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
tools/wta/src/agent_hooks_installer.rs:2388
preserve_copilot_registrationreturnsfalseon read/parse failures without recording anything inmessages, which makes locked-directory uninstall paths much harder to diagnose (and can silently prevent later restore/retry behavior). Consider pushing an explicit message for these failure cases before returning.
This issue also appears on line 2433 of the same file.
let Ok(text) = fs::read_to_string(&config_path) else {
return false;
};
let Ok(config) = serde_json::from_str::<Value>(&strip_jsonc_line_comments(&text)) else {
return false;
tools/wta/src/agent_hooks_installer.rs:2437
restore_deferred_copilot_registrationbails out early on deferred-file read/parse errors without adding any diagnostic tomessages, so a failed restore after a locked uninstall can be silent. Adding a message on these failure paths would make troubleshooting and telemetry much clearer.
let Ok(deferred_text) = fs::read_to_string(&deferred_path) else {
return false;
};
let Ok(entry) = serde_json::from_str::<Value>(&deferred_text) else {
return false;
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tools/wta/src/agent_hooks_installer.rs:2487
restore_deferred_copilot_registrationwill write whatever JSON is present in the deferred snapshot into~/.copilot/config.jsonwithout validating that it actually refers to the expected Copilot plugin (PLUGIN_NAME/MARKETPLACE_NAME). If the deferred file is stale/corrupted (or manually edited), this can inject an unrelated entry into Copilot’s config.
Validate the snapshot’s name and marketplace before applying it (and consider deleting the deferred file if it doesn’t match) to keep the restore path bounded to the intended plugin.
let deferred_path = copilot_deferred_registration_path(home);
let Ok(deferred_text) = fs::read_to_string(&deferred_path) else {
return false;
};
let Ok(entry) = serde_json::from_str::<Value>(&deferred_text) else {
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 786d81b1-3d9e-49fa-8d4f-8b375389dff9
Brings in #658 (cargo fmt across WTA) and #656 (agent integration coverage). Replayed turns keep this branch's expanded rendering, and session_registry.rs keeps main's CRLF line endings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b7f01b22-494a-42a1-8600-fbbf089fce4a
Summary
/newreplacement, popup filtering, model lifecycle, prompt history, autofix, WSL shell context, and BYOK Credential Manager resolutionProduct regression attribution
The only confirmed product regression found by the full integration investigation was input-dialog double-click word selection. It was introduced by #624 when input-focus restoration cleared the multi-click record on the first MouseUp. This PR fixes that path and adds a focused Rust regression test.
The other investigated interaction failures were test geometry, foreground ownership, inherited navigation state, or an incomplete redock oracle rather than additional product regressions.
Scope
Hook installer behavior and hook-specific tests are intentionally unchanged from current
mainand are outside this PR.Validation
Full-suite investigation
The last complete 255-test run before the focused fixes reported 226 passed / 17 failed / 12 skipped. Every failure was investigated: one pre-existing product regression is fixed here, the interaction/redock test defects were corrected, legacy oracle/state cases were isolated, and three OpenCode live-event cases remain dependent on an external endpoint that returned service unavailable. Focused suites covering the changed paths pass; a newer full 255-test report has not been claimed.