hanzoai/cloud
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 1 Wiki Activity

fix(zapface): carry the HTTP verb, stop inferring it from the method name

Browse Source 
The ZAP dispatcher chose its HTTP method by looking at the method NAME: a "get-"
prefix meant GET, everything else meant POST. That was sound only because the /v1
surface encoded its verb in every route name — /v1/get-store and /v1/update-store
are different names, so a name WAS a complete request description.

The RESTful surface removes that signal entirely. /v1/ai/providers/{owner}/{name}
answers GET, PATCH and DELETE at one path, and no prefix distinguishes them. Under
the old rule every one of those would have been sent as a POST: a read as a write,
and a DELETE landing on the create route instead of destroying anything. Silently,
with a 2xx.

So the method is now an HTTP request line — "<VERB> <path>":

    GET    ai/providers/acme/openai
    PATCH  ai/providers/acme/openai
    DELETE ai/providers/acme/openai
    POST   ai/providers

A method with no verb is REFUSED, not defaulted. Defaulting would convert a
caller's omission into a wrong-but-plausible request, and on a surface where the
verb decides between reading a resource and destroying it, a plausible wrong guess
is the worst available outcome. The integration test asserts the refusal names the
missing verb.

The fixture in the integration test now switches on METHOD AND PATH, which is what
made the old design's problem concrete: it is the same pair the dispatcher has to
carry, and the reason a name alone can no longer produce it.

Worth recording: this face has no first-party consumer left — console's src/lib/zap
is gone and nothing imports it — but /zap is still mounted in serve.go, so it is
fixed rather than left silently guessing. Whether to retire the mount is a separate
call. The cloud.event checkout carrying the same code is a WORKTREE of this repo on
another branch; it inherits this fix on merge and was deliberately not touched.

Co-authored-by: Hanzo Dev <dev@hanzo.ai>
This commit is contained in:
hanzo-dev
2026-07-27 08:38:05 -07:00
parent eb6b93c09f
commit 98d5a7c15e
3 changed files with 123 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
zapface/dispatch.go
+54 -12
View File
@@ -99,21 +99,25 @@ func (d *dispatcher) dispatch(call zaprpc.Call, cookieHeader, authHeader, accept
// buildHTTPRequest maps (method, SuperJSON input) onto a /v1 request.
//
// the /v1 convention (mirrored from console/src/lib/api/providers.ts):
// - "get-*" -> GET /v1/<method> with scalar input fields as the query.
// - others -> POST /v1/<method> with scalar input fields lifted to the
// The method is an HTTP request line, "<VERB> <path>" (see splitMethod):
// - a read verb (GET/HEAD/DELETE) -> no body; scalar input fields become the query.
// - a write verb (POST/PUT/PATCH) -> scalar input fields lifted to the
// query (identity hints like id/owner) and the single nested
// object/array field as the JSON body (the resource); if there is no
// nested field, the whole input is the body.
//
// This single rule covers every twin shape without per-endpoint coupling:
//
// get-provider {id} -> GET ?id=...
// get-providers {owner,store,...} -> GET ?owner=...&store=...
// update-provider {id, provider} -> POST ?id=... body=provider
// add-provider <provider> -> POST body=<provider>
// GET ai/providers/acme/openai {} -> GET /v1/ai/providers/acme/openai
// GET ai/providers {owner,store} -> GET /v1/ai/providers?owner=...&store=...
// PATCH ai/providers/acme/openai {provider} -> PATCH … body=provider
// POST ai/providers <provider> -> POST /v1/ai/providers body=<provider>
// DELETE ai/providers/acme/openai {} -> DELETE /v1/ai/providers/acme/openai
func buildHTTPRequest(req zapRequest) (*http.Request, error) {
path := "/v1/" + strings.TrimPrefix(req.method, "/")
verb, path, err := splitMethod(req.method)
if err != nil {
return nil, err
}
inputJSON := superJSONUnwrap(req.payload)
scalars, nested := splitInput(inputJSON)
@@ -127,13 +131,13 @@ func buildHTTPRequest(req zapRequest) (*http.Request, error) {
target += "?" + enc
}
if strings.HasPrefix(req.method, "get-") {
r := httptest.NewRequest(http.MethodGet, target, nil)
if verb == http.MethodGet || verb == http.MethodHead || verb == http.MethodDelete {
r := httptest.NewRequest(verb, target, nil)
r.Header.Set("Accept", "application/json")
return r, nil
}
// Mutation: choose the body.
// Mutation with a body: choose it.
var body []byte
switch {
case nested != nil:
@@ -143,13 +147,51 @@ func buildHTTPRequest(req zapRequest) (*http.Request, error) {
default:
body = []byte("{}")
}
r := httptest.NewRequest(http.MethodPost, target, bytes.NewReader(body))
r := httptest.NewRequest(verb, target, bytes.NewReader(body))
r.Header.Set("Content-Type", "application/json")
r.Header.Set("Accept", "application/json")
r.ContentLength = int64(len(body))
return r, nil
}
// zapVerbs are the methods a ZAP call may name. Anything else is refused rather
// than coerced — a caller that cannot say what it wants to do does not get a guess.
var zapVerbs = map[string]bool{
http.MethodGet: true, http.MethodHead: true, http.MethodPost: true,
http.MethodPut: true, http.MethodPatch: true, http.MethodDelete: true,
}
// splitMethod reads a ZAP method as an HTTP request line: "<VERB> <path>", e.g.
// "GET rag/stores" or "PATCH ai/providers/acme/openai". The path is rooted at /v1.
//
// The verb is CARRIED, not inferred. It used to be guessed from the method name —
// a "get-" prefix meant GET and everything else meant POST — which worked only
// because the route surface encoded its verb in every route name (/v1/get-store
// vs /v1/update-store). Once the surface became RESTful that signal disappeared:
// one path answers GET, PATCH and DELETE, and no prefix distinguishes them. A
// heuristic there would silently send a delete as a POST.
//
// A method with no verb is an ERROR, not a default. Defaulting would turn a
// caller's omission into a wrong-but-plausible request — for a surface where the
// verb decides between reading a resource and destroying it, that is the one
// behaviour worth refusing outright.
func splitMethod(method string) (verb, path string, err error) {
m := strings.TrimSpace(method)
head, rest, ok := strings.Cut(m, " ")
if !ok {
return "", "", fmt.Errorf("method %q must be %q — the HTTP verb is required, not inferred", method, "<VERB> <path>")
}
verb = strings.ToUpper(strings.TrimSpace(head))
if !zapVerbs[verb] {
return "", "", fmt.Errorf("method %q names an unsupported verb %q", method, verb)
}
rest = strings.TrimSpace(rest)
if rest == "" {
return "", "", fmt.Errorf("method %q names no path", method)
}
return verb, "/v1/" + strings.TrimPrefix(rest, "/"), nil
}
// splitInput separates a JSON object into its scalar fields (rendered as query
// strings) and at most one nested object/array field (the resource body). A
// non-object input yields no scalars and the whole value as the nested body.
zapface/integration_test.go
+39 -18
View File
@@ -26,27 +26,31 @@ func startZapApp(t *testing.T) (string, func()) {
t.Helper()
app := zip.New(zip.Config{})
// /v1 handlers (mirror ai/mount.go: app.All("/v1/*", ...)).
// /v1 handlers (mirror ai/mount.go: app.All("/v1/*", ...)). The surface is
// RESTful, so these switch on METHOD AND PATH — the same pair the dispatcher
// now has to carry, and the reason it can no longer infer a verb from a name.
app.All("/v1/*", func(c *zip.Ctx) error {
path := c.Path()
path, method := c.Path(), c.Method()
switch {
case strings.HasSuffix(path, "/get-global-providers"):
case method == "GET" && strings.HasSuffix(path, "/ai/providers/global"):
return c.JSON(200, fiber.Map{
"status": "ok", "msg": "",
"data": []fiber.Map{
{"owner": "admin", "name": "openai", "category": "Model", "_cookie": c.Header("Cookie")},
},
})
case strings.HasSuffix(path, "/get-providers"):
case method == "GET" && strings.HasSuffix(path, "/ai/providers"):
return c.JSON(200, fiber.Map{
"status": "ok", "msg": "",
"data": []fiber.Map{{"owner": c.Query("owner"), "name": "p1"}},
"data2": 1,
})
case strings.HasSuffix(path, "/add-provider"):
case method == "POST" && strings.HasSuffix(path, "/ai/providers"):
body := map[string]any{}
_ = json.Unmarshal(c.Body(), &body)
return c.JSON(200, fiber.Map{"status": "ok", "msg": "added " + asString(body["name"])})
case method == "DELETE" && strings.Contains(path, "/ai/providers/"):
return c.JSON(200, fiber.Map{"status": "ok", "msg": "deleted " + path})
default:
return c.JSON(404, fiber.Map{"status": "error", "msg": "not found"})
}
@@ -130,10 +134,10 @@ func TestEndToEndOverWebSocket(t *testing.T) {
}
}
// 1) get-global-providers -> ok, real data, cookie replayed to /v1 handler.
rep := call("get-global-providers", nil, 1)
// 1) A global listing -> ok, real data, cookie replayed to the /v1 handler.
rep := call("GET ai/providers/global", nil, 1)
if !rep.ok || rep.status != 200 {
t.Fatalf("get-global-providers: ok=%v status=%d err=%s", rep.ok, rep.status, rep.errorJSON)
t.Fatalf("GET ai/providers/global: ok=%v status=%d err=%s", rep.ok, rep.status, rep.errorJSON)
}
var providers []map[string]any
if err := json.Unmarshal(superJSONUnwrap(rep.result), &providers); err != nil {
@@ -146,26 +150,43 @@ func TestEndToEndOverWebSocket(t *testing.T) {
t.Fatalf("cookie not replayed to /v1 handler: %v", providers[0]["_cookie"])
}
// 2) get-providers with a query param.
rep = call("get-providers", map[string]any{"owner": "admin"}, 2)
// 2) A collection list with a query param.
rep = call("GET ai/providers", map[string]any{"owner": "admin"}, 2)
if !rep.ok {
t.Fatalf("get-providers !ok: %s", rep.errorJSON)
t.Fatalf("GET ai/providers !ok: %s", rep.errorJSON)
}
_ = json.Unmarshal(superJSONUnwrap(rep.result), &providers)
if providers[0]["owner"] != "admin" {
t.Fatalf("get-providers owner query not forwarded: %v", providers)
t.Fatalf("owner query not forwarded: %v", providers)
}
// 3) add-provider (POST body).
rep = call("add-provider", map[string]any{"owner": "admin", "name": "claude"}, 3)
// 3) Create (POST body).
rep = call("POST ai/providers", map[string]any{"owner": "admin", "name": "claude"}, 3)
if !rep.ok {
t.Fatalf("add-provider !ok: %s", rep.errorJSON)
t.Fatalf("POST ai/providers !ok: %s", rep.errorJSON)
}
// 4) unknown method -> /v1 404 envelope -> !ok.
rep = call("get-nonexistent", nil, 4)
// 4) A DELETE reaches the DELETE route — the case the old prefix heuristic
// got wrong: it had no "get-" prefix, so it would have been sent as a POST
// and silently landed on the create route instead of destroying anything.
rep = call("DELETE ai/providers/admin/openai", nil, 4)
if !rep.ok {
t.Fatalf("DELETE ai/providers/admin/openai !ok: %s", rep.errorJSON)
}
// 5) unknown path -> /v1 404 envelope -> !ok.
rep = call("GET ai/nonexistent", nil, 5)
if rep.ok {
t.Fatalf("unknown method should not be ok")
t.Fatalf("unknown path should not be ok")
}
// 6) A method with NO verb is refused outright, not guessed at.
rep = call("get-providers", nil, 6)
if rep.ok {
t.Fatalf("a verbless method must be refused, not inferred")
}
if !strings.Contains(rep.errorJSON, "verb is required") {
t.Fatalf("refusal should name the missing verb, got %s", rep.errorJSON)
}
}
zapface/wire_test.go
+30 -21
View File
@@ -36,7 +36,7 @@ func buildClientRequestBytes(method, payload string, promiseID uint32) []byte {
// TestParseClientRequest proves the server decodes a real client frame: outer
// rpc envelope -> inner ZapRequest -> (method, SuperJSON payload).
func TestParseClientRequest(t *testing.T) {
const method = "get-providers"
const method = "GET ai/providers"
// console sends SuperJSON.stringify(input); a plain object X -> {"json":X}.
input := map[string]any{"owner": "admin", "store": "default", "limit": "20"}
innerJSON, _ := json.Marshal(input)
@@ -176,45 +176,54 @@ func TestBuildHTTPRequest(t *testing.T) {
wantBodyHa string // a substring expected in the body ('' = no body)
}{
{
name: "get-global-providers no args",
method: "get-global-providers",
name: "global listing, no args",
method: "GET ai/providers/global",
input: nil,
wantMethod: "GET",
wantPath: "/v1/get-global-providers",
wantPath: "/v1/ai/providers/global",
},
{
name: "get-provider with id",
method: "get-provider",
input: map[string]any{"id": "admin/openai"},
name: "member read — the id is in the PATH, not a query param",
method: "GET ai/providers/admin/openai",
input: nil,
wantMethod: "GET",
wantPath: "/v1/get-provider",
wantQuery: map[string]string{"id": "admin/openai"},
wantPath: "/v1/ai/providers/admin/openai",
},
{
name: "get-providers list query",
method: "get-providers",
name: "collection list keeps its filters as a query",
method: "GET ai/providers",
input: map[string]any{"owner": "admin", "store": "default"},
wantMethod: "GET",
wantPath: "/v1/get-providers",
wantPath: "/v1/ai/providers",
wantQuery: map[string]string{"owner": "admin", "store": "default"},
},
{
name: "update-provider id+resource",
method: "update-provider",
input: map[string]any{"id": "admin/openai", "provider": map[string]any{"owner": "admin", "name": "openai", "type": "OpenAI"}},
wantMethod: "POST",
wantPath: "/v1/update-provider",
wantQuery: map[string]string{"id": "admin/openai"},
// The verb travels with the call. Under the old prefix heuristic this
// same request would have been sent as a POST.
name: "member update is a PATCH with the resource as the body",
method: "PATCH ai/providers/admin/openai",
input: map[string]any{"provider": map[string]any{"owner": "admin", "name": "openai", "type": "OpenAI"}},
wantMethod: "PATCH",
wantPath: "/v1/ai/providers/admin/openai",
wantBodyHa: `"type":"OpenAI"`,
},
{
name: "add-provider bare resource",
method: "add-provider",
name: "create posts the bare resource to the collection",
method: "POST ai/providers",
input: map[string]any{"owner": "admin", "name": "openai", "type": "OpenAI"},
wantMethod: "POST",
wantPath: "/v1/add-provider",
wantPath: "/v1/ai/providers",
wantBodyHa: `"name":"openai"`,
},
{
// A DELETE carries no body — and under the old heuristic it would have
// been a POST, i.e. a destroy sent as a write to the wrong route.
name: "member delete is a DELETE with no body",
method: "DELETE ai/providers/admin/openai",
input: nil,
wantMethod: "DELETE",
wantPath: "/v1/ai/providers/admin/openai",
},
}
for _, tc := range cases {