Agent sessions approval row - #298929
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an “approval row” UI to Agent Sessions so users can explicitly confirm pending tool invocations, backed by a new per-session approval-tracking model.
Changes:
- Introduces
AgentSessionApprovalModelto derive per-session approval info (label/language + confirm action) from live chat tool-invocation state. - Updates the agent sessions tree renderer + delegate to render the approval row and adjust row height dynamically.
- Adds component fixtures and a dedicated unit test suite for the approval model; enables the feature in the Sessions window.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/test/browser/componentFixtures/agentSessionsViewer.fixture.ts | Adds visual fixtures covering many session variants, including approval-row examples. |
| src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionApprovalModel.test.ts | Unit tests for approval derivation across tool invocation states and data variants. |
| src/vs/workbench/contrib/chat/browser/agentSessions/media/agentsessionsviewer.css | Styles for the new approval row UI. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.ts | Renders approval row, emits height-change events, and supports dynamic item height. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts | Wires approval model into the sessions tree and updates row height when approval visibility changes. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionApprovalModel.ts | New model that tracks “needs approval” state per session via observables. |
| src/vs/sessions/contrib/sessions/browser/sessionsViewPane.ts | Enables the approval row in the Sessions window. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Copilot i forgot to correctly adjust this test, can you adjust it so it passes again. It should be expecting sh and not bash
AssertionError: {"label":"legacy-cmd","language":"sh"} deepStrictEqual {"label":"legacy-cmd","language":"bash"} |
|
Benjamin Christopher Simmonds (@benibenj) I've opened a new pull request, #298933, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * fix: update test expectation for legacy terminal tool data to expect 'sh' language id Co-authored-by: benibenj <44439583+benibenj@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: benibenj <44439583+benibenj@users.noreply.github.com>
41054e8
into
main
Copilot Generated Description:Introduce an approval row for agent sessions, allowing for user confirmation on tool invocations. This includes updates to the session model and rendering logic to support the new approval feature.