From 1d89c17ef605832dd270cd8533e291794c3f08ff Mon Sep 17 00:00:00 2001 From: Haitao Pan Date: Wed, 22 Apr 2026 16:55:06 +0800 Subject: [PATCH] fix hermes acp adapter responses --- internal/hermesadapter/client.go | 2 +- internal/hermesadapter/config.go | 92 ++++++++++ internal/hermesadapter/server.go | 29 ++++ internal/hermesadapter/server_test.go | 197 ++++++++++++++++++++++ scripts/ci/verify_hermes_acp_scenarios.sh | 174 +++++++++++++++++++ 5 files changed, 493 insertions(+), 1 deletion(-) create mode 100644 internal/hermesadapter/config.go create mode 100755 scripts/ci/verify_hermes_acp_scenarios.sh diff --git a/internal/hermesadapter/client.go b/internal/hermesadapter/client.go index f892e63..1602fba 100644 --- a/internal/hermesadapter/client.go +++ b/internal/hermesadapter/client.go @@ -201,7 +201,7 @@ func (c *stdioRPCClient) callLocked(method string, params map[string]any) (map[s } var payload map[string]any if err := json.Unmarshal(line, &payload); err != nil { - return nil, fmt.Errorf("decode hermes acp response: %w", err) + continue } if responseID, _ := payload["id"].(string); responseID != "" { if responseID == requestID { diff --git a/internal/hermesadapter/config.go b/internal/hermesadapter/config.go new file mode 100644 index 0000000..54bfaae --- /dev/null +++ b/internal/hermesadapter/config.go @@ -0,0 +1,92 @@ +package hermesadapter + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" +) + +func resolveHermesConfiguredModel() string { + for _, path := range hermesConfigCandidatePaths() { + if model := readHermesModelFromConfig(path); model != "" { + return model + } + } + return "" +} + +func hermesConfigCandidatePaths() []string { + candidates := make([]string, 0, 3) + if hermesHome := strings.TrimSpace(os.Getenv("HERMES_HOME")); hermesHome != "" { + candidates = append(candidates, filepath.Join(hermesHome, "config.yaml")) + } + if home := strings.TrimSpace(os.Getenv("HOME")); home != "" { + candidates = append(candidates, filepath.Join(home, ".hermes", "config.yaml")) + } + if userHome, err := os.UserHomeDir(); err == nil { + userHome = strings.TrimSpace(userHome) + if userHome != "" { + candidates = append(candidates, filepath.Join(userHome, ".hermes", "config.yaml")) + } + } + return dedupeStrings(candidates) +} + +func readHermesModelFromConfig(path string) string { + path = strings.TrimSpace(path) + if path == "" { + return "" + } + data, err := os.ReadFile(path) + if err != nil { + return "" + } + var config map[string]any + if err := yaml.Unmarshal(data, &config); err != nil { + return "" + } + if model := resolveHermesModelValue(config["model"]); model != "" { + return model + } + if model := strings.TrimSpace(fmt.Sprint(config["model"])); model != "" && model != "" { + return model + } + return "" +} + +func resolveHermesModelValue(value any) string { + switch model := value.(type) { + case string: + return strings.TrimSpace(model) + case map[string]any: + for _, key := range []string{"default", "model"} { + if candidate := strings.TrimSpace(fmt.Sprint(model[key])); candidate != "" && candidate != "" { + return candidate + } + } + } + return "" +} + +func dedupeStrings(values []string) []string { + if len(values) == 0 { + return values + } + seen := make(map[string]struct{}, len(values)) + result := make([]string, 0, len(values)) + for _, value := range values { + value = strings.TrimSpace(value) + if value == "" { + continue + } + if _, ok := seen[value]; ok { + continue + } + seen[value] = struct{}{} + result = append(result, value) + } + return result +} diff --git a/internal/hermesadapter/server.go b/internal/hermesadapter/server.go index fb94fbc..9070920 100644 --- a/internal/hermesadapter/server.go +++ b/internal/hermesadapter/server.go @@ -350,6 +350,7 @@ func (s *Server) handleHermesACPUpstreamSessionRequest(method string, params map workingDirectory = "." } + createdUpstreamSession := false if state.upstreamSessionID == "" || method == "session.start" { newSessionResp, err := s.client.Call("session/new", map[string]any{ "cwd": workingDirectory, @@ -372,6 +373,7 @@ func (s *Server) handleHermesACPUpstreamSessionRequest(method string, params map "error": "hermes upstream did not return a session id", } } + createdUpstreamSession = true } s.sessionsMu.Lock() @@ -385,6 +387,33 @@ func (s *Server) handleHermesACPUpstreamSessionRequest(method string, params map current.model = strings.TrimSpace(shared.StringArg(params, "model", current.model)) s.sessionsMu.Unlock() + resolvedModel := strings.TrimSpace(current.model) + if resolvedModel == "" { + resolvedModel = resolveHermesConfiguredModel() + } + if resolvedModel != "" && (createdUpstreamSession || resolvedModel != current.model) { + if _, err := s.client.Call("session/set_model", map[string]any{ + "sessionId": state.upstreamSessionID, + "modelId": resolvedModel, + }); err != nil { + return map[string]any{ + "success": false, + "provider": s.providerID, + "mode": "single-agent", + "error": err.Error(), + "upstreamMethod": "session/set_model", + } + } + s.sessionsMu.Lock() + current = s.sessions[sessionID] + if current == nil { + current = &adapterSession{} + s.sessions[sessionID] = current + } + current.model = resolvedModel + s.sessionsMu.Unlock() + } + var outputParts []string notificationHandler := func(notification map[string]any) { text := extractHermesSessionUpdateText(notification) diff --git a/internal/hermesadapter/server_test.go b/internal/hermesadapter/server_test.go index f0dcf2c..ec8fba7 100644 --- a/internal/hermesadapter/server_test.go +++ b/internal/hermesadapter/server_test.go @@ -5,6 +5,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "github.com/gorilla/websocket" @@ -12,6 +14,13 @@ import ( "xworkmate-bridge/internal/shared" ) +func isolateHermesConfig(t *testing.T) { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("HERMES_HOME", "") +} + type stubClient struct { initResult initializeResult initErr error @@ -59,6 +68,8 @@ func TestHandleCapabilitiesSynthesizesProviderResponse(t *testing.T) { } func TestHandleRPCSessionStartReturnsUpstreamResult(t *testing.T) { + isolateHermesConfig(t) + var stub *stubClient stub = &stubClient{initResult: initializeResult{ProtocolVersion: 1}} stub.callFn = func(method string, params map[string]any) (map[string]any, error) { @@ -125,6 +136,8 @@ func TestHandleRPCSessionStartReturnsUpstreamResult(t *testing.T) { } func TestHandleRPCSessionStartRejectsEmptyUpstreamResponse(t *testing.T) { + isolateHermesConfig(t) + var stub *stubClient stub = &stubClient{initResult: initializeResult{ProtocolVersion: 1}} stub.callFn = func(method string, params map[string]any) (map[string]any, error) { @@ -184,6 +197,190 @@ func TestNewServerDefaultsHermesToSessionPrompt(t *testing.T) { } } +func TestHandleRPCSessionMessageReusesUpstreamSession(t *testing.T) { + isolateHermesConfig(t) + + var stub *stubClient + stub = &stubClient{initResult: initializeResult{ProtocolVersion: 1}} + promptCalls := 0 + stub.callFn = func(method string, params map[string]any) (map[string]any, error) { + switch method { + case "session/new": + return map[string]any{ + "result": map[string]any{ + "sessionId": "upstream-session-1", + }, + }, nil + case "session/prompt": + promptCalls++ + if got := params["sessionId"]; got != "upstream-session-1" { + t.Fatalf("expected upstream session id to persist, got %#v", got) + } + if stub.notificationHandler != nil { + text := "first" + if promptCalls > 1 { + text = "second" + } + stub.notificationHandler(map[string]any{ + "method": "session/update", + "params": map[string]any{ + "update": map[string]any{ + "sessionUpdate": "agent_message_chunk", + "text": text, + }, + }, + }) + } + return map[string]any{ + "result": map[string]any{ + "stopReason": "end_turn", + }, + }, nil + default: + return map[string]any{"result": map[string]any{}}, nil + } + } + + server := NewServer(stub) + server.upstreamMethod = "session/prompt" + + startBody, _ := json.Marshal(shared.RPCRequest{ + JSONRPC: "2.0", + ID: 1, + Method: "session.start", + Params: map[string]any{ + "sessionId": "s1", + "taskPrompt": "first", + "workingDirectory": "/tmp/demo", + }, + }) + startReq := httptest.NewRequest(http.MethodPost, "http://127.0.0.1/acp/rpc", bytes.NewReader(startBody)) + startReq.Header.Set("Authorization", "Bearer test-token") + startRec := httptest.NewRecorder() + server.HandleRPC(startRec, startReq) + + if startRec.Code != http.StatusOK { + t.Fatalf("expected 200 for session.start, got %d", startRec.Code) + } + var startEnvelope map[string]any + if err := json.NewDecoder(startRec.Body).Decode(&startEnvelope); err != nil { + t.Fatalf("decode start response: %v", err) + } + startResult := startEnvelope["result"].(map[string]any) + if got := startResult["output"]; got != "first" { + t.Fatalf("expected first output, got %#v", startResult) + } + + messageBody, _ := json.Marshal(shared.RPCRequest{ + JSONRPC: "2.0", + ID: 2, + Method: "session.message", + Params: map[string]any{ + "sessionId": "s1", + "taskPrompt": "second", + "workingDirectory": "/tmp/demo", + }, + }) + messageReq := httptest.NewRequest(http.MethodPost, "http://127.0.0.1/acp/rpc", bytes.NewReader(messageBody)) + messageReq.Header.Set("Authorization", "Bearer test-token") + messageRec := httptest.NewRecorder() + server.HandleRPC(messageRec, messageReq) + + if messageRec.Code != http.StatusOK { + t.Fatalf("expected 200 for session.message, got %d", messageRec.Code) + } + var messageEnvelope map[string]any + if err := json.NewDecoder(messageRec.Body).Decode(&messageEnvelope); err != nil { + t.Fatalf("decode message response: %v", err) + } + messageResult := messageEnvelope["result"].(map[string]any) + if got := messageResult["output"]; got != "second" { + t.Fatalf("expected second output, got %#v", messageResult) + } + if len(stub.methods) != 3 || stub.methods[0] != "session/new" || stub.methods[1] != "session/prompt" || stub.methods[2] != "session/prompt" { + t.Fatalf("expected session/new then two session/prompt calls, got %#v", stub.methods) + } +} + +func TestHandleRPCSessionStartUsesConfiguredHermesModelBeforePrompt(t *testing.T) { + home := t.TempDir() + hermesHome := filepath.Join(home, ".hermes") + if err := os.MkdirAll(hermesHome, 0o755); err != nil { + t.Fatalf("mkdir hermes home: %v", err) + } + if err := os.WriteFile(filepath.Join(hermesHome, "config.yaml"), []byte("model:\n default: hermes-default-model\n"), 0o644); err != nil { + t.Fatalf("write hermes config: %v", err) + } + t.Setenv("HOME", home) + t.Setenv("HERMES_HOME", "") + + var stub *stubClient + stub = &stubClient{initResult: initializeResult{ProtocolVersion: 1}} + stub.callFn = func(method string, params map[string]any) (map[string]any, error) { + switch method { + case "session/new": + return map[string]any{ + "result": map[string]any{ + "sessionId": "upstream-session-1", + }, + }, nil + case "session/set_model": + if got := params["sessionId"]; got != "upstream-session-1" { + t.Fatalf("expected sessionId upstream-session-1, got %#v", got) + } + if got := params["modelId"]; got != "hermes-default-model" { + t.Fatalf("expected configured hermes model, got %#v", got) + } + return map[string]any{"result": map[string]any{}}, nil + case "session/prompt": + if stub.notificationHandler != nil { + stub.notificationHandler(map[string]any{ + "method": "session/update", + "params": map[string]any{ + "update": map[string]any{ + "sessionUpdate": "agent_message_chunk", + "text": "ok", + }, + }, + }) + } + return map[string]any{ + "result": map[string]any{ + "stopReason": "end_turn", + }, + }, nil + default: + return map[string]any{"result": map[string]any{}}, nil + } + } + + server := NewServer(stub) + server.upstreamMethod = "session/prompt" + + body, _ := json.Marshal(shared.RPCRequest{ + JSONRPC: "2.0", + ID: 1, + Method: "session.start", + Params: map[string]any{ + "sessionId": "s-model", + "taskPrompt": "hello", + "workingDirectory": "/tmp/demo", + }, + }) + request := httptest.NewRequest(http.MethodPost, "http://127.0.0.1/acp/rpc", bytes.NewReader(body)) + request.Header.Set("Authorization", "Bearer test-token") + recorder := httptest.NewRecorder() + + server.HandleRPC(recorder, request) + + if recorder.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", recorder.Code) + } + if len(stub.methods) != 3 || stub.methods[0] != "session/new" || stub.methods[1] != "session/set_model" || stub.methods[2] != "session/prompt" { + t.Fatalf("expected session/new then session/set_model then session/prompt, got %#v", stub.methods) + } +} + func TestHandleWebSocketCapabilities(t *testing.T) { server := NewServer(&stubClient{initResult: initializeResult{ProtocolVersion: 1}}) httpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/scripts/ci/verify_hermes_acp_scenarios.sh b/scripts/ci/verify_hermes_acp_scenarios.sh new file mode 100755 index 0000000..5b4b65a --- /dev/null +++ b/scripts/ci/verify_hermes_acp_scenarios.sh @@ -0,0 +1,174 @@ +#!/usr/bin/env bash +set -euo pipefail + +BRIDGE_SERVER_URL="${BRIDGE_SERVER_URL:-https://xworkmate-bridge.svc.plus}" +BRIDGE_AUTH_TOKEN="${BRIDGE_AUTH_TOKEN:-}" +HERMES_RPC_URL="${HERMES_RPC_URL:-${BRIDGE_SERVER_URL%/}/acp-server/hermes/acp/rpc}" + +if [[ -z "${BRIDGE_AUTH_TOKEN}" ]]; then + echo "Error: BRIDGE_AUTH_TOKEN is required" >&2 + exit 1 +fi + +rpc_post() { + local url="$1" + local payload="$2" + curl \ + --silent \ + --show-error \ + --fail \ + --location \ + --max-time 180 \ + -H "Accept: application/json" \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer ${BRIDGE_AUTH_TOKEN}" \ + --data "${payload}" \ + "${url}" +} + +json_get() { + local response_json="$1" + local expr="$2" + RESPONSE_JSON="${response_json}" python3 - "$expr" <<'PY' +import json +import os +import sys + +expr = sys.argv[1] +payload = json.loads(os.environ["RESPONSE_JSON"]) + +def get(value, path): + current = value + for part in path: + if isinstance(current, list): + try: + current = current[int(part)] + except Exception: + return "" + continue + if not isinstance(current, dict): + return "" + current = current.get(part) + if current is None: + return "" + if current is None: + return "" + if isinstance(current, (dict, list)): + return json.dumps(current, ensure_ascii=False) + return str(current) + +print(get(payload, expr.split("."))) +PY +} + +assert_nonempty() { + local name="$1" + local value="$2" + if [[ -z "${value}" ]]; then + echo "FAIL: ${name} is empty" >&2 + exit 1 + fi +} + +assert_contains() { + local name="$1" + local haystack="$2" + local needle="$3" + if [[ "${haystack}" != *"${needle}"* ]]; then + echo "FAIL: ${name} does not contain ${needle}: ${haystack}" >&2 + exit 1 + fi +} + +echo "--- Verifying Hermes ACP scenarios for ${BRIDGE_SERVER_URL} ---" + +echo "Scenario A: long dialogue" +hermes_caps="$( + rpc_post \ + "${HERMES_RPC_URL}" \ + '{"jsonrpc":"2.0","id":"hermes-capabilities","method":"acp.capabilities","params":{}}' +)" +assert_contains "hermes capabilities" "$(json_get "${hermes_caps}" "result.providers")" "hermes" + +hermes_session_id="hermes-scenario-$(date +%s)" +hermes_thread_id="${hermes_session_id}" + +hermes_start="$( + rpc_post \ + "${HERMES_RPC_URL}" \ + "{\"jsonrpc\":\"2.0\",\"id\":\"hermes-start\",\"method\":\"session.start\",\"params\":{\"sessionId\":\"${hermes_session_id}\",\"threadId\":\"${hermes_thread_id}\",\"taskPrompt\":\"Reply with exactly pong\",\"workingDirectory\":\"/tmp/hermes-long-dialogue\"}}" +)" +assert_nonempty "hermes start output" "$(json_get "${hermes_start}" "result.output")" +assert_contains "hermes start output" "$(json_get "${hermes_start}" "result.output")" "pong" +assert_contains "hermes start upstream method" "$(json_get "${hermes_start}" "result.upstreamMethod")" "session/prompt" +hermes_upstream_session_id="$(json_get "${hermes_start}" "result.upstreamSessionId")" +assert_nonempty "hermes upstream session id" "${hermes_upstream_session_id}" + +hermes_message_1="$( + rpc_post \ + "${HERMES_RPC_URL}" \ + "{\"jsonrpc\":\"2.0\",\"id\":\"hermes-message-1\",\"method\":\"session.message\",\"params\":{\"sessionId\":\"${hermes_session_id}\",\"threadId\":\"${hermes_thread_id}\",\"taskPrompt\":\"Reply with exactly pong again\",\"workingDirectory\":\"/tmp/hermes-long-dialogue\"}}" +)" +assert_nonempty "hermes message1 output" "$(json_get "${hermes_message_1}" "result.output")" +assert_contains "hermes message1 upstream method" "$(json_get "${hermes_message_1}" "result.upstreamMethod")" "session/prompt" +assert_nonempty "hermes message1 upstream session id" "$(json_get "${hermes_message_1}" "result.upstreamSessionId")" + +hermes_message_2="$( + rpc_post \ + "${HERMES_RPC_URL}" \ + "{\"jsonrpc\":\"2.0\",\"id\":\"hermes-message-2\",\"method\":\"session.message\",\"params\":{\"sessionId\":\"${hermes_session_id}\",\"threadId\":\"${hermes_thread_id}\",\"taskPrompt\":\"Reply with exactly pong one more time\",\"workingDirectory\":\"/tmp/hermes-long-dialogue\"}}" +)" +assert_nonempty "hermes message2 output" "$(json_get "${hermes_message_2}" "result.output")" +assert_contains "hermes message2 upstream method" "$(json_get "${hermes_message_2}" "result.upstreamMethod")" "session/prompt" + +if [[ "$(json_get "${hermes_message_1}" "result.upstreamSessionId")" != "${hermes_upstream_session_id}" ]]; then + echo "FAIL: Hermes upstream session id changed on message 1" >&2 + exit 1 +fi +if [[ "$(json_get "${hermes_message_2}" "result.upstreamSessionId")" != "${hermes_upstream_session_id}" ]]; then + echo "FAIL: Hermes upstream session id changed on message 2" >&2 + exit 1 +fi + +echo "Scenario B: skills install routing" +skills_resolve="$( + rpc_post \ + "${BRIDGE_SERVER_URL%/}/acp/rpc" \ + '{"jsonrpc":"2.0","id":"skills-resolve","method":"xworkmate.routing.resolve","params":{"taskPrompt":"translate and dub this video with subtitles","workingDirectory":"/tmp/skills-install","routing":{"routingMode":"auto","allowSkillInstall":true,"availableSkills":[{"id":"docx","label":"docx","description":"docs","installed":true}]}}}' +)" +assert_contains "skills resolve source" "$(json_get "${skills_resolve}" "result.skillResolutionSource")" "find_skills" +assert_contains "skills resolve needs install" "$(json_get "${skills_resolve}" "result.needsSkillInstall")" "true" +install_request_id="$(json_get "${skills_resolve}" "result.skillInstallRequestId")" +assert_nonempty "skill install request id" "${install_request_id}" +skill_candidate_id="$(json_get "${skills_resolve}" "result.skillCandidates.0.id")" +assert_nonempty "skill candidate id" "${skill_candidate_id}" + +skills_retry="$( + rpc_post \ + "${BRIDGE_SERVER_URL%/}/acp/rpc" \ + "{\"jsonrpc\":\"2.0\",\"id\":\"skills-retry\",\"method\":\"xworkmate.routing.resolve\",\"params\":{\"taskPrompt\":\"translate and dub this video with subtitles\",\"workingDirectory\":\"/tmp/skills-install\",\"routing\":{\"routingMode\":\"auto\",\"allowSkillInstall\":true,\"installApproval\":{\"requestId\":\"${install_request_id}\",\"approvedSkillKeys\":[\"${skill_candidate_id}\"]},\"availableSkills\":[{\"id\":\"docx\",\"label\":\"docx\",\"description\":\"docs\",\"installed\":true}]}}}" +)" +assert_contains "skills retry needs install" "$(json_get "${skills_retry}" "result.needsSkillInstall")" "false" +assert_nonempty "skills retry resolved skills" "$(json_get "${skills_retry}" "result.resolvedSkills")" + +echo "Scenario C: long dialogue + skills task" +task_session_id="hermes-task-$(date +%s)" +task_thread_id="${task_session_id}" +task_start="$( + rpc_post \ + "${BRIDGE_SERVER_URL%/}/acp/rpc" \ + "{\"jsonrpc\":\"2.0\",\"id\":\"task-start\",\"method\":\"session.start\",\"params\":{\"sessionId\":\"${task_session_id}\",\"threadId\":\"${task_thread_id}\",\"taskPrompt\":\"Create a powerpoint deck outline for a launch plan and use the selected skills.\",\"workingDirectory\":\"/tmp/hermes-task\",\"provider\":\"hermes\",\"routing\":{\"routingMode\":\"explicit\",\"explicitExecutionTarget\":\"singleAgent\",\"explicitProviderId\":\"hermes\",\"explicitSkills\":[\"pptx\",\"docx\"],\"allowSkillInstall\":false,\"availableSkills\":[{\"id\":\"pptx\",\"label\":\"pptx\",\"description\":\"slides\",\"installed\":true},{\"id\":\"docx\",\"label\":\"docx\",\"description\":\"docs\",\"installed\":true}]},\"selectedSkills\":[\"pptx\",\"docx\"],\"executionTarget\":\"agent\"}}" +)" +assert_nonempty "task start output" "$(json_get "${task_start}" "result.output")" +task_resolved_skills="$(json_get "${task_start}" "result.resolvedSkills")" +assert_contains "task start resolved skills" "${task_resolved_skills}" "pptx" + +task_followup="$( + rpc_post \ + "${BRIDGE_SERVER_URL%/}/acp/rpc" \ + "{\"jsonrpc\":\"2.0\",\"id\":\"task-message\",\"method\":\"session.message\",\"params\":{\"sessionId\":\"${task_session_id}\",\"threadId\":\"${task_thread_id}\",\"taskPrompt\":\"Continue the same deck outline and keep the structure short.\",\"workingDirectory\":\"/tmp/hermes-task\",\"provider\":\"hermes\",\"routing\":{\"routingMode\":\"explicit\",\"explicitExecutionTarget\":\"singleAgent\",\"explicitProviderId\":\"hermes\",\"explicitSkills\":[\"pptx\",\"docx\"],\"allowSkillInstall\":false,\"availableSkills\":[{\"id\":\"pptx\",\"label\":\"pptx\",\"description\":\"slides\",\"installed\":true},{\"id\":\"docx\",\"label\":\"docx\",\"description\":\"docs\",\"installed\":true}]},\"selectedSkills\":[\"pptx\",\"docx\"],\"executionTarget\":\"agent\"}}" +)" +assert_nonempty "task followup output" "$(json_get "${task_followup}" "result.output")" +assert_contains "task followup upstream method" "$(json_get "${task_followup}" "result.upstreamMethod")" "session/prompt" + +echo "Hermes ACP scenarios verified."