Revert: Re-add post response vars#6307
Conversation
WalkthroughThis PR adds post-response (response-scoped) variables across UI, collection merging, CLI and Electron runners, and test coverage: UI shows "Pre Request" and "Post Response" tables; collection merge gathers response vars; runners execute post-response var processing and emit environment updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Bruno App (UI)
participant Runner as Runner (CLI/Electron)
participant VarsRT as VarsRuntime
participant Env as Environment Store
User->>App: Trigger request execution
App->>Runner: Start request (includes vars.req & vars.res)
Runner->>Runner: Run pre-request script
Runner->>Runner: Send HTTP request
Runner->>Runner: Receive HTTP response
alt request.vars.res exists
Runner->>VarsRT: runPostResponseVars(request, response, env, ctx)
VarsRT->>VarsRT: Evaluate response variables
VarsRT-->>Runner: Return env updates / errors
Runner->>Env: Emit env updates (script, persistent, global) [IPC in Electron]
Runner->>Env: Update collection global variables
else no response vars
Runner->>Runner: Skip post-response var processing
end
Runner->>Runner: Execute post-response script
Runner-->>App: Return execution result
App-->>User: Display response & state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
Comment |
This reverts commit 786a341 while keeping code related to presets deleted
04ecd89 to
64ea4fa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/RequestPane/Vars/index.js (1)
7-8: Add safe array defaults for request/response vars to avoid undefined issues
get(item, 'draft.request.vars.req')/get(item, 'request.vars.req')(and theresequivalents) have no[]fallback. For requests created before vars were introduced, these paths may be missing, makingrequestVars/responseVarsundefinedand potentially breakingVarsTablewhen it iterates them.To align with other components and harden this pane, default these to empty arrays:
- const requestVars = item.draft ? get(item, 'draft.request.vars.req') : get(item, 'request.vars.req'); - const responseVars = item.draft ? get(item, 'draft.request.vars.res') : get(item, 'request.vars.res'); + const requestVars = item.draft ? get(item, 'draft.request.vars.req', []) : get(item, 'request.vars.req', []); + const responseVars = item.draft ? get(item, 'draft.request.vars.res', []) : get(item, 'request.vars.res', []);The Pre Request / Post Response sections themselves look good once these defaults are in place.
Also applies to: 13-19
🧹 Nitpick comments (2)
tests/request/newlines/newlines-persistence.spec.ts (1)
50-62: Multi-table vars selectors look good; consider asserting cell values tooTargeting
.btn-add-var.first()/last() and addressingtable.first()vstable.nth(1)makes the test align with the new Pre Request / Post Response layout and is a solid improvement. To further harden the persistence check, you could also assert the actual cell contents (including newlines) for both tables after restart usinggetTableCell(...)instead of only checking row counts.Based on learnings, this also keeps coverage aligned with the “add tests for new functionality” guideline.
Also applies to: 83-85
packages/bruno-electron/src/utils/collection.js (1)
99-132: Response-vars aggregation mirrors request-vars behavior correctlyThe new
resVarsaggregation follows the same precedence and last-writer-wins semantics asreqVars, and safely writes torequest.vars.resonly whenrequest.varsexists. This looks consistent and correct.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/bruno-app/src/components/CollectionSettings/Vars/index.js(1 hunks)packages/bruno-app/src/components/CollectionSettings/index.js(2 hunks)packages/bruno-app/src/components/FolderSettings/Vars/index.js(1 hunks)packages/bruno-app/src/components/FolderSettings/index.js(1 hunks)packages/bruno-app/src/components/RequestPane/HttpRequestPane/index.js(1 hunks)packages/bruno-app/src/components/RequestPane/Vars/index.js(1 hunks)packages/bruno-cli/src/runner/run-single-request.js(1 hunks)packages/bruno-cli/src/utils/collection.js(1 hunks)packages/bruno-electron/src/ipc/network/index.js(1 hunks)packages/bruno-electron/src/utils/collection.js(1 hunks)tests/request/newlines/newlines-persistence.spec.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-app/src/components/FolderSettings/Vars/index.jspackages/bruno-app/src/components/RequestPane/HttpRequestPane/index.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-app/src/components/RequestPane/Vars/index.jstests/request/newlines/newlines-persistence.spec.tspackages/bruno-cli/src/runner/run-single-request.jspackages/bruno-app/src/components/FolderSettings/index.jspackages/bruno-cli/src/utils/collection.jspackages/bruno-app/src/components/CollectionSettings/Vars/index.jspackages/bruno-electron/src/utils/collection.jspackages/bruno-app/src/components/CollectionSettings/index.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
tests/request/newlines/newlines-persistence.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/request/newlines/newlines-persistence.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
tests/request/newlines/newlines-persistence.spec.ts
🧬 Code graph analysis (7)
packages/bruno-app/src/components/RequestPane/HttpRequestPane/index.js (2)
packages/bruno-app/src/components/RequestPane/Vars/index.js (2)
responseVars(8-8)requestVars(7-7)packages/bruno-app/src/components/RequestPane/Settings/index.js (1)
getPropertyFromDraftOrRequest(24-25)
packages/bruno-electron/src/ipc/network/index.js (3)
packages/bruno-cli/src/runner/run-single-request.js (7)
postResponseVars(595-595)request(122-122)request(464-464)result(173-184)result(614-626)result(662-674)response(486-486)packages/bruno-app/src/utils/network/index.js (1)
result(255-255)packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
envVars(65-65)processEnvVars(66-66)
packages/bruno-app/src/components/RequestPane/Vars/index.js (3)
packages/bruno-app/src/components/CollectionSettings/Vars/index.js (2)
requestVars(10-10)responseVars(11-11)packages/bruno-app/src/components/FolderSettings/Vars/index.js (2)
requestVars(10-10)responseVars(11-11)packages/bruno-app/src/components/RequestPane/HttpRequestPane/index.js (2)
requestVars(101-101)responseVars(102-102)
tests/request/newlines/newlines-persistence.spec.ts (1)
tests/utils/page/locators.ts (1)
getTableCell(79-79)
packages/bruno-cli/src/runner/run-single-request.js (2)
packages/bruno-js/tests/runtime.spec.js (1)
VarsRuntime(5-5)packages/bruno-cli/src/commands/run.js (2)
runtimeVariables(334-334)processEnvVars(466-468)
packages/bruno-cli/src/utils/collection.js (5)
packages/bruno-electron/src/utils/collection.js (8)
collectionRoot(54-54)collectionRoot(154-154)collectionRoot(620-620)folderRoot(24-24)folderRoot(67-67)folderRoot(108-108)folderRoot(164-164)folderRoot(628-628)packages/bruno-cli/src/runner/prepare-request.js (3)
get(1-1)collectionRoot(51-51)request(16-16)packages/bruno-electron/src/ipc/network/index.js (2)
collectionRoot(324-324)request(325-325)packages/bruno-app/src/components/ResponsePane/Timeline/index.js (1)
collectionRoot(12-12)packages/bruno-app/src/components/FolderSettings/index.js (1)
folderRoot(23-23)
packages/bruno-app/src/components/CollectionSettings/Vars/index.js (2)
packages/bruno-app/src/components/CollectionSettings/index.js (2)
responseVars(43-45)requestVars(40-42)packages/bruno-app/src/utils/collections/index.js (1)
requestVars(1527-1527)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (7)
packages/bruno-app/src/components/CollectionSettings/Vars/index.js (1)
11-23: Post-response vars wiring and UI look consistent
responseVarsuses the same draft/root resolution pattern and safe[]default asrequestVars, and the Pre Request / Post Response sections with separateVarsTableinstances are consistent with Folder and Request panes. No issues from a data-flow or UX perspective.packages/bruno-cli/src/utils/collection.js (1)
202-235: Response vars merge mirrors request vars correctlyThe new
resVarsmerge follows the same pattern asreqvars: safeget(..., []), folder vs item handling, and last-writer-wins semantics across the tree, then normalizing intorequest.vars.reswithenabled: trueandtype: 'response'. No correctness concerns here.packages/bruno-app/src/components/RequestPane/HttpRequestPane/index.js (1)
102-111: Vars badge now correctly counts both pre- and post-response varsIncluding
responseVarsinactiveVarsLengthvia the same draft/request resolver keeps the badge in sync with both tables and is safe due to the[]default. Implementation is sound.packages/bruno-app/src/components/CollectionSettings/index.js (1)
35-60: Collection-level vars/auth/proxy wiring is consistent and safeThe refactor to resolve headers, request/response vars, auth, proxy, client certs, and protobuf via draft/root with explicit
getdefaults is consistent with other components;activeVarsCountcorrectly sums enabled request and response vars. No functional issues spotted.packages/bruno-app/src/components/FolderSettings/Vars/index.js (1)
11-23: Folder-level Pre/Post vars implementation is consistent
responseVarsreuse of the existing draft/root pattern plus the new Pre Request / Post Response sections andvarTypeflags matches the collection and request panes; no functional issues observed.packages/bruno-cli/src/runner/run-single-request.js (1)
594-607: CLI may need to await post-response vars and should confirm response var sourceTwo points to verify:
- Source of
postResponseVars— You're aggregating response vars intorequest.vars.resinmergeVars, but this reads fromitem:const postResponseVars = get(item, 'request.vars.res');Confirm whether merged collection/folder-level
request.vars.resshould be included here, and if so, update to read from the preparedrequestwith a fallback:- const postResponseVars = get(item, 'request.vars.res'); + const postResponseVars = get(request, 'vars.res', get(item, 'request.vars.res', []));
- Sync vs async execution — Verify whether
VarsRuntime.runPostResponseVarsis asynchronous. If it returns a Promise, addawaitto ensure vars are fully processed before post-response scripts and tests run.packages/bruno-app/src/components/FolderSettings/index.js (1)
31-32: Correctly including post-response vars in active countUsing a separate
responseVarsarray and summing enabled entries from both request and response scopes is clear, null-safe, and aligns with the existing pattern forrequestVars.
Description
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.