diff --git a/python/src/agent_v2/router.py b/python/src/agent_v2/router.py index f614c73..b32e470 100644 --- a/python/src/agent_v2/router.py +++ b/python/src/agent_v2/router.py @@ -28,6 +28,7 @@ from contextlib import suppress from datetime import UTC, date, datetime from pathlib import Path +from typing import Literal from fastapi import FastAPI, HTTPException, Request from pydantic import BaseModel, Field @@ -170,7 +171,7 @@ class ChatRequestV2(BaseModel): class ApproveRequest(BaseModel): - decision: str = "allow_once" # allow_once, allow_session, deny + decision: Literal["allow_once", "allow_session", "deny"] = "allow_once" reason: str | None = None diff --git a/python/src/agent_v2/runtime/conversation.py b/python/src/agent_v2/runtime/conversation.py index 41ecfc8..2c4610d 100644 --- a/python/src/agent_v2/runtime/conversation.py +++ b/python/src/agent_v2/runtime/conversation.py @@ -371,6 +371,8 @@ async def turn( def approve(self, event_id: str, decision: str) -> bool: """Handle approval decision from frontend. Returns True if event was found.""" + if decision not in {"allow_once", "allow_session", "deny"}: + return False evt = self._approval_events.get(event_id) if evt is None: return False diff --git a/python/tests/agent_v2/test_approval_flow.py b/python/tests/agent_v2/test_approval_flow.py index a06b02d..9e99678 100644 --- a/python/tests/agent_v2/test_approval_flow.py +++ b/python/tests/agent_v2/test_approval_flow.py @@ -657,6 +657,23 @@ async def test_approve_nonexistent_event(self, workspace: Path): ) assert not rt.approve("nonexistent_id", "allow_once") + @pytest.mark.asyncio + async def test_approve_rejects_unknown_decision_without_unblocking(self, workspace: Path): + registry = create_default_registry(workspace_root=workspace) + policy = policy_from_registry(PermissionMode.WORKSPACE_WRITE, registry.permission_specs()) + rt = ConversationRuntime( + provider=MockProvider(), + tool_registry=registry, + permission_policy=policy, + session=Session(workspace=str(workspace)), + ) + approval_event = asyncio.Event() + rt._approval_events["evt_001"] = approval_event + + assert not rt.approve("evt_001", "always_allow") + assert not approval_event.is_set() + assert "evt_001" not in rt._approval_decisions + @pytest.mark.asyncio async def test_abort_unblocks_approval(self, workspace: Path): provider = MockProvider( diff --git a/python/tests/integration/test_download_approve_e2e.py b/python/tests/integration/test_download_approve_e2e.py index 243672a..ee9ad65 100644 --- a/python/tests/integration/test_download_approve_e2e.py +++ b/python/tests/integration/test_download_approve_e2e.py @@ -248,6 +248,14 @@ def test_approve_reject_decision(self, client, agent_app_with_session): assert resp.status_code == 200 assert resp.json()["status"] == "ok" + def test_approve_rejects_unknown_decision(self, agent_app_with_session): + session_id, event_id, tc = agent_app_with_session + resp = tc.post( + f"/api/agent/v2/approve/{session_id}/{event_id}", + json={"decision": "always_allow"}, + ) + assert resp.status_code == 422 + # ── Fixture for approve happy path ─────────────────────────────────────── diff --git a/src/__tests__/AgentApprovalInline.test.ts b/src/__tests__/AgentApprovalInline.test.ts index 326ea1a..a854109 100644 --- a/src/__tests__/AgentApprovalInline.test.ts +++ b/src/__tests__/AgentApprovalInline.test.ts @@ -25,6 +25,7 @@ import AgentApprovalInline from '../components/AgentApprovalInline.vue' describe('AgentApprovalInline', () => { const basePending = { event_id: 'evt_1', + session_id: 'sess_1', tool_name: 'write_file', args: { file_path: 'draft.md' }, risk: 'destructive', @@ -62,4 +63,18 @@ describe('AgentApprovalInline', () => { expect(wrapper.text()).toContain('高风险') expect(wrapper.find('.approval-risk').classes()).toContain('risk-destructive') }) + + it.each([ + ['.allow-once', 'allow_once'], + ['.allow-session', 'allow_session'], + ['.deny', 'deny'], + ])('emits the decision when %s is clicked', async (selector, decision) => { + const wrapper = mount(AgentApprovalInline, { + props: { pending: basePending }, + }) + + await wrapper.get(selector).trigger('click') + + expect(wrapper.emitted('decide')).toEqual([[decision]]) + }) }) diff --git a/src/__tests__/AiPanel.test.ts b/src/__tests__/AiPanel.test.ts index 842c3af..eb92f15 100644 --- a/src/__tests__/AiPanel.test.ts +++ b/src/__tests__/AiPanel.test.ts @@ -33,11 +33,17 @@ vi.mock('vue-i18n', () => ({ })) import AiPanel from '../components/AiPanel.vue' +import { currentWorkspaceGrant } from '../composables/useProject' +import { useFileTree } from '../composables/useFileTree' +import { _resetForTesting } from '../composables/useAgentChat' describe('AiPanel workflow routing', () => { const fetchMock = vi.fn() beforeEach(() => { + _resetForTesting() + currentWorkspaceGrant.value = 'grant-ai-panel' + useFileTree().rootDir.value = 'D:\\paper' fetchMock.mockReset() fetchMock.mockResolvedValue({ ok: true, @@ -80,5 +86,51 @@ describe('AiPanel workflow routing', () => { await flushPromises() expect(String(fetchMock.mock.calls[0][0])).toMatch(/\/api\/agent\/v2\/chat$/) + const payload = JSON.parse(fetchMock.mock.calls[0][1].body) + expect(payload).toMatchObject({ + workspace_root: 'D:\\paper', + workspace_grant: 'grant-ai-panel', + }) + }) + + it('submits a rendered approval with its owner session and workspace grant', async () => { + readSseStream.mockImplementationOnce( + async (_reader: unknown, handler: (type: string, data: Record) => void) => { + handler('session_started', { metadata: { session_id: 'sess_ai_panel' } }) + handler('await_approval', { + event_id: 'evt_ai_panel', + metadata: { + tool_name: 'write_file', + args: { file_path: 'draft.md' }, + reason: 'Confirm the edit', + }, + }) + }, + ) + + const wrapper = mount(AiPanel, { + props: { editorContext: 'Editor context', workspaceFiles: [] }, + }) + await wrapper.find('textarea').setValue('Update the project') + await wrapper.find('.ac-send-btn').trigger('click') + await flushPromises() + + expect(wrapper.find('.approval-bar').exists()).toBe(true) + await wrapper.find('.allow-once').trigger('click') + await flushPromises() + + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(fetchMock.mock.calls[1]).toEqual([ + '/api/agent/v2/approve/sess_ai_panel/evt_ai_panel', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ + 'Content-Type': 'application/json', + 'X-Workspace-Grant': 'grant-ai-panel', + }), + body: JSON.stringify({ decision: 'allow_once' }), + }), + ]) + expect(wrapper.find('.approval-bar').exists()).toBe(false) }) }) diff --git a/src/__tests__/useAgentChat.test.ts b/src/__tests__/useAgentChat.test.ts index 2dd6b41..0b5a49b 100644 --- a/src/__tests__/useAgentChat.test.ts +++ b/src/__tests__/useAgentChat.test.ts @@ -677,6 +677,61 @@ describe('useAgentChat', () => { expect(approvalEvents.length).toBeGreaterThan(0) }) + it('keeps the approval bound to the session that raised it', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + makeSseResponse([ + makeSessionStartedChunk('sess_approval_owner'), + makeAwaitApprovalChunk('write_file', 'Confirm the edit', 'evt_owned'), + makeDoneChunk(), + ]), + ) + .mockResolvedValueOnce(new Response('{}', { status: 200 })) + .mockResolvedValueOnce( + makeSseResponse([makeSessionStartedChunk('sess_approval_owner'), makeDoneChunk()]), + ) + vi.stubGlobal('fetch', fetchMock) + + const { sendMessage, sendApproval, activeRunSessionId, pendingApproval } = useAgentChat() + await sendMessage('Update the draft') + + expect(activeRunSessionId.value).toBeNull() + expect(pendingApproval.value).toMatchObject({ + event_id: 'evt_owned', + session_id: 'sess_approval_owner', + }) + expect(await sendApproval('evt_owned', 'allow_once')).toBe(true) + expect(fetchMock).toHaveBeenLastCalledWith( + 'http://127.0.0.1:18088/api/agent/v2/approve/sess_approval_owner/evt_owned', + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({ decision: 'allow_once' }), + }), + ) + + await sendMessage('Continue the task') + expect(pendingApproval.value).toBeNull() + }) + + it.each(['allow_once', 'allow_session', 'deny'] as const)( + 'routes %s from the editor overlay to its explicit session owner', + async (decision) => { + const fetchMock = vi.fn().mockResolvedValue(new Response('{}', { status: 200 })) + vi.stubGlobal('fetch', fetchMock) + + const { sendApproval } = useAgentChat() + expect(await sendApproval('evt_inline', decision, undefined, 'sess_inline')).toBe(true) + expect(fetchMock).toHaveBeenCalledWith( + 'http://127.0.0.1:18088/api/agent/v2/approve/sess_inline/evt_inline', + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({ decision }), + }), + ) + }, + ) + it('does not let an older approval_received event clear a newer tool approval', async () => { const openStream = makeOpenSseResponse([ makeSessionStartedChunk('sess_overlap'), diff --git a/src/components/AgentApprovalInline.vue b/src/components/AgentApprovalInline.vue index 24ddb9a..e507709 100644 --- a/src/components/AgentApprovalInline.vue +++ b/src/components/AgentApprovalInline.vue @@ -17,24 +17,31 @@
{{ t('agent.confirmEach') }} - diff --git a/src/components/AgentPanel.vue b/src/components/AgentPanel.vue index 69f3b02..3b249a6 100644 --- a/src/components/AgentPanel.vue +++ b/src/components/AgentPanel.vue @@ -782,7 +782,6 @@ const { abortSession, startNewWorkflow, loadWorkflowMessages, - activeRunSessionId, pendingCheckpoint, fetchSessions: _fetchSessions, fetchAgentSkills, @@ -1090,7 +1089,10 @@ const showApprovalFallback = computed(() => async function handleApprovalDecision(decision: 'allow_once' | 'allow_session' | 'deny') { const pending = pendingApproval.value if (!pending) return - await sendApproval(pending.event_id, decision) + const accepted = await sendApproval(pending.event_id, decision) + if (!accepted) { + showWarning(t('agent.approvalSubmitFailed'), 8000) + } } // Route file-edit approvals to inline diff editor overlay @@ -1101,7 +1103,7 @@ watch(pendingApproval, (p) => { setActiveEdit({ editId: p.event_id, eventId: p.event_id, - sessionId: activeRunSessionId.value || '', + sessionId: p.session_id, operation: (p.tool_name === 'write_file' ? 'write_file' : 'str_replace') as 'str_replace' | 'write_file', filePath: (args?.file_path as string) || '', diff --git a/src/components/AiPanel.vue b/src/components/AiPanel.vue index 632b22b..0a5dd40 100644 --- a/src/components/AiPanel.vue +++ b/src/components/AiPanel.vue @@ -407,8 +407,9 @@ import { } from '../composables/useAiPanelState' import { API_BASE } from '../utils/api' import AgentApprovalInline from './AgentApprovalInline.vue' -import type { PendingApproval } from '../composables/useAgentChat' +import { useAgentChat, type PendingApproval } from '../composables/useAgentChat' import { useFileTree } from '../composables/useFileTree' +import { currentWorkspaceGrant } from '../composables/useProject' import { useEditorState } from '../composables/useEditorState' import { useEditor } from '../composables/useEditor' import { useSpeechRecognition } from '../composables/useSpeechRecognition' @@ -474,6 +475,7 @@ const copiedId = ref(null) let copiedTimer: ReturnType | null = null const acSessionId = ref(null) const pendingApproval = ref(null) +const { sendApproval: sendAgentApproval } = useAgentChat() const { rootDir, refresh: refreshFileTree } = useFileTree() const { tabs: editorTabs, setActiveEdit, clearActiveEdit } = useEditorState() const { reloadOpenTabs, applyExternalFileUpdate } = useEditor() @@ -817,6 +819,7 @@ async function doSend(text: string) { context_text: props.editorContext?.trim() || undefined, context_file: props.activeFile?.trim() || undefined, workspace_root: rootDir.value?.trim() || undefined, + workspace_grant: currentWorkspaceGrant.value || undefined, }), signal: aiAbortCtrl.value.signal, }) @@ -866,6 +869,7 @@ async function doSend(text: string) { } else if (evtType === 'await_approval') { pendingApproval.value = { event_id: (d.event_id as string) || '', + session_id: acSessionId.value || '', tool_name: (meta?.tool_name as string) || (meta?.tool as string) || '', args: (meta?.args ?? meta?.arguments) as Record | undefined, risk: meta?.risk as string | undefined, @@ -960,19 +964,19 @@ function stopStream() { } async function handleApprovalDecision(decision: 'allow_once' | 'allow_session' | 'deny') { - const sid = acSessionId.value - const eventId = pendingApproval.value?.event_id - if (!sid || !eventId) return - pendingApproval.value = null - clearActiveEdit() - try { - await fetch(`${API}/api/agent/v2/approve/${sid}/${eventId}`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ decision }), - }) - } catch { - /* non-fatal */ + const pending = pendingApproval.value + if (!pending) return + const accepted = await sendAgentApproval( + pending.event_id, + decision, + undefined, + pending.session_id, + ) + if (accepted) { + pendingApproval.value = null + clearActiveEdit() + } else { + showWarning(t('agent.approvalSubmitFailed'), 8000) } } diff --git a/src/components/MonacoEditor.vue b/src/components/MonacoEditor.vue index 4caf1ac..0f34bbf 100644 --- a/src/components/MonacoEditor.vue +++ b/src/components/MonacoEditor.vue @@ -755,7 +755,7 @@ watch(activeEdit, (edit) => { function _dispatchInlineDecision(decision: 'allow_once' | 'deny') { const edit = activeEdit.value if (!edit) return - sendApproval(edit.eventId, decision).then((ok) => { + sendApproval(edit.eventId, decision, undefined, edit.sessionId).then((ok) => { if (ok) clearActiveEdit() // On failure, widget stays visible for retry }) diff --git a/src/composables/useAgentChat.ts b/src/composables/useAgentChat.ts index 6c4f266..31acd8a 100644 --- a/src/composables/useAgentChat.ts +++ b/src/composables/useAgentChat.ts @@ -135,6 +135,8 @@ const pendingCheckpoint = ref(null) export interface PendingApproval { event_id: string + /** Immutable owner captured from session_started; never route via mutable global state. */ + session_id: string tool_name: string args?: Record risk?: string @@ -178,17 +180,19 @@ export function _resetForTesting(): void { /** Agent chat composable (singleton). Manages ReAct loop SSE streaming, session lifecycle, per-session approval state, and RAG documents. */ export function useAgentChat() { - function _setApproval(value: PendingApproval | null) { - const sid = activeRunSessionId.value - if (sid) _approvalBySession.set(sid, value) + function _setApproval(value: PendingApproval) { + if (value.session_id) _approvalBySession.set(value.session_id, value) pendingApproval.value = value } function _clearApproval(eventId?: string) { // Approval events can overlap in a multi-tool turn. A late HTTP response or // approval_received event for tool A must never clear tool B's newer card. - if (eventId && pendingApproval.value?.event_id !== eventId) return - _setApproval(null) + const current = pendingApproval.value + if (eventId && current?.event_id !== eventId) return + const ownerSessionId = current?.session_id || activeRunSessionId.value + if (ownerSessionId) _approvalBySession.set(ownerSessionId, null) + pendingApproval.value = null } // ── Shared SSE event handler ────────────────────────────────────── @@ -297,6 +301,7 @@ export function useAgentChat() { case 'await_approval': _setApproval({ event_id: agentEvent.event_id || '', + session_id: activeRunSessionId.value || '', tool_name: (agentEvent.metadata?.tool_name as string) || (agentEvent.metadata?.tool as string) || @@ -588,9 +593,18 @@ export function useAgentChat() { eventId: string, decision: 'allow_once' | 'allow_session' | 'deny', reason?: string, + sessionId?: string, ): Promise { - const sid = activeRunSessionId.value - if (!sid || !eventId) return false + const approvalOwner = + pendingApproval.value?.event_id === eventId ? pendingApproval.value.session_id : null + const sid = sessionId || approvalOwner || activeRunSessionId.value + if (!sid || !eventId) { + logger.warn('sendApproval skipped: approval routing identity is missing', { + eventId, + hasActiveSession: Boolean(activeRunSessionId.value), + }) + return false + } try { const resp = await fetch(`${API_URL}/api/agent/v2/approve/${sid}/${eventId}`, { @@ -607,6 +621,16 @@ export function useAgentChat() { _clearApproval(eventId) return true } + const detail = await resp + .json() + .then((body) => body?.detail) + .catch(() => undefined) + logger.warn('sendApproval rejected by backend', { + sessionId: sid, + eventId, + status: resp.status, + detail, + }) } catch (e) { logger.warn('sendApproval failed', { error: e }) } diff --git a/src/i18n/locales/en-US.json b/src/i18n/locales/en-US.json index b44f35a..8edbd35 100644 --- a/src/i18n/locales/en-US.json +++ b/src/i18n/locales/en-US.json @@ -322,6 +322,7 @@ }, "agent": { "approvalRequest": "Agent requests permission to run", + "approvalSubmitFailed": "The approval was not delivered because the task may be disconnected or expired. Retry, or stop and resend the task.", "approvalEditReason": "Agent will edit {path}", "userDeniedChange": "The change to {path} was denied", "approvalTimedOutChange": "Approval timed out for {path}", diff --git a/src/i18n/locales/zh-CN.json b/src/i18n/locales/zh-CN.json index c540065..16e1842 100644 --- a/src/i18n/locales/zh-CN.json +++ b/src/i18n/locales/zh-CN.json @@ -325,6 +325,7 @@ }, "agent": { "approvalRequest": "Agent 请求执行", + "approvalSubmitFailed": "审批未送达，任务可能已断开或过期。请重试；若仍失败，请停止任务后重新发送。", "approvalEditReason": "Agent 将修改 {path}", "userDeniedChange": "用户已拒绝修改 {path}", "approvalTimedOutChange": "修改审批已超时：{path}",