feat: enhance route matching to prefer literal segments over wildcard…

.github/workflows/ci.yml

@@ -58,6 +58,26 @@ jobs:
       - name: Verify module compiles
         run: go build ./...
 
+  build-wasm:
+    name: Build (wasip1)
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+
+      - name: Setup Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+          cache-dependency-path: go.sum
+
+      - name: Verify module compiles for the wasip1 plugin target
+        run: GOOS=wasip1 GOARCH=wasm go build ./...
+
   test:
     name: Test
     runs-on: ubuntu-latest

context.go

@@ -77,6 +77,14 @@ func newContext(db DBBackend, kv KVBackend, log LogBackend, cfg ConfigBackend, p
 	}
 }
 
+// matchRoute finds the registered route for method+path. When more than one
+// registered pattern matches the same incoming path — e.g. a plugin
+// registers both "/views/:viewId/panels/:panelId" and
+// "/views/:viewId/panels/layout" under the same method, and the request is
+// for ".../panels/layout" — the most specific pattern wins (the one with
+// fewer ":param" segments), so a literal segment like "layout" always beats
+// a same-position ":panelId" wildcard. Without this, Go's randomized map
+// iteration order would make the match nondeterministic across calls.
 func (c *Context) matchRoute(method, path string) (RouteHandler, map[string]string, bool) {
 	method = strings.ToUpper(method)
 	if handler, ok := c.routes[routeKey{method, path}]; ok {
@@ -85,22 +93,60 @@ func (c *Context) matchRoute(method, path string) (RouteHandler, map[string]stri
 
 	projectID, relativePath, hasProjectScope := splitProjectPath(path)
 
+	var (
+		bestHandler     RouteHandler
+		bestParams      map[string]string
+		bestSpecificity = -1
+		found           bool
+	)
+	consider := func(pattern string, handler RouteHandler, tryPath string, withProjectID bool) {
+		params, ok := matchRoutePattern(pattern, tryPath)
+		if !ok {
+			return
+		}
+		specificity := routeSpecificity(pattern)
+		if specificity <= bestSpecificity {
+			return
+		}
+		if withProjectID {
+			params["projectId"] = projectID
+		}
+		bestHandler, bestParams, bestSpecificity, found = handler, params, specificity, true
+	}
+
 	for key, handler := range c.routes {
 		if key.method != method {
 			continue
 		}
-		if params, ok := matchRoutePattern(key.path, path); ok {
-			return handler, params, true
-		}
+		consider(key.path, handler, path, false)
 		if hasProjectScope {
-			if params, ok := matchRoutePattern(key.path, relativePath); ok {
-				params["projectId"] = projectID
-				return handler, params, true
-			}
+			consider(key.path, handler, relativePath, true)
 		}
 	}
 
-	return nil, nil, false
+	return bestHandler, bestParams, found
+}
+
+// routeSpecificity counts a pattern's literal (non-":param") segments —
+// used to break ties when multiple registered patterns match the same
+// incoming path. Any leading "/projects/:projectId" scope prefix is
+// stripped first, so a fully-qualified pattern like
+// "/projects/:projectId/tasks/:taskId" and its relative-style equivalent
+// "/tasks/:taskId" (matched via splitProjectPath's implicit projectId
+// injection) score the same — spelling the prefix out explicitly is a
+// registration-style choice, not extra genuine specificity.
+func routeSpecificity(pattern string) int {
+	segments := splitPathSegments(pattern)
+	if len(segments) >= 2 && segments[0] == "projects" && strings.HasPrefix(segments[1], ":") {
+		segments = segments[2:]
+	}
+	n := 0
+	for _, seg := range segments {
+		if !strings.HasPrefix(seg, ":") {
+			n++
+		}
+	}
+	return n
 }
 
 func splitProjectPath(path string) (projectID, relativePath string, ok bool) {

context_test.go

@@ -0,0 +1,85 @@
+package plugin
+
+import "testing"
+
+// Regression test for a route-matching ambiguity: when a plugin registers
+// both "/views/:viewId/panels/:panelId" and "/views/:viewId/panels/layout"
+// under the same method, a request for ".../panels/layout" matches both
+// patterns. matchRoute must always prefer the more specific (literal
+// "layout") pattern over the wildcard ":panelId" one — previously this was
+// resolved by Go's randomized map iteration order, so the "wrong" handler
+// (treating "layout" as a panel ID) would win nondeterministically across
+// calls. Note: plugintest.Context.Call bypasses this entirely by passing the
+// literal ":panelId" pattern string as the path, so it never exercised this
+// branch — this test calls matchRoute with a real resolved path instead.
+func TestMatchRoute_PrefersLiteralSegmentOverWildcard(t *testing.T) {
+	ctx := newContext(newWASMDBBackend(), newWASMKVBackend(), newWASMLogBackend(), newWASMConfigBackend(), newWASMPermissionBackend())
+
+	var gotByPanelID, gotLayout bool
+	ctx.Route("PATCH", "/views/:viewId/panels/:panelId", func(_ *Request, _ *Response) {
+		gotByPanelID = true
+	})
+	ctx.Route("PATCH", "/views/:viewId/panels/layout", func(_ *Request, _ *Response) {
+		gotLayout = true
+	})
+
+	for i := 0; i < 50; i++ {
+		gotByPanelID, gotLayout = false, false
+		handler, params, ok := ctx.matchRoute("PATCH", "/views/view-abc/panels/layout")
+		if !ok {
+			t.Fatalf("iteration %d: expected a route match, got none", i)
+		}
+		handler(nil, nil)
+		if !gotLayout || gotByPanelID {
+			t.Fatalf("iteration %d: expected the literal /panels/layout route to win, got panelId route instead (params=%v)", i, params)
+		}
+	}
+
+	// Sanity check the wildcard route still matches a real panel ID.
+	handler, params, ok := ctx.matchRoute("PATCH", "/views/view-abc/panels/panel-123")
+	if !ok {
+		t.Fatal("expected a route match for a real panel ID")
+	}
+	gotByPanelID, gotLayout = false, false
+	handler(nil, nil)
+	if !gotByPanelID || gotLayout {
+		t.Fatalf("expected the :panelId route to win for a non-'layout' panel ID, got params=%v", params)
+	}
+	if params["panelId"] != "panel-123" {
+		t.Fatalf("expected panelId param %q, got %q", "panel-123", params["panelId"])
+	}
+}
+
+// Regression test for routeSpecificity treating the "/projects/:projectId"
+// scope prefix as extra specificity. A fully-qualified pattern that spells
+// the prefix out ("/projects/:projectId/tasks/:taskId") has 2 raw literal
+// segments ("projects", "tasks"), while a relative-style literal pattern
+// matched via splitProjectPath's implicit projectId injection
+// ("/tasks/summary") also has 2 ("tasks", "summary") — but is the genuinely
+// more specific match for a request path ending in ".../tasks/summary".
+// Before stripping the prefix, both scored 2 and the winner depended on map
+// iteration order; after stripping, the wildcard pattern scores 1 and the
+// literal one correctly and deterministically wins.
+func TestMatchRoute_PrefersLiteralOverWildcardAcrossRegistrationStyles(t *testing.T) {
+	ctx := newContext(newWASMDBBackend(), newWASMKVBackend(), newWASMLogBackend(), newWASMConfigBackend(), newWASMPermissionBackend())
+
+	var gotWildcard, gotLiteral bool
+	ctx.Route("GET", "/projects/:projectId/tasks/:taskId", func(_ *Request, _ *Response) {
+		gotWildcard = true
+	})
+	ctx.Route("GET", "/tasks/summary", func(_ *Request, _ *Response) {
+		gotLiteral = true
+	})
+
+	for i := 0; i < 50; i++ {
+		gotWildcard, gotLiteral = false, false
+		handler, _, ok := ctx.matchRoute("GET", "/projects/proj-1/tasks/summary")
+		if !ok {
+			t.Fatalf("iteration %d: expected a route match, got none", i)
+		}
+		handler(nil, nil)
+		if !gotLiteral || gotWildcard {
+			t.Fatalf("iteration %d: expected the literal /tasks/summary route to win, got wildcard route instead", i)
+		}
+	}
+}

wasm_backends.go

@@ -59,14 +59,21 @@ func (b *wasmDBBackend) Query(sql string, params []any) (*DBQueryResult, error)
 		return nil, err
 	}
 
-	outputBuf := make([]byte, 8)
-	hostDBQuery(
+	// 16 bytes: [0:4]=resultPtr [4:8]=resultLen [8:12]=errPtr [12:16]=errLen.
+	// db_query2 (unlike the deprecated db_query) reports execution errors
+	// back through errPtr/errLen instead of silently returning a zero-length
+	// result indistinguishable from a genuine zero-row success.
+	outputBuf := make([]byte, 16)
+	hostDBQuery2(
 		int64(ptrOf(sqlBytes)), int64(len(sqlBytes)),
 		int64(ptrOf(paramsJSON)), int64(len(paramsJSON)),
 		int64(ptrOf(outputBuf)), int64(ptrOf(outputBuf[4:])),
+		int64(ptrOf(outputBuf[8:])), int64(ptrOf(outputBuf[12:])),
 	)
-	resPtr := int32(uint32(outputBuf[0]) | uint32(outputBuf[1])<<8 | uint32(outputBuf[2])<<16 | uint32(outputBuf[3])<<24)
-	resLen := int32(uint32(outputBuf[4]) | uint32(outputBuf[5])<<8 | uint32(outputBuf[6])<<16 | uint32(outputBuf[7])<<24)
+	resPtr, resLen, errPtr, errLen := decodeQuery2Output(outputBuf)
+	if errLen > 0 {
+		return nil, &hostError{string(wasmSlice(errPtr, errLen))}
+	}
 	if resLen == 0 {
 		return &DBQueryResult{}, nil
 	}

wasm_imports.go

@@ -15,10 +15,21 @@ func hostLog(level int32, ptr, length int64)
 
 // paca.db_query(sqlPtr i64, sqlLen i64, paramsPtr i64, paramsLen i64, resultPtrPtr i64, resultLenPtr i64)
 //
+// Deprecated: has no error channel, so a query that fails to execute is
+// indistinguishable from one that succeeded with zero rows. Use
+// hostDBQuery2. Kept only so already-compiled plugin binaries that still
+// import this signature keep working against newer hosts.
+//
 //go:wasmimport paca db_query
 //go:noescape
 func hostDBQuery(sqlPtr, sqlLen, paramsPtr, paramsLen, resultPtrPtr, resultLenPtr int64)
 
+// paca.db_query2(sqlPtr i64, sqlLen i64, paramsPtr i64, paramsLen i64, resultPtrPtr i64, resultLenPtr i64, errPtrPtr i64, errLenPtr i64)
+//
+//go:wasmimport paca db_query2
+//go:noescape
+func hostDBQuery2(sqlPtr, sqlLen, paramsPtr, paramsLen, resultPtrPtr, resultLenPtr, errPtrPtr, errLenPtr int64)
+
 // paca.db_exec(sqlPtr i64, sqlLen i64, paramsPtr i64, paramsLen i64, rowsAffectedPtr i64, errPtrPtr i64, errLenPtr i64)
 //
 //go:wasmimport paca db_exec

wasm_output.go

@@ -0,0 +1,14 @@
+package plugin
+
+// decodeQuery2Output parses the 16-byte output buffer written by the
+// paca.db_query2 host import: [0:4]=resultPtr [4:8]=resultLen
+// [8:12]=errPtr [12:16]=errLen. Kept in a non-wasip1-tagged file so it can
+// be unit tested on the host build; wasm_backends.go (wasip1-only) is the
+// real caller.
+func decodeQuery2Output(buf []byte) (resPtr, resLen, errPtr, errLen int32) {
+	resPtr = int32(uint32(buf[0]) | uint32(buf[1])<<8 | uint32(buf[2])<<16 | uint32(buf[3])<<24)
+	resLen = int32(uint32(buf[4]) | uint32(buf[5])<<8 | uint32(buf[6])<<16 | uint32(buf[7])<<24)
+	errPtr = int32(uint32(buf[8]) | uint32(buf[9])<<8 | uint32(buf[10])<<16 | uint32(buf[11])<<24)
+	errLen = int32(uint32(buf[12]) | uint32(buf[13])<<8 | uint32(buf[14])<<16 | uint32(buf[15])<<24)
+	return
+}

wasm_output_test.go

@@ -0,0 +1,46 @@
+package plugin
+
+import "testing"
+
+func TestDecodeQuery2Output(t *testing.T) {
+	tests := []struct {
+		name                                           string
+		buf                                            []byte
+		wantResPtr, wantResLen, wantErrPtr, wantErrLen int32
+	}{
+		{
+			name:       "success with rows",
+			buf:        []byte{0x10, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0, 0, 0, 0, 0, 0, 0, 0},
+			wantResPtr: 0x10,
+			wantResLen: 0x20,
+			wantErrPtr: 0,
+			wantErrLen: 0,
+		},
+		{
+			name:       "success with zero rows",
+			buf:        make([]byte, 16),
+			wantResPtr: 0,
+			wantResLen: 0,
+			wantErrPtr: 0,
+			wantErrLen: 0,
+		},
+		{
+			name:       "execution error",
+			buf:        []byte{0, 0, 0, 0, 0, 0, 0, 0, 0x40, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00},
+			wantResPtr: 0,
+			wantResLen: 0,
+			wantErrPtr: 0x40,
+			wantErrLen: 0x05,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			resPtr, resLen, errPtr, errLen := decodeQuery2Output(tt.buf)
+			if resPtr != tt.wantResPtr || resLen != tt.wantResLen || errPtr != tt.wantErrPtr || errLen != tt.wantErrLen {
+				t.Fatalf("decodeQuery2Output(%v) = (%d, %d, %d, %d), want (%d, %d, %d, %d)",
+					tt.buf, resPtr, resLen, errPtr, errLen, tt.wantResPtr, tt.wantResLen, tt.wantErrPtr, tt.wantErrLen)
+			}
+		})
+	}
+}