fix clarify replay and compression timeout (#1044)
This commit is contained in:
@@ -5,6 +5,7 @@
|
|||||||
"ts": "node -r ts-node/register"
|
"ts": "node -r ts-node/register"
|
||||||
},
|
},
|
||||||
"env": {
|
"env": {
|
||||||
|
"NODE_ENV": "development",
|
||||||
"TS_NODE_PROJECT": "packages/server/tsconfig.json",
|
"TS_NODE_PROJECT": "packages/server/tsconfig.json",
|
||||||
"HERMES_WEB_UI_STOP_GATEWAYS_ON_SHUTDOWN": "0"
|
"HERMES_WEB_UI_STOP_GATEWAYS_ON_SHUTDOWN": "0"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1313,7 +1313,6 @@ async function handleSessionModelCustomSubmit() {
|
|||||||
v-model:value="clarifyResponse"
|
v-model:value="clarifyResponse"
|
||||||
size="small"
|
size="small"
|
||||||
:placeholder="t('chat.clarifyPlaceholder')"
|
:placeholder="t('chat.clarifyPlaceholder')"
|
||||||
@keyup.enter="handleClarify()"
|
|
||||||
/>
|
/>
|
||||||
<NButton size="small" type="primary" @click="handleClarify()">
|
<NButton size="small" type="primary" @click="handleClarify()">
|
||||||
{{ t('chat.clarifySubmit') }}
|
{{ t('chat.clarifySubmit') }}
|
||||||
|
|||||||
@@ -52,7 +52,7 @@ export interface CompressionConfig {
|
|||||||
headMessageCount: number
|
headMessageCount: number
|
||||||
/** Number of recent messages to keep verbatim (default: 10) */
|
/** Number of recent messages to keep verbatim (default: 10) */
|
||||||
tailMessageCount: number
|
tailMessageCount: number
|
||||||
/** Timeout for LLM summarization call (default: 60_000ms) */
|
/** Timeout for LLM summarization call (default: 300_000ms) */
|
||||||
summarizationTimeoutMs: number
|
summarizationTimeoutMs: number
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -61,7 +61,7 @@ export const DEFAULT_COMPRESSION_CONFIG: CompressionConfig = {
|
|||||||
summaryBudget: 8_000,
|
summaryBudget: 8_000,
|
||||||
headMessageCount: 0,
|
headMessageCount: 0,
|
||||||
tailMessageCount: 10,
|
tailMessageCount: 10,
|
||||||
summarizationTimeoutMs: 120_000,
|
summarizationTimeoutMs: 300_000,
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface CompressedResult {
|
export interface CompressedResult {
|
||||||
|
|||||||
@@ -2332,10 +2332,11 @@ class WorkerProcess:
|
|||||||
except OSError:
|
except OSError:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
def request(self, req: dict[str, Any]) -> dict[str, Any]:
|
def request(self, req: dict[str, Any], timeout: float | None = None) -> dict[str, Any]:
|
||||||
self.start()
|
self.start()
|
||||||
self.last_used_at = time.time()
|
self.last_used_at = time.time()
|
||||||
return _send_bridge_request(self.endpoint, req, self.REQUEST_TIMEOUT_SECONDS)
|
request_timeout = timeout if timeout is not None and timeout > 0 else self.REQUEST_TIMEOUT_SECONDS
|
||||||
|
return _send_bridge_request(self.endpoint, req, request_timeout)
|
||||||
|
|
||||||
|
|
||||||
def _worker_endpoint(key: str) -> str:
|
def _worker_endpoint(key: str) -> str:
|
||||||
@@ -2648,10 +2649,19 @@ class BridgeBroker:
|
|||||||
forwarded = dict(req)
|
forwarded = dict(req)
|
||||||
forwarded["profile"] = profile
|
forwarded["profile"] = profile
|
||||||
forwarded.pop("worker_key", None)
|
forwarded.pop("worker_key", None)
|
||||||
resp = worker.request(forwarded)
|
resp = worker.request(forwarded, self._worker_request_timeout(req))
|
||||||
self._record_response_routes(profile, key, resp)
|
self._record_response_routes(profile, key, resp)
|
||||||
return resp
|
return resp
|
||||||
|
|
||||||
|
def _worker_request_timeout(self, req: dict[str, Any]) -> float:
|
||||||
|
try:
|
||||||
|
timeout = float(req.get("timeout", 0) or 0)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
timeout = 0
|
||||||
|
if timeout <= 0:
|
||||||
|
return WorkerProcess.REQUEST_TIMEOUT_SECONDS
|
||||||
|
return max(WorkerProcess.REQUEST_TIMEOUT_SECONDS, timeout + 10)
|
||||||
|
|
||||||
def handle(self, req: dict[str, Any]) -> dict[str, Any]:
|
def handle(self, req: dict[str, Any]) -> dict[str, Any]:
|
||||||
action = str(req.get("action") or "").strip()
|
action = str(req.get("action") or "").strip()
|
||||||
if not action:
|
if not action:
|
||||||
|
|||||||
@@ -250,6 +250,7 @@ export class ChatRunSocket {
|
|||||||
|
|
||||||
socket.on('clarify.respond', async (data: { session_id?: string; clarify_id?: string; response?: string }) => {
|
socket.on('clarify.respond', async (data: { session_id?: string; clarify_id?: string; response?: string }) => {
|
||||||
if (!data.session_id || !data.clarify_id) return
|
if (!data.session_id || !data.clarify_id) return
|
||||||
|
this.clearClarifyEventState(data.session_id, data.clarify_id)
|
||||||
try {
|
try {
|
||||||
const result = await this.bridge.clarifyRespond(data.clarify_id, data.response || '')
|
const result = await this.bridge.clarifyRespond(data.clarify_id, data.response || '')
|
||||||
this.emitToSession(socket, data.session_id, 'clarify.resolved', {
|
this.emitToSession(socket, data.session_id, 'clarify.resolved', {
|
||||||
@@ -386,6 +387,19 @@ export class ChatRunSocket {
|
|||||||
|
|
||||||
// --- Helpers ---
|
// --- Helpers ---
|
||||||
|
|
||||||
|
private clearClarifyEventState(sessionId: string, clarifyId: string) {
|
||||||
|
const state = this.sessionMap.get(sessionId)
|
||||||
|
if (!state?.events.length) return
|
||||||
|
|
||||||
|
const nextEvents = state.events.filter(({ event, data }) => {
|
||||||
|
if (event !== 'clarify.requested' && event !== 'clarify.resolved') return true
|
||||||
|
return data?.clarify_id !== clarifyId
|
||||||
|
})
|
||||||
|
if (nextEvents.length !== state.events.length) {
|
||||||
|
state.events = nextEvents
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private emitToSession(socket: Socket, sessionId: string, event: string, payload: any) {
|
private emitToSession(socket: Socket, sessionId: string, event: string, payload: any) {
|
||||||
const tagged = { ...payload, session_id: sessionId }
|
const tagged = { ...payload, session_id: sessionId }
|
||||||
this.nsp.to(`session:${sessionId}`).emit(event, tagged)
|
this.nsp.to(`session:${sessionId}`).emit(event, tagged)
|
||||||
|
|||||||
@@ -406,7 +406,7 @@ class RoutedWorker:
|
|||||||
self.requests = []
|
self.requests = []
|
||||||
self.stopped = False
|
self.stopped = False
|
||||||
|
|
||||||
def request(self, req):
|
def request(self, req, timeout=None):
|
||||||
self.requests.append(req)
|
self.requests.append(req)
|
||||||
action = req.get("action")
|
action = req.get("action")
|
||||||
if action == "chat":
|
if action == "chat":
|
||||||
@@ -634,6 +634,37 @@ assert not thread.is_alive(), blocking_conn.response
|
|||||||
blocked_resp = json.loads(blocking_conn.response.decode("utf-8"))
|
blocked_resp = json.loads(blocking_conn.response.decode("utf-8"))
|
||||||
assert blocked_resp["ok"] is True, blocked_resp
|
assert blocked_resp["ok"] is True, blocked_resp
|
||||||
assert blocked_resp["blocked"] is True, blocked_resp
|
assert blocked_resp["blocked"] is True, blocked_resp
|
||||||
|
`)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('extends profile worker request timeout from wait requests', () => {
|
||||||
|
runPython(String.raw`
|
||||||
|
${harness}
|
||||||
|
|
||||||
|
broker = bridge.BridgeBroker("ipc:///tmp/unused.sock")
|
||||||
|
assert broker._worker_request_timeout({"action": "chat"}) == bridge.WorkerProcess.REQUEST_TIMEOUT_SECONDS
|
||||||
|
assert broker._worker_request_timeout({"action": "chat", "timeout": 60}) == bridge.WorkerProcess.REQUEST_TIMEOUT_SECONDS
|
||||||
|
assert broker._worker_request_timeout({"action": "chat", "timeout": 300}) == 310
|
||||||
|
|
||||||
|
captured = {}
|
||||||
|
worker = bridge.WorkerProcess("default", "default", "ipc:///tmp/worker.sock", None, None)
|
||||||
|
worker.start = lambda: None
|
||||||
|
original_send = bridge._send_bridge_request
|
||||||
|
try:
|
||||||
|
def fake_send(endpoint, req, timeout):
|
||||||
|
captured["endpoint"] = endpoint
|
||||||
|
captured["req"] = req
|
||||||
|
captured["timeout"] = timeout
|
||||||
|
return {"ok": True}
|
||||||
|
bridge._send_bridge_request = fake_send
|
||||||
|
response = worker.request({"action": "chat"}, 310)
|
||||||
|
finally:
|
||||||
|
bridge._send_bridge_request = original_send
|
||||||
|
|
||||||
|
assert response["ok"] is True, response
|
||||||
|
assert captured["endpoint"] == "ipc:///tmp/worker.sock", captured
|
||||||
|
assert captured["req"] == {"action": "chat"}, captured
|
||||||
|
assert captured["timeout"] == 310, captured
|
||||||
`)
|
`)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -90,6 +90,48 @@ describe('ChatRunSocket clarify responses', () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('does not replay answered clarify prompts when the session resumes', async () => {
|
||||||
|
bridgeMock.clarifyRespond.mockResolvedValue({ ok: true, resolved: true })
|
||||||
|
const { ChatRunSocket } = await import('../../packages/server/src/services/hermes/run-chat')
|
||||||
|
const { handlers, io, socket } = createSocketHarness()
|
||||||
|
const server = new ChatRunSocket(io as any)
|
||||||
|
const toolEvent = {
|
||||||
|
event: 'tool.started',
|
||||||
|
data: { event: 'tool.started', tool_call_id: 'tool-1' },
|
||||||
|
}
|
||||||
|
;(server as any).sessionMap.set('session-1', {
|
||||||
|
messages: [],
|
||||||
|
isWorking: true,
|
||||||
|
events: [
|
||||||
|
{
|
||||||
|
event: 'clarify.requested',
|
||||||
|
data: {
|
||||||
|
event: 'clarify.requested',
|
||||||
|
clarify_id: 'clarify-1',
|
||||||
|
question: 'Pick one',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
toolEvent,
|
||||||
|
],
|
||||||
|
queue: [],
|
||||||
|
})
|
||||||
|
|
||||||
|
;(server as any).onConnection(socket)
|
||||||
|
await handlers.get('clarify.respond')?.({
|
||||||
|
session_id: 'session-1',
|
||||||
|
clarify_id: 'clarify-1',
|
||||||
|
response: 'Use option A',
|
||||||
|
})
|
||||||
|
await handlers.get('resume')?.({ session_id: 'session-1' })
|
||||||
|
|
||||||
|
expect((server as any).sessionMap.get('session-1').events).toEqual([toolEvent])
|
||||||
|
expect(socket.emit).toHaveBeenCalledWith('resumed', expect.objectContaining({
|
||||||
|
session_id: 'session-1',
|
||||||
|
isWorking: true,
|
||||||
|
events: [toolEvent],
|
||||||
|
}))
|
||||||
|
})
|
||||||
|
|
||||||
it('emits an unresolved clarify result when the bridge rejects the response', async () => {
|
it('emits an unresolved clarify result when the bridge rejects the response', async () => {
|
||||||
bridgeMock.clarifyRespond.mockRejectedValue(new Error('unknown clarify request'))
|
bridgeMock.clarifyRespond.mockRejectedValue(new Error('unknown clarify request'))
|
||||||
const { ChatRunSocket } = await import('../../packages/server/src/services/hermes/run-chat')
|
const { ChatRunSocket } = await import('../../packages/server/src/services/hermes/run-chat')
|
||||||
|
|||||||
Reference in New Issue
Block a user