Revert "修复审批请求在聊天中无提示且无法响应 (#467)" (#553)

This reverts commit 56c7b59eaf.
This commit is contained in:
ekko
2026-05-09 08:36:13 +08:00
committed by GitHub
parent 56c7b59eaf
commit 9045f2a987
12 changed files with 9 additions and 767 deletions
-24
View File
@@ -1,24 +0,0 @@
import { describe, expect, it } from 'vitest'
import { isApprovalCommand, parseApprovalCommand } from '../../packages/client/src/utils/approval-commands'
describe('approval command parsing', () => {
it('maps slash commands to the upstream run approval choices', () => {
expect(parseApprovalCommand('/approve')).toEqual({ choice: 'once', all: false })
expect(parseApprovalCommand('/approve session')).toEqual({ choice: 'session', all: false })
expect(parseApprovalCommand('/approve always')).toEqual({ choice: 'always', all: false })
expect(parseApprovalCommand('/deny')).toEqual({ choice: 'deny', all: false })
})
it('keeps all as resolve-all, not as always allow', () => {
expect(parseApprovalCommand(' /approve all ')).toEqual({ choice: 'once', all: true })
expect(parseApprovalCommand('/DENY ALL')).toEqual({ choice: 'deny', all: true })
})
it('rejects ordinary chat text and malformed approval-like commands', () => {
expect(parseApprovalCommand('approve')).toBeNull()
expect(parseApprovalCommand('/approve please')).toBeNull()
expect(parseApprovalCommand('/deny session')).toBeNull()
expect(parseApprovalCommand('/always')).toBeNull()
expect(isApprovalCommand('hello')).toBe(false)
})
})
-120
View File
@@ -1,120 +0,0 @@
// @vitest-environment jsdom
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { mount } from '@vue/test-utils'
const mockChatStore = vi.hoisted(() => ({
isStreaming: true,
isAborting: false,
activeSession: null as any,
setAutoPlaySpeech: vi.fn(),
sendMessage: vi.fn(),
stopStreaming: vi.fn(),
}))
vi.mock('../../packages/client/src/stores/hermes/chat', () => ({
useChatStore: () => mockChatStore,
}))
vi.mock('../../packages/client/src/stores/hermes/app', () => ({
useAppStore: () => ({ selectedModel: 'test-model' }),
}))
vi.mock('../../packages/client/src/stores/hermes/profiles', () => ({
useProfilesStore: () => ({ activeProfileName: 'default' }),
}))
vi.mock('../../packages/client/src/api/hermes/sessions', () => ({
fetchContextLength: vi.fn(() => Promise.resolve(200000)),
}))
vi.mock('vue-i18n', () => ({
useI18n: () => ({ t: (key: string) => key }),
}))
vi.mock('naive-ui', async (importActual) => {
const actual = await importActual<typeof import('naive-ui')>()
return {
...actual,
useMessage: () => ({
error: vi.fn(),
success: vi.fn(),
}),
}
})
import ChatInput from '../../packages/client/src/components/hermes/chat/ChatInput.vue'
function mountInput() {
return mount(ChatInput, {
global: {
stubs: {
NButton: {
props: ['disabled'],
emits: ['click'],
template: '<button :disabled="disabled" @click="$emit(\'click\')"><slot name="icon"/><slot /></button>',
},
NTooltip: {
template: '<div><slot name="trigger"/><slot /></div>',
},
NSwitch: {
props: ['value'],
emits: ['update:value'],
template: '<input type="checkbox" />',
},
},
},
})
}
describe('ChatInput approval commands while streaming', () => {
beforeEach(() => {
vi.clearAllMocks()
mockChatStore.isStreaming = true
mockChatStore.isAborting = false
})
it('allows /approve to be submitted while a run is streaming', async () => {
const wrapper = mountInput()
const textarea = wrapper.find('textarea')
await textarea.setValue('/approve')
const buttons = wrapper.findAll('button')
const sendButton = buttons[buttons.length - 1]
expect(sendButton.attributes('disabled')).toBeUndefined()
await sendButton.trigger('click')
expect(mockChatStore.sendMessage).toHaveBeenCalledWith('/approve', undefined)
expect((textarea.element as HTMLTextAreaElement).value).toBe('')
})
it('allows ordinary messages while streaming so the run queue can handle them', async () => {
const wrapper = mountInput()
const textarea = wrapper.find('textarea')
await textarea.setValue('hello while busy')
const buttons = wrapper.findAll('button')
const sendButton = buttons[buttons.length - 1]
expect(sendButton.attributes('disabled')).toBeUndefined()
await textarea.trigger('keydown', { key: 'Enter' })
expect(mockChatStore.sendMessage).toHaveBeenCalledWith('hello while busy', undefined)
expect((textarea.element as HTMLTextAreaElement).value).toBe('')
})
it('allows session/always/all approval commands while streaming', async () => {
const wrapper = mountInput()
const textarea = wrapper.find('textarea')
await textarea.setValue('/approve session')
await textarea.trigger('keydown', { key: 'Enter' })
expect(mockChatStore.sendMessage).toHaveBeenLastCalledWith('/approve session', undefined)
await textarea.setValue('/approve always')
await textarea.trigger('keydown', { key: 'Enter' })
expect(mockChatStore.sendMessage).toHaveBeenLastCalledWith('/approve always', undefined)
await textarea.setValue('/deny all')
await textarea.trigger('keydown', { key: 'Enter' })
expect(mockChatStore.sendMessage).toHaveBeenLastCalledWith('/deny all', undefined)
})
})
-130
View File
@@ -1,130 +0,0 @@
// @vitest-environment jsdom
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { createPinia, setActivePinia } from 'pinia'
const chatApiMocks = vi.hoisted(() => ({
lastRunEventHandler: null as null | ((evt: any) => void),
startRunViaSocket: vi.fn((_payload: any, onEvent: (evt: any) => void) => {
chatApiMocks.lastRunEventHandler = onEvent
return { abort: vi.fn() }
}),
resumeSession: vi.fn((sessionId: string, onResumed: (data: any) => void) => {
onResumed({ session_id: sessionId, messages: [], isWorking: false, events: [] })
return {} as any
}),
registerSessionHandlers: vi.fn(() => vi.fn()),
unregisterSessionHandlers: vi.fn(),
getChatRunSocket: vi.fn(() => ({ emit: vi.fn() })),
submitApprovalViaSocket: vi.fn(),
}))
vi.mock('@/api/hermes/chat', () => chatApiMocks)
vi.mock('@/api/hermes/sessions', () => ({
deleteSession: vi.fn(),
fetchSession: vi.fn(),
fetchSessions: vi.fn(() => Promise.resolve({ sessions: [] })),
}))
vi.mock('@/api/client', () => ({
getApiKey: vi.fn(() => ''),
}))
import { useChatStore } from '@/stores/hermes/chat'
describe('chat store approval commands', () => {
beforeEach(() => {
setActivePinia(createPinia())
vi.clearAllMocks()
})
it('submits /approve through the active streaming run instead of starting a new run', async () => {
const store = useChatStore()
store.newChat()
const sessionId = store.activeSessionId!
await store.sendMessage('start risky work')
expect(chatApiMocks.startRunViaSocket).toHaveBeenCalledTimes(1)
expect(store.isStreaming).toBe(true)
await store.sendMessage('/approve session')
expect(chatApiMocks.startRunViaSocket).toHaveBeenCalledTimes(1)
expect(chatApiMocks.submitApprovalViaSocket).toHaveBeenCalledWith(sessionId, 'session', false)
expect(store.messages.at(-1)?.role).toBe('user')
expect(store.messages.at(-1)?.content).toBe('/approve session')
})
it('queues ordinary chat text while a run is streaming', async () => {
const store = useChatStore()
store.newChat()
await store.sendMessage('start risky work')
expect(store.isStreaming).toBe(true)
await store.sendMessage('this should queue behind the active run')
expect(chatApiMocks.startRunViaSocket).toHaveBeenCalledTimes(2)
expect(chatApiMocks.startRunViaSocket.mock.calls[1][0]).toMatchObject({
input: 'this should queue behind the active run',
session_id: store.activeSessionId,
})
expect(chatApiMocks.startRunViaSocket.mock.calls[1][0].queue_id).toEqual(expect.any(String))
expect(chatApiMocks.submitApprovalViaSocket).not.toHaveBeenCalled()
expect(store.messages.map(m => m.content)).not.toContain('this should queue behind the active run')
})
it('deduplicates repeated approval request pattern labels', async () => {
const store = useChatStore()
store.newChat()
await store.sendMessage('start risky work')
const onEvent = chatApiMocks.lastRunEventHandler
expect(onEvent).toEqual(expect.any(Function))
onEvent?.({
event: 'approval.request',
run_id: 'run-approval-1',
command: "bash -lc 'printf ok'",
description: 'shell command via -c/-lc flag',
pattern_key: 'shell command via -c/-lc flag',
pattern_keys: ['shell command via -c/-lc flag'],
choices: ['once', 'session', 'always', 'deny'],
})
const approvalPrompt = store.messages.find(m =>
m.role === 'system' && m.content.includes('Approval required'),
)
expect(approvalPrompt?.content).toContain('Patterns: shell command via -c/-lc flag')
expect(approvalPrompt?.content).not.toContain('shell command via -c/-lc flag, shell command via -c/-lc flag')
})
it('deduplicates the optimistic and upstream approval response for the same run', async () => {
const store = useChatStore()
store.newChat()
await store.sendMessage('start risky work')
const onEvent = chatApiMocks.lastRunEventHandler
expect(onEvent).toEqual(expect.any(Function))
onEvent?.({
event: 'approval.responded',
run_id: 'run-approval-1',
choice: 'once',
all: false,
resolved: 1,
})
onEvent?.({
event: 'approval.responded',
run_id: 'run-approval-1',
timestamp: Date.now() / 1000,
choice: 'once',
resolved: 1,
})
const approvalResponses = store.messages.filter(m =>
m.role === 'system' && m.content.includes('Approval approved once. Resolved 1 pending request.'),
)
expect(approvalResponses).toHaveLength(1)
})
})
@@ -48,23 +48,6 @@ describe('MessageItem tool details', () => {
})
})
it('renders system approval prompt content in the chat bubble', () => {
const wrapper = mount(MessageItem, {
props: {
message: {
id: 'approval-request',
role: 'system',
content: 'Approval required\n\nCommand:\n```\nbash -lc \'printf ok\'\n```\n\nReply with /approve to approve once.',
timestamp: Date.now(),
} satisfies Message,
},
})
expect(wrapper.text()).toContain('Approval required')
expect(wrapper.text()).toContain('Reply with /approve')
expect(wrapper.find('.message-bubble.system').exists()).toBe(true)
})
it('renders highlighted code blocks for tool arguments and tool results', async () => {
const wrapper = mount(MessageItem, {
props: {
@@ -1,102 +0,0 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { ChatRunSocket } from '../../packages/server/src/services/hermes/chat-run-socket'
function makeChatRunSocket() {
const emit = vi.fn()
const nsp = {
use: vi.fn(),
on: vi.fn(),
to: vi.fn(() => ({ emit })),
emit,
adapter: { rooms: new Map() },
}
const io = { of: vi.fn(() => nsp) }
const gatewayManager = {
getUpstream: vi.fn(() => 'http://127.0.0.1:9999'),
getApiKey: vi.fn(() => 'test-key'),
}
return new ChatRunSocket(io as any, gatewayManager)
}
describe('ChatRunSocket approval event normalization', () => {
beforeEach(() => {
vi.unstubAllGlobals()
})
it('normalizes upstream approval_required status into canonical approval.request for clients', () => {
const service = makeChatRunSocket() as any
const event = service.normalizeApprovalRequest({
event: 'tool.completed',
status: 'approval_required',
command: 'rm -rf /tmp/example',
description: 'dangerous command',
pattern_key: 'rm_rf',
}, 'run-123')
expect(event).toMatchObject({
event: 'approval.request',
run_id: 'run-123',
command: 'rm -rf /tmp/example',
description: 'dangerous command',
pattern_key: 'rm_rf',
choices: ['once', 'session', 'always', 'deny'],
})
})
it('passes through upstream approval.request choices and pattern_keys', () => {
const service = makeChatRunSocket() as any
const event = service.normalizeApprovalRequest({
event: 'approval.request',
run_id: 'run-456',
command: 'git reset --hard HEAD',
pattern_keys: ['git_reset_hard'],
choices: ['once', 'session', 'always', 'deny'],
}, 'ignored')
expect(event).toMatchObject({
event: 'approval.request',
run_id: 'run-456',
pattern_keys: ['git_reset_hard'],
choices: ['once', 'session', 'always', 'deny'],
})
})
it('posts approval responses to the upstream run-scoped approval endpoint after capability check', async () => {
const service = makeChatRunSocket() as any
service.sessionMap.set('session-1', {
messages: [],
isWorking: true,
events: [],
runId: 'run-123',
profile: 'default',
})
const fetchMock = vi.fn()
.mockResolvedValueOnce({
ok: true,
json: async () => ({
features: { approval_events: true, run_approval_response: true },
endpoints: { run_approval: { method: 'POST', path: '/v1/runs/{run_id}/approval' } },
}),
})
.mockResolvedValueOnce({
ok: true,
json: async () => ({ resolved: 1, choice: 'session' }),
})
vi.stubGlobal('fetch', fetchMock)
await service.handleApprovalRespond({ connected: true, emit: vi.fn() }, 'session-1', 'session', false)
expect(fetchMock).toHaveBeenCalledTimes(2)
expect(fetchMock.mock.calls[0][0]).toBe('http://127.0.0.1:9999/v1/capabilities')
expect(fetchMock.mock.calls[1][0]).toBe('http://127.0.0.1:9999/v1/runs/run-123/approval')
expect(JSON.parse(fetchMock.mock.calls[1][1].body)).toEqual({ choice: 'session', all: false })
})
it('ignores ordinary upstream events', () => {
const service = makeChatRunSocket() as any
expect(service.normalizeApprovalRequest({ event: 'tool.completed', status: 'done' }, 'run-123')).toBeNull()
})
})
+1 -3
View File
@@ -2,14 +2,12 @@
* Tests for session-sync service
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest'
import { getDb } from '../../packages/server/src/db/index'
import { initAllStores } from '../../packages/server/src/db/hermes/init'
import { getDb, ensureTable } from '../../packages/server/src/db/index'
import { syncAllHermesSessionsOnStartup } from '../../packages/server/src/services/hermes/session-sync'
describe('session-sync', () => {
beforeEach(() => {
// Reset database before each test
initAllStores()
const db = getDb()
if (db) {
db.exec('DELETE FROM sessions')