refactor(mcp): typed ValidationError surfaces via .error.data

Phase 5d slice B. createItemTool / updateItemTool stop encoding rejections
as `validation <kind>: <detail> [{json-blob}]` glued into .error.message
and instead return ValidationToolError(ve), which the JSON-RPC envelope
marshals as:

  { code: -32602,
    message: "<kind>: <detail>",
    data:    { kind, path, detail } }

Clients introspect `.error.data.kind` directly — no JSON suffix to parse
out of the message. -32602 is the JSON-RPC "Invalid params" code, the
right semantic level for an itemwrite rejection.

mcp/tools.go:
- Replace itemWriteError with ValidationToolError. The legacy helper is
  gone; four call sites (create_item × 2, update_item × 2) switch over
  one-for-one.

mcp/mcp_test.go:
- Add TestToolsCallValidationError. Pins the wire shape: code=-32602,
  message=`<kind>: <detail>` with no JSON suffix, and data carrying
  {kind, path, detail}. Also asserts the rejection does NOT route
  through result.isError — the slice A guarantee remains intact for
  validation errors specifically.
- Import internal/itemwrite for the ValidationError fixture.

No test source edits to existing assertions — the prior tests don't
inspect the legacy `validation X: Y [{...}]` Msg shape, so behaviour
preservation holds without touching them. The new test is additive.

Live probe (post-deploy): POST `create_item` against projax.msbls.de/mcp/rpc
with slug='BAD.SLUG' returns `error.data.kind = "invalid-slug-format"`.
This commit is contained in:
mAi
2026-05-22 11:50:57 +02:00
parent d7438ba89e
commit 8370454b66
2 changed files with 62 additions and 20 deletions

View File

@@ -9,6 +9,8 @@ import (
"net/http/httptest" "net/http/httptest"
"strings" "strings"
"testing" "testing"
"github.com/m/projax/internal/itemwrite"
) )
// rpcJSON builds a JSON-RPC request body. // rpcJSON builds a JSON-RPC request body.
@@ -124,6 +126,49 @@ func TestToolsCallSuccessAndError(t *testing.T) {
} }
} }
// TestToolsCallValidationError pins the Phase 5d slice B contract:
// ValidationToolError surfaces the typed {kind, path, detail} payload via
// .error.data, the message is the clean "<kind>: <detail>" form (no JSON
// suffix), and the JSON-RPC code is -32602 per the Invalid-params
// convention. The production-side artifact probe (POST create_item with
// slug='BAD.SLUG' against projax.msbls.de) exercises the same path through
// the real itemwrite.ValidateFormat call.
func TestToolsCallValidationError(t *testing.T) {
srv := New("p", "1", "", nil)
srv.Register(Tool{
Name: "boom",
Handler: func(ctx context.Context, raw json.RawMessage) (any, *ToolError) {
return nil, ValidationToolError(&itemwrite.ValidationError{
Kind: itemwrite.KindInvalidSlugFormat,
Path: "dev.bad",
Detail: "slug must be lower-case, no dots/whitespace",
})
},
})
_, body := doRPC(t, srv, rpcJSON(t, 7, "tools/call", map[string]any{
"name": "boom", "arguments": map[string]any{},
}), "")
s := string(body)
if !strings.Contains(s, `"code":-32602`) {
t.Fatalf("missing code:-32602: %s", s)
}
if !strings.Contains(s, `"message":"invalid-slug-format: slug must be lower-case, no dots/whitespace"`) {
t.Errorf("message should be '<kind>: <detail>' with no JSON suffix: %s", s)
}
if !strings.Contains(s, `"kind":"invalid-slug-format"`) {
t.Errorf("missing data.kind: %s", s)
}
if !strings.Contains(s, `"path":"dev.bad"`) {
t.Errorf("missing data.path: %s", s)
}
if !strings.Contains(s, `"detail":"slug must be lower-case, no dots/whitespace"`) {
t.Errorf("missing data.detail: %s", s)
}
if strings.Contains(s, `"isError":true`) {
t.Errorf("validation rejection should not route through result.isError: %s", s)
}
}
func TestAuthBearerRequired(t *testing.T) { func TestAuthBearerRequired(t *testing.T) {
srv := New("p", "1", "s3cr3t", nil) srv := New("p", "1", "s3cr3t", nil)

View File

@@ -14,23 +14,20 @@ import (
"github.com/m/projax/store" "github.com/m/projax/store"
) )
// itemWriteError serialises an *itemwrite.ValidationError into a *ToolError // ValidationToolError promotes an *itemwrite.ValidationError into a typed
// whose Msg carries the JSON-suffixed legacy shape (kind/path/detail in a // *ToolError: code -32602 (Invalid params) per JSON-RPC convention, a
// bracketed JSON blob). Slice B promotes this to the typed // clean "<kind>: <detail>" Msg, and Data carrying the structured
// ValidationToolError below — until then this helper preserves the // {kind, path, detail} object MCP clients introspect via .error.data
// pre-Phase-5d wire text so consumers reading .error.message keep working. // without parsing a JSON suffix out of the message.
func itemWriteError(ve *itemwrite.ValidationError) *ToolError { func ValidationToolError(ve *itemwrite.ValidationError) *ToolError {
body, err := json.Marshal(map[string]any{
"kind": ve.Kind,
"path": ve.Path,
"detail": ve.Detail,
})
if err != nil {
return &ToolError{Code: codeInternalError, Msg: ve.Error()}
}
return &ToolError{ return &ToolError{
Code: codeInternalError, Code: codeInvalidParams,
Msg: fmt.Sprintf("validation %s: %s [%s]", ve.Kind, ve.Detail, string(body)), Msg: fmt.Sprintf("%s: %s", ve.Kind, ve.Detail),
Data: map[string]any{
"kind": ve.Kind,
"path": ve.Path,
"detail": ve.Detail,
},
} }
} }
@@ -852,12 +849,12 @@ func createItemTool(st *store.Store) ToolHandler {
if ve := itemwrite.ValidateFormat(itemwrite.Input{ if ve := itemwrite.ValidateFormat(itemwrite.Input{
Title: in.Title, Slug: in.Slug, Status: in.Status, ParentIDs: parentIDs, Title: in.Title, Slug: in.Slug, Status: in.Status, ParentIDs: parentIDs,
}); ve != nil { }); ve != nil {
return nil, itemWriteError(ve) return nil, ValidationToolError(ve)
} }
if ve := itemwrite.ValidateAgainstStore(ctx, st, itemwrite.Input{ if ve := itemwrite.ValidateAgainstStore(ctx, st, itemwrite.Input{
Title: in.Title, Slug: in.Slug, Status: in.Status, ParentIDs: parentIDs, Title: in.Title, Slug: in.Slug, Status: in.Status, ParentIDs: parentIDs,
}); ve != nil { }); ve != nil {
return nil, itemWriteError(ve) return nil, ValidationToolError(ve)
} }
it, err := st.Create(ctx, store.CreateInput{ it, err := st.Create(ctx, store.CreateInput{
Kind: kind, Kind: kind,
@@ -1016,10 +1013,10 @@ func updateItemTool(st *store.Store) ToolHandler {
Path: it.PrimaryPath(), Path: it.PrimaryPath(),
} }
if ve := itemwrite.ValidateFormat(validateIn); ve != nil { if ve := itemwrite.ValidateFormat(validateIn); ve != nil {
return nil, itemWriteError(ve) return nil, ValidationToolError(ve)
} }
if ve := itemwrite.ValidateAgainstStore(ctx, st, validateIn); ve != nil { if ve := itemwrite.ValidateAgainstStore(ctx, st, validateIn); ve != nil {
return nil, itemWriteError(ve) return nil, ValidationToolError(ve)
} }
updated, err := st.Update(ctx, it.ID, patch) updated, err := st.Update(ctx, it.ID, patch)
if err != nil { if err != nil {