← back to Cli Printing Press
fix(cli): cliutil.BuildPath replaces buildProxyPath with proper URL encoding (#437, #439) (#443)
d9472b76b10ae2ab5653713e9fc63a77f20efed5 · 2026-04-30 11:15:10 -0700 · Trevin Chow
* fix(cli): rewrite buildProxyPath as testable cliutil.BuildPath
Closes #437 and #439 — both touch the proxy-envelope path helper and
land together since the rewrite creates the seam the new test exercises.
#437 URL encoding + edge cases:
- Move buildProxyPath out of client.go.tmpl into a new
cliutil_proxypath.go.tmpl, exported as cliutil.BuildPath. Conditional
emission gated on ClientPattern == "proxy-envelope" — REST CLIs don't
ship the helper or its tests.
- Rewrite using net/url instead of string concatenation:
* Param values containing ?, &, =, # are now correctly URL-encoded
rather than corrupting the URL.
* Output is deterministic — url.Values.Encode sorts keys
alphabetically. The prior map-range form produced non-stable
output across runs, breaking any caller that used the URL as a
cache key or fingerprint.
* Paths with an existing query string (/api?op=list) merge new params
through the parsed URL rather than emitting a second ? separator.
* Trailing ? and & in the spec-declared path are normalized.
* Duplicate keys (same key in path's query AND params map) now have
params winning (Set semantics) instead of producing duplicate-key
output the proxy endpoint rejected as malformed.
#439 runtime test:
- New cliutil_proxypath_test.go.tmpl ships into every proxy-envelope
CLI's test suite. 16 table-driven cases covering: empty params,
single-param append, existing-query merge, value encoding (?, &, =,
space, non-ASCII), empty-value drop, all-empty short-circuit,
trailing-?/& normalization, duplicate-key override.
- TestBuildPath_Deterministic asserts 100 calls produce the same string
to lock in the ordering guarantee.
- TestBuildPath_MalformedPathReturnedUnchanged covers the url.Parse
err branch.
Generator-level changes:
- generator_test.go: TestProxyEnvelopeBuildPathHonorsExistingQueryString
is replaced by:
* TestProxyEnvelopeBuildPathEmittedAsCliutil — proves the structural
contract (helper + test file emitted, client.go calls cliutil.BuildPath,
inline buildProxyPath is gone).
* TestCliutilProxyPath_NotEmittedForRESTClients — proves the helper
is NOT shipped to non-proxy CLIs.
Behavioral coverage moved to the runtime test in the generated CLI.
- client.go.tmpl: drop inline buildProxyPath; both call sites route
through cliutil.BuildPath. The "sort" import is now gated on the
REST branch (proxy-envelope CLIs don't use it after this change).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): apply /simplify findings to BuildPath bundle
Findings from the three-agent /simplify pass on PR #443.
Quality:
- Trim BuildPath godoc from 18 lines to a focused contract:
empty-drop, params-wins-on-collision, alphabetical ordering,
malformed-verbatim. The "improvements over the prior" framing was
git-archaeology — belongs in the PR body, not the source.
- Drop the pointer-comment in client.go.tmpl ("Path-building is in
cliutil.BuildPath…"). cliutil.BuildPath at the call site is self-
documenting; the pointer-comment carried zero WHY.
- Trim TestCliutilProxyPath_NotEmittedForRESTClients comment.
Quality (semantic fix):
- Remove the `added` flag and the verbatim-preserve branch. With the
flag, `/api/things?` + all-empty params returned the path verbatim
while `/api/things?` + non-empty params normalized the trailing `?`
away — an asymmetry the doc comment claimed didn't exist. The
rewrite always passes through url.Parse + Encode and clears
ForceQuery when the merged query is empty so the doc-promised
"trailing `?`/` & ` are normalized" actually holds in every case.
- New test case: "all empty values normalizes a trailing question
mark" pins the consistent behavior.
Efficiency:
- Add BenchmarkBuildPath with three cases (no params, typical 5-key,
20-key with encoding). Establishes a baseline for future profile-
driven optimization (parsed-path caching, sync.Pool); without one
any later "this is slow" claim has to invent its own anchor.
Codify the design intent in the godoc: optimized for correctness over
throughput, hot path blocks on a network call so a few extra
allocations per request are invisible against that.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(cli): refresh stale golden fixture for mcp_tools OpenWithContext
PR #441 migrated handleSearch and handleSQL in mcp_tools.go.tmpl from
store.Open to store.OpenWithContext(ctx, ...) but didn't run
scripts/golden.sh update — the generate-golden-api fixture's emitted
internal/mcp/tools.go was left at the old form. This is the trailing
fixture refresh; no behavioral change in #443's own diff.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Files touched
M internal/generator/generator.goM internal/generator/proxy_envelope_test.goM internal/generator/templates/client.go.tmplA internal/generator/templates/cliutil_proxypath.go.tmplA internal/generator/templates/cliutil_proxypath_test.go.tmplM testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
Diff
commit d9472b76b10ae2ab5653713e9fc63a77f20efed5
Author: Trevin Chow <trevin@trevinchow.com>
Date: Thu Apr 30 11:15:10 2026 -0700
fix(cli): cliutil.BuildPath replaces buildProxyPath with proper URL encoding (#437, #439) (#443)
* fix(cli): rewrite buildProxyPath as testable cliutil.BuildPath
Closes #437 and #439 — both touch the proxy-envelope path helper and
land together since the rewrite creates the seam the new test exercises.
#437 URL encoding + edge cases:
- Move buildProxyPath out of client.go.tmpl into a new
cliutil_proxypath.go.tmpl, exported as cliutil.BuildPath. Conditional
emission gated on ClientPattern == "proxy-envelope" — REST CLIs don't
ship the helper or its tests.
- Rewrite using net/url instead of string concatenation:
* Param values containing ?, &, =, # are now correctly URL-encoded
rather than corrupting the URL.
* Output is deterministic — url.Values.Encode sorts keys
alphabetically. The prior map-range form produced non-stable
output across runs, breaking any caller that used the URL as a
cache key or fingerprint.
* Paths with an existing query string (/api?op=list) merge new params
through the parsed URL rather than emitting a second ? separator.
* Trailing ? and & in the spec-declared path are normalized.
* Duplicate keys (same key in path's query AND params map) now have
params winning (Set semantics) instead of producing duplicate-key
output the proxy endpoint rejected as malformed.
#439 runtime test:
- New cliutil_proxypath_test.go.tmpl ships into every proxy-envelope
CLI's test suite. 16 table-driven cases covering: empty params,
single-param append, existing-query merge, value encoding (?, &, =,
space, non-ASCII), empty-value drop, all-empty short-circuit,
trailing-?/& normalization, duplicate-key override.
- TestBuildPath_Deterministic asserts 100 calls produce the same string
to lock in the ordering guarantee.
- TestBuildPath_MalformedPathReturnedUnchanged covers the url.Parse
err branch.
Generator-level changes:
- generator_test.go: TestProxyEnvelopeBuildPathHonorsExistingQueryString
is replaced by:
* TestProxyEnvelopeBuildPathEmittedAsCliutil — proves the structural
contract (helper + test file emitted, client.go calls cliutil.BuildPath,
inline buildProxyPath is gone).
* TestCliutilProxyPath_NotEmittedForRESTClients — proves the helper
is NOT shipped to non-proxy CLIs.
Behavioral coverage moved to the runtime test in the generated CLI.
- client.go.tmpl: drop inline buildProxyPath; both call sites route
through cliutil.BuildPath. The "sort" import is now gated on the
REST branch (proxy-envelope CLIs don't use it after this change).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): apply /simplify findings to BuildPath bundle
Findings from the three-agent /simplify pass on PR #443.
Quality:
- Trim BuildPath godoc from 18 lines to a focused contract:
empty-drop, params-wins-on-collision, alphabetical ordering,
malformed-verbatim. The "improvements over the prior" framing was
git-archaeology — belongs in the PR body, not the source.
- Drop the pointer-comment in client.go.tmpl ("Path-building is in
cliutil.BuildPath…"). cliutil.BuildPath at the call site is self-
documenting; the pointer-comment carried zero WHY.
- Trim TestCliutilProxyPath_NotEmittedForRESTClients comment.
Quality (semantic fix):
- Remove the `added` flag and the verbatim-preserve branch. With the
flag, `/api/things?` + all-empty params returned the path verbatim
while `/api/things?` + non-empty params normalized the trailing `?`
away — an asymmetry the doc comment claimed didn't exist. The
rewrite always passes through url.Parse + Encode and clears
ForceQuery when the merged query is empty so the doc-promised
"trailing `?`/` & ` are normalized" actually holds in every case.
- New test case: "all empty values normalizes a trailing question
mark" pins the consistent behavior.
Efficiency:
- Add BenchmarkBuildPath with three cases (no params, typical 5-key,
20-key with encoding). Establishes a baseline for future profile-
driven optimization (parsed-path caching, sync.Pool); without one
any later "this is slow" claim has to invent its own anchor.
Codify the design intent in the godoc: optimized for correctness over
throughput, hot path blocks on a network call so a few extra
allocations per request are invisible against that.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(cli): refresh stale golden fixture for mcp_tools OpenWithContext
PR #441 migrated handleSearch and handleSQL in mcp_tools.go.tmpl from
store.Open to store.OpenWithContext(ctx, ...) but didn't run
scripts/golden.sh update — the generate-golden-api fixture's emitted
internal/mcp/tools.go was left at the old form. This is the trailing
fixture refresh; no behavioral change in #443's own diff.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
internal/generator/generator.go | 13 ++
internal/generator/proxy_envelope_test.go | 63 +++++--
internal/generator/templates/client.go.tmpl | 31 +---
.../generator/templates/cliutil_proxypath.go.tmpl | 49 +++++
.../templates/cliutil_proxypath_test.go.tmpl | 206 +++++++++++++++++++++
.../printing-press-golden/internal/mcp/tools.go | 4 +-
6 files changed, 317 insertions(+), 49 deletions(-)
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index d3f686f9..f71befd3 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -1237,6 +1237,19 @@ func (g *Generator) renderOptionalSupportFiles() error {
}
}
+ // Emit the cliutil proxypath helper only for proxy-envelope clients —
+ // the BuildPath function is the only caller of net/url.Values in the
+ // cliutil package, and there's no point shipping it (and its tests)
+ // into CLIs that don't speak the proxy-envelope protocol.
+ if g.Spec.ClientPattern == "proxy-envelope" {
+ if err := g.renderTemplate("cliutil_proxypath.go.tmpl", filepath.Join("internal", "cliutil", "proxypath.go"), g.Spec); err != nil {
+ return fmt.Errorf("rendering cliutil proxypath: %w", err)
+ }
+ if err := g.renderTemplate("cliutil_proxypath_test.go.tmpl", filepath.Join("internal", "cliutil", "proxypath_test.go"), g.Spec); err != nil {
+ return fmt.Errorf("rendering cliutil proxypath test: %w", err)
+ }
+ }
+
// Emit the auto-refresh wrapper only when cache is explicitly enabled
// and the CLI has both a store and a sync path to call. Without sync
// there is nothing to refresh with; without cache.enabled there is no
diff --git a/internal/generator/proxy_envelope_test.go b/internal/generator/proxy_envelope_test.go
index 05952fcf..1b7a6730 100644
--- a/internal/generator/proxy_envelope_test.go
+++ b/internal/generator/proxy_envelope_test.go
@@ -8,16 +8,15 @@ import (
"github.com/stretchr/testify/require"
)
-// TestProxyEnvelopeBuildPathHonorsExistingQueryString — when a proxy-
-// envelope path already carries a query string (e.g. /api?op=list),
-// buildProxyPath must use `&` to append additional params instead of `?`.
-// Two `?` separators in one URL produce a path the upstream proxy rejects
-// as malformed.
-//
-// We assert against the emitted client.go rather than calling the helper
-// directly because buildProxyPath only exists inside the proxy-envelope
-// template branch.
-func TestProxyEnvelopeBuildPathHonorsExistingQueryString(t *testing.T) {
+// TestProxyEnvelopeBuildPathEmittedAsCliutil — the proxy-envelope path
+// helper now lives in cliutil.BuildPath so it can be unit-tested
+// directly. The behavioral assertions on URL encoding, ordering, and
+// trailing-separator handling live in cliutil_proxypath_test.go.tmpl
+// (which the generator emits alongside the helper). This canary just
+// proves the structural contract: the helper file is emitted, the
+// client wires through the cliutil call, and the old inline helper is
+// gone from client.go.
+func TestProxyEnvelopeBuildPathEmittedAsCliutil(t *testing.T) {
t.Parallel()
apiSpec := minimalSpec("proxy-joiner")
@@ -26,16 +25,40 @@ func TestProxyEnvelopeBuildPathHonorsExistingQueryString(t *testing.T) {
outputDir := filepath.Join(t.TempDir(), "proxy-joiner-pp-cli")
require.NoError(t, New(apiSpec, outputDir).Generate())
+ // 1. The helper is emitted into the cliutil package.
+ proxyPathSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cliutil", "proxypath.go"))
+ require.NoError(t, err, "cliutil/proxypath.go must be emitted for proxy-envelope clients")
+ require.Contains(t, string(proxyPathSrc), "func BuildPath(",
+ "cliutil.BuildPath must be exported so client.go can call it")
+ require.Contains(t, string(proxyPathSrc), `"net/url"`,
+ "BuildPath must use net/url for correct query encoding")
+
+ // 2. Its test file is emitted next to it.
+ _, err = os.ReadFile(filepath.Join(outputDir, "internal", "cliutil", "proxypath_test.go"))
+ require.NoError(t, err, "cliutil/proxypath_test.go must ship into the generated CLI")
+
+ // 3. The client routes through cliutil and no longer carries an
+ // inline helper that could drift from the cliutil implementation.
clientSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "client", "client.go"))
require.NoError(t, err)
- src := string(clientSrc)
-
- require.Contains(t, src, "func buildProxyPath(",
- "proxy-envelope client must emit buildProxyPath helper")
- require.Contains(t, src, `joiner := "?"`,
- "buildProxyPath must default to ? joiner")
- require.Contains(t, src, `strings.Contains(path, "?")`,
- "buildProxyPath must check for an existing query string before appending")
- require.Contains(t, src, `joiner = "&"`,
- "buildProxyPath must use & when path already contains ?")
+ require.Contains(t, string(clientSrc), "cliutil.BuildPath(path, params)",
+ "client.go must call cliutil.BuildPath in the proxy-envelope branch")
+ require.NotContains(t, string(clientSrc), "func buildProxyPath(",
+ "client.go must not carry an inline buildProxyPath; the helper moved to cliutil")
+}
+
+// TestCliutilProxyPath_NotEmittedForRESTClients keeps the helper
+// scoped — REST CLIs would carry it as dead weight.
+func TestCliutilProxyPath_NotEmittedForRESTClients(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := minimalSpec("rest-only")
+ // Default ClientPattern is REST (empty string) — no override.
+
+ outputDir := filepath.Join(t.TempDir(), "rest-only-pp-cli")
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ _, err := os.Stat(filepath.Join(outputDir, "internal", "cliutil", "proxypath.go"))
+ require.Error(t, err, "cliutil/proxypath.go must NOT be emitted for non-proxy-envelope clients")
+ require.True(t, os.IsNotExist(err), "expected file-not-exist error, got: %v", err)
}
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index 7cb80011..e71d5a03 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -15,7 +15,9 @@ import (
"net/url"
"os"
"path/filepath"
+{{- if ne .ClientPattern "proxy-envelope"}}
"sort"
+{{- end}}
"strings"
"time"
@@ -70,31 +72,6 @@ func serviceForPath(path string) string {
{{- end}}
return "{{.Name}}" // default: API name slug
}
-
-// buildProxyPath appends query params into the path string for the envelope.
-// When the spec-declared path already carries a query string (e.g. an
-// /api?op=list endpoint that takes additional params), use `&` as the
-// joiner so the result stays parseable instead of producing two `?`
-// separators.
-func buildProxyPath(path string, params map[string]string) string {
- if len(params) == 0 {
- return path
- }
- parts := make([]string, 0, len(params))
- for k, v := range params {
- if v != "" {
- parts = append(parts, k+"="+v)
- }
- }
- if len(parts) == 0 {
- return path
- }
- joiner := "?"
- if strings.Contains(path, "?") {
- joiner = "&"
- }
- return path + joiner + strings.Join(parts, "&")
-}
{{end}}
// APIError carries HTTP status information for structured exit codes.
@@ -392,7 +369,7 @@ func (c *Client) do(method, path string, params map[string]string, body any, hea
envelope := proxyEnvelope{
Service: serviceForPath(path),
Method: method,
- Path: buildProxyPath(path, params),
+ Path: cliutil.BuildPath(path, params),
}
if bodyBytes != nil {
envelope.Body = json.RawMessage(bodyBytes)
@@ -526,7 +503,7 @@ func (c *Client) dryRun(method, targetURL, path string, params map[string]string
envelope := proxyEnvelope{
Service: serviceForPath(path),
Method: method,
- Path: buildProxyPath(path, params),
+ Path: cliutil.BuildPath(path, params),
}
if body != nil {
envelope.Body = json.RawMessage(body)
diff --git a/internal/generator/templates/cliutil_proxypath.go.tmpl b/internal/generator/templates/cliutil_proxypath.go.tmpl
new file mode 100644
index 00000000..f56e0299
--- /dev/null
+++ b/internal/generator/templates/cliutil_proxypath.go.tmpl
@@ -0,0 +1,49 @@
+// Copyright {{currentYear}} {{.Owner}}. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cliutil
+
+import (
+ "net/url"
+)
+
+// BuildPath appends query params to a request path for proxy-envelope
+// clients. Non-obvious contract:
+//
+// - Empty param values are dropped (not encoded as `key=`).
+// - When a key appears in both the path's existing query string and
+// the params map, params wins (url.Values.Set semantics).
+// - Output ordering is alphabetical-by-key (url.Values.Encode), so
+// callers can fingerprint the result safely.
+// - Malformed paths that fail url.Parse are returned verbatim; the
+// proxy request will surface the parse error during dispatch.
+//
+// Optimized for correctness, not throughput. The proxy-envelope path
+// blocks on a network call (~tens to hundreds of ms); a few extra
+// allocations here are invisible against that.
+func BuildPath(path string, params map[string]string) string {
+ if len(params) == 0 {
+ return path
+ }
+
+ u, err := url.Parse(path)
+ if err != nil {
+ return path
+ }
+
+ q := u.Query()
+ for k, v := range params {
+ if v == "" {
+ continue
+ }
+ q.Set(k, v)
+ }
+ u.RawQuery = q.Encode()
+ if u.RawQuery == "" {
+ // url.Parse sets ForceQuery on a trailing `?` so u.String()
+ // preserves it; clear the flag when the merged query is empty
+ // so the doc-promised normalization actually holds.
+ u.ForceQuery = false
+ }
+ return u.String()
+}
diff --git a/internal/generator/templates/cliutil_proxypath_test.go.tmpl b/internal/generator/templates/cliutil_proxypath_test.go.tmpl
new file mode 100644
index 00000000..68469352
--- /dev/null
+++ b/internal/generator/templates/cliutil_proxypath_test.go.tmpl
@@ -0,0 +1,206 @@
+// Copyright {{currentYear}} {{.Owner}}. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cliutil
+
+import "testing"
+
+func TestBuildPath(t *testing.T) {
+ t.Parallel()
+
+ cases := []struct {
+ name string
+ path string
+ params map[string]string
+ want string
+ }{
+ {
+ name: "no params returns path verbatim",
+ path: "/api/things",
+ params: nil,
+ want: "/api/things",
+ },
+ {
+ name: "empty map returns path verbatim",
+ path: "/api/things",
+ params: map[string]string{},
+ want: "/api/things",
+ },
+ {
+ name: "single param appended with question mark",
+ path: "/api/things",
+ params: map[string]string{"q": "test"},
+ want: "/api/things?q=test",
+ },
+ {
+ name: "multiple params sorted alphabetically",
+ path: "/api/things",
+ params: map[string]string{"z": "1", "a": "2", "m": "3"},
+ want: "/api/things?a=2&m=3&z=1",
+ },
+ {
+ name: "path with existing query merges with ampersand",
+ path: "/api/things?op=list",
+ params: map[string]string{"q": "test"},
+ want: "/api/things?op=list&q=test",
+ },
+ {
+ name: "value containing question mark is URL-encoded",
+ path: "/api/things",
+ params: map[string]string{"q": "what?"},
+ want: "/api/things?q=what%3F",
+ },
+ {
+ name: "value containing ampersand is URL-encoded",
+ path: "/api/things",
+ params: map[string]string{"q": "a&b"},
+ want: "/api/things?q=a%26b",
+ },
+ {
+ name: "value containing equals is URL-encoded",
+ path: "/api/things",
+ params: map[string]string{"q": "k=v"},
+ want: "/api/things?q=k%3Dv",
+ },
+ {
+ name: "value containing space is URL-encoded",
+ path: "/api/things",
+ params: map[string]string{"q": "hello world"},
+ want: "/api/things?q=hello+world",
+ },
+ {
+ name: "empty values are dropped",
+ path: "/api/things",
+ params: map[string]string{"a": "1", "b": "", "c": "3"},
+ want: "/api/things?a=1&c=3",
+ },
+ {
+ name: "all empty values preserves an existing query string",
+ path: "/api/things?op=list",
+ params: map[string]string{"a": "", "b": ""},
+ want: "/api/things?op=list",
+ },
+ {
+ name: "all empty values normalizes a trailing question mark",
+ path: "/api/things?",
+ params: map[string]string{"a": "", "b": ""},
+ want: "/api/things",
+ },
+ {
+ name: "trailing question mark in path is normalized",
+ path: "/api/things?",
+ params: map[string]string{"q": "v"},
+ want: "/api/things?q=v",
+ },
+ {
+ name: "trailing ampersand in path is normalized",
+ path: "/api/things?op=list&",
+ params: map[string]string{"q": "v"},
+ want: "/api/things?op=list&q=v",
+ },
+ {
+ name: "params overrides duplicate key in existing query",
+ path: "/api/things?op=list",
+ params: map[string]string{"op": "create"},
+ want: "/api/things?op=create",
+ },
+ {
+ name: "non-ASCII value is properly encoded",
+ path: "/api/things",
+ params: map[string]string{"q": "café"},
+ want: "/api/things?q=caf%C3%A9",
+ },
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+ got := BuildPath(tc.path, tc.params)
+ if got != tc.want {
+ t.Errorf("BuildPath(%q, %v) = %q, want %q", tc.path, tc.params, got, tc.want)
+ }
+ })
+ }
+}
+
+// TestBuildPath_Deterministic asserts the same input produces the same
+// output across calls. The prior map-range implementation was non-stable
+// on multi-key inputs, which broke fingerprint-based caching anywhere
+// the rendered URL was used as a key.
+func TestBuildPath_Deterministic(t *testing.T) {
+ t.Parallel()
+
+ path := "/api/things"
+ params := map[string]string{
+ "alpha": "1", "beta": "2", "gamma": "3", "delta": "4", "epsilon": "5",
+ }
+
+ first := BuildPath(path, params)
+ for i := 0; i < 100; i++ {
+ got := BuildPath(path, params)
+ if got != first {
+ t.Fatalf("BuildPath produced non-deterministic output on iteration %d:\n first: %q\n got: %q", i, first, got)
+ }
+ }
+}
+
+// TestBuildPath_MalformedPathReturnedUnchanged covers the err branch in
+// url.Parse. A truly malformed path (control characters, etc.) is
+// returned verbatim rather than corrupted further; the proxy request
+// will surface the parse error to the caller during dispatch.
+func TestBuildPath_MalformedPathReturnedUnchanged(t *testing.T) {
+ t.Parallel()
+
+ // url.Parse only fails on a small set of inputs (most malformed
+ // paths still parse to a Path-only URL). A control character in
+ // the path is one of the rejection cases.
+ malformed := "/api/things\x7f"
+ got := BuildPath(malformed, map[string]string{"q": "v"})
+ if got != malformed {
+ t.Fatalf("BuildPath on malformed path: got %q, want %q (verbatim)", got, malformed)
+ }
+}
+
+// BenchmarkBuildPath establishes a baseline for the per-request cost.
+// Run with `go test -bench=. ./internal/cliutil` to compare against
+// future changes — the helper sits on the proxy-envelope hot path and
+// any optimization (parsed-path caching, sync.Pool) needs a numeric
+// anchor before it pays for itself.
+func BenchmarkBuildPath(b *testing.B) {
+ cases := []struct {
+ name string
+ path string
+ params map[string]string
+ }{
+ {
+ name: "no params",
+ path: "/api/things",
+ params: nil,
+ },
+ {
+ name: "small typical proxy-envelope params",
+ path: "/api/things?op=list",
+ params: map[string]string{
+ "q": "test", "limit": "20", "cursor": "abc123", "sort": "name", "order": "asc",
+ },
+ },
+ {
+ name: "many params with encoding",
+ path: "/api/things",
+ params: map[string]string{
+ "a": "1", "b": "2", "c": "3?", "d": "4&5", "e": "6=7", "f": "café",
+ "g": "7", "h": "8", "i": "9", "j": "10", "k": "11", "l": "12",
+ "m": "13", "n": "14", "o": "15", "p": "16", "q": "17", "r": "18",
+ "s": "19", "t": "20",
+ },
+ },
+ }
+
+ for _, tc := range cases {
+ b.Run(tc.name, func(b *testing.B) {
+ for i := 0; i < b.N; i++ {
+ _ = BuildPath(tc.path, tc.params)
+ }
+ })
+ }
+}
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
index 7f31d036..a46e683a 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
@@ -277,7 +277,7 @@ func handleSearch(ctx context.Context, req mcplib.CallToolRequest) (*mcplib.Call
limit = int(v)
}
- db, err := store.Open(dbPath())
+ db, err := store.OpenWithContext(ctx, dbPath())
if err != nil {
return mcplib.NewToolResultError(fmt.Sprintf("opening database: %v", err)), nil
}
@@ -307,7 +307,7 @@ func handleSQL(ctx context.Context, req mcplib.CallToolRequest) (*mcplib.CallToo
}
}
- db, err := store.Open(dbPath())
+ db, err := store.OpenWithContext(ctx, dbPath())
if err != nil {
return mcplib.NewToolResultError(fmt.Sprintf("opening database: %v", err)), nil
}
← c11d0d95 fix(cli): store.OpenWithContext + fast-path version check (#
·
back to Cli Printing Press
·
feat(cli): framework-collision detection in resource-name ex 6d357b36 →