Surface workspace-mcp tool-handler errors as execution failures
Decision: workspace-mcp's /api/tool, /api/tools direct-HTTP endpoints are a first-class, server-sanctioned transport (see transport.py's "Direct HTTP API for non-MCP clients"), not a workaround — Hexis's REST provider is the right integration, no migration to the SSE/JSON-RPC MCP transport needed. The actual gap: workspace-mcp catches tool-handler exceptions and reports them as an ERROR-coded warning inside a 200 response envelope rather than an HTTP error status, so Execute()'s status-code check alone let every such failure through as a successful execution. Detect the ERROR-coded warning and return it as an error so it maps to Hexis's failed status. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ghELqYhZNLub2TXGMazqA
This commit is contained in:
@@ -175,11 +175,42 @@ func (p *WorkspaceMCPProvider) Execute(capability *domain.Capability, req *domai
|
|||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// workspace-mcp catches tool-handler exceptions and reports them as an
|
||||||
|
// ERROR-coded warning inside a 200 response envelope rather than an HTTP
|
||||||
|
// error status, so a plain status-code check would report every such
|
||||||
|
// failure as a successful execution. Surface it as an error here so it
|
||||||
|
// maps to Hexis's failed execution status instead.
|
||||||
|
if envelopeErr := workspaceEnvelopeError(result); envelopeErr != "" {
|
||||||
|
return map[string]any{"result": result}, fmt.Errorf("workspace tool %q reported an error: %s", toolName, envelopeErr)
|
||||||
|
}
|
||||||
|
|
||||||
return map[string]any{
|
return map[string]any{
|
||||||
"result": result,
|
"result": result,
|
||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func workspaceEnvelopeError(result any) string {
|
||||||
|
envelope, ok := result.(map[string]any)
|
||||||
|
if !ok {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
warnings, ok := envelope["warnings"].([]any)
|
||||||
|
if !ok {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
for _, w := range warnings {
|
||||||
|
warning, ok := w.(map[string]any)
|
||||||
|
if !ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if code, _ := warning["code"].(string); code == "ERROR" {
|
||||||
|
message, _ := warning["message"].(string)
|
||||||
|
return message
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
func (p *WorkspaceMCPProvider) capabilityToTool(capName string) string {
|
func (p *WorkspaceMCPProvider) capabilityToTool(capName string) string {
|
||||||
for toolName, mapping := range p.allowlist.Tools {
|
for toolName, mapping := range p.allowlist.Tools {
|
||||||
if mapping.Capability == capName {
|
if mapping.Capability == capName {
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
package provider
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/kami/hexis/internal/domain"
|
||||||
|
)
|
||||||
|
|
||||||
|
func newWorkspaceTestServer(t *testing.T, status int, body map[string]any) (*httptest.Server, *WorkspaceMCPProvider) {
|
||||||
|
t.Helper()
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(status)
|
||||||
|
json.NewEncoder(w).Encode(body)
|
||||||
|
}))
|
||||||
|
t.Cleanup(srv.Close)
|
||||||
|
|
||||||
|
allowlist := ToolAllowlist{Tools: map[string]ToolMapping{
|
||||||
|
"workspace_tool": {Capability: "ws.do_thing", Risk: "write", ReadOnly: false},
|
||||||
|
}}
|
||||||
|
return srv, NewWorkspaceMCPProvider(srv.URL, allowlist)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExecute_SucceedsOnPlainResult(t *testing.T) {
|
||||||
|
_, p := newWorkspaceTestServer(t, http.StatusOK, map[string]any{
|
||||||
|
"summary": "ok", "items": []any{}, "warnings": []any{}, "truncated": false,
|
||||||
|
})
|
||||||
|
|
||||||
|
cap := &domain.Capability{Name: "ws.do_thing"}
|
||||||
|
result, err := p.Execute(cap, &domain.ExecuteRequest{})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected no error, got %v", err)
|
||||||
|
}
|
||||||
|
if result == nil {
|
||||||
|
t.Fatal("expected non-nil result")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestExecute_ErrorEnvelopeReportedAsFailure verifies that a 200 response
|
||||||
|
// carrying workspace-mcp's ERROR-coded warning envelope (its convention for
|
||||||
|
// tool-handler exceptions) is treated as a failed execution, not a success.
|
||||||
|
func TestExecute_ErrorEnvelopeReportedAsFailure(t *testing.T) {
|
||||||
|
_, p := newWorkspaceTestServer(t, http.StatusOK, map[string]any{
|
||||||
|
"summary": "boom",
|
||||||
|
"items": []any{},
|
||||||
|
"warnings": []any{
|
||||||
|
map[string]any{"code": "ERROR", "message": "boom"},
|
||||||
|
},
|
||||||
|
"truncated": false,
|
||||||
|
})
|
||||||
|
|
||||||
|
cap := &domain.Capability{Name: "ws.do_thing"}
|
||||||
|
_, err := p.Execute(cap, &domain.ExecuteRequest{})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for ERROR-coded warning envelope, got nil")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user