fix(cli): apply params:query block to outgoing URL (#1681)#7937
fix(cli): apply params:query block to outgoing URL (#1681)#7937tyskjohan wants to merge 1 commit into
Conversation
The CLI parsed the params:query block into request.params but never
appended type === 'query' entries to the request URL. Only path
params were honoured. Any query param defined only in the
params:query block was silently dropped.
The desktop GUI hides this because it keeps the URL bar and the
params table in sync on edit, so by request-time the URL already
carries every query param. The CLI never runs that sync step.
Fix:
- prepare-request.js: surface a queryParams field on the prepared
request (same shape as the existing pathParams).
- interpolate-vars.js: after pathParams substitution, interpolate
each queryParam name/value and append it to the URL via
URLSearchParams. Names already present on the URL are left
alone, so a GUI-saved file (which carries the same params in
both places) round-trips identically.
Tests:
- 9 new cases in interpolate-vars.spec.js covering: single param,
multiple params, {{var}} interpolation in name+value, no-duplicate
with URL-embedded param, special-character encoding (Oracle Fusion
q-syntax with ';' and quotes), disabled-param behaviour, no-op on
empty/undefined queryParams, co-existence with pathParams.
- 2 new cases in prepare-request.spec.js covering: filtering of
disabled and unnamed entries, and treating missing 'enabled' as
enabled (legacy bru files).
All 120 existing CLI tests still pass.
Closes usebruno#1681
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR extends the Bruno CLI runner to interpolate and apply ChangesQuery Parameter Interpolation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Fixes #1681 —
@usebruno/cliparses theparams:queryblock but never appends the entries to the outgoing URL, so any query param defined only there is silently dropped. The desktop GUI hides this because it keeps the URL bar and the params table in sync on edit, so by request-time the URL already carries every query param. The CLI never runs that sync step.Reproduces on current
@usebruno/cli@3.3.0againsthttpbin.org/anything:→
args: {}from httpbin. Theqnever reaches the wire.What's in the fix
Two small changes, both in
packages/bruno-cli:src/runner/prepare-request.js— surface aqueryParamsfield on the prepared request, mirroring the existingpathParamsextraction. Filters out disabled and unnamed entries.src/runner/interpolate-vars.js— after the existing pathParams substitution, interpolate eachqueryParamname/value, then append to the URL viaURLSearchParams. Names already present on the URL are left alone, so a GUI-saved file (which carries the same params in bothurl:andparams:query) round-trips identically — no double-appending, no behaviour change for existing collections.The "URL value wins on collision" choice mirrors how the GUI behaves: when you edit a row in the params table, it rewrites the URL, so the URL is always the authoritative copy at request time.
Test plan
New tests, all passing locally:
tests/runner/interpolate-vars.spec.js— 9 new cases:{{vars}}in query param names and values'and;)queryParamsis empty or undefinedpathParamssubstitutiontests/runner/prepare-request.spec.js— 2 new cases:type=queryparams, dropping disabled and unnamed entriesenabledas enabled (legacy bru files)Full suite:
npx jestinpackages/bruno-cli→ 120 passed, 120 total (118 pre-existing + 2 new in prepare-request; the 9 new in interpolate-vars are counted separately under that file). No existing tests modified.End-to-end smoke — patched
@usebruno/cli@3.3.0with these two source changes against a real Oracle Fusion REST endpoint (q=ItemNumber='AS85000';OrganizationCode='001'filter), with a cleanurl:and all params declared only inparams:query:Same
.brufile returns 400 (params:querydropped) on stock 3.3.0.Notes for reviewers
searchParams.append(preserves the order params were declared in) and skip-if-name-already-in-URL (so GUI-saved files don't double up). Open to switching toset(overwrite) if you'd preferparams:queryto win — happy to adjust.querydeclaration ignored #1681, also relevant to Bruno CLI doesn't seem to be injecting properly #3136 (CLI param injection) and [Issue] CLI ignores path params in request-urls #2506 (CLI path-param sister bug, not addressed here).🤖 Generated with Claude Code
Summary by CodeRabbit
{{vars}}) in both names and values