Mock Server Implementation#8734
Conversation
Port performance improvements from enterprise mock-server-port branch: batched request logs, route hit counts, debounced store writes, and bulk response loading.
WalkthroughAdds a beta-gated mock server platform with Electron-backed isolated servers, persisted responses, OpenAPI generation, request matching, Redux synchronization, dashboard and editor interfaces, mock-server tabs, and Jest/Playwright coverage. ChangesMock Server Platform
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/bruno-app/src/components/Preferences/Beta/index.js (1)
76-93: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNew toggle exposes a pre-existing broken debounce:
betaSchemais regenerated every render, sodebouncedSavenever actually debounces.
generateValidationSchema()builds a fresh Yup object on every render (Line 93), anddebouncedSave'suseCallbackdepends on it (Line 125-135), so a brand-newdebounce()wrapper is created each render. The auto-save effect (Line 138-145) then tears down/re-runs every render too, calling.flush()on the stale instance almost immediately — so saves fire near-instantly instead of after the intended 500ms pause. This was previously unreachable since no feature hadtoggle: true; the new Mock Server toggle is the first control that mutatesformik.valuesand triggers this path.🐛 Proposed fix: memoize the schema
- const betaSchema = generateValidationSchema(); + const betaSchema = useMemo(() => generateValidationSchema(), []);(Requires importing
useMemofromreactif not already imported.)Also applies to: 125-145, 171-182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/Preferences/Beta/index.js` around lines 76 - 93, Memoize the schema returned by generateValidationSchema so betaSchema remains stable across renders, importing useMemo from React if needed. Preserve the existing debouncedSave callback and auto-save effect behavior while ensuring their dependencies no longer recreate the debounce wrapper on every form update.packages/bruno-app/src/components/ResponseExample/ResponseExampleResponsePane/index.js (1)
44-97: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale persisted
'try-result'tab renders blank with no active tab highlighted.
activeTabcomes from persistedresponsePaneTab, which can stay'try-result'from a prior visit while localtryResultresets tonullon remount.tabConfigcorrectly hides the'try-result'tab button in that case, butgetTabPanelstill matchescase 'try-result'unconditionally, rendering an emptyMockResponseTryResultwith no tab visually active.🩹 Proposed fix
case 'try-result': { + if (!tryResult) { + return getTabPanel('response'); + } return ( <MockResponseTryResult🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/ResponseExample/ResponseExampleResponsePane/index.js` around lines 44 - 97, Guard the 'try-result' branch in getTabPanel so it renders MockResponseTryResult only when tryResult exists; otherwise fall back to the response tab/content and ensure the active tab is synchronized with the visible tabConfig entries when persisted responsePaneTab is stale.
🧹 Nitpick comments (14)
packages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorActions.spec.js (1)
9-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSmoke test only — no behavior coverage.
This only checks that the action creators exist as functions, not that
updateMockResponseRules,syncMockResponseEditorSaved, etc. actually produce the expected state changes. Consider replacing/augmenting with tests that dispatch these actions through the reducer and assert on resulting state (mirroring the behavior-driven style used intabs.spec.js).As per coding guidelines: "Write behavior-driven tests that assert observable outcomes rather than implementation details" and "Prioritize high-value tests covering critical, complex, or failure-prone behavior over maximizing coverage."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorActions.spec.js` around lines 9 - 17, Replace the function-existence smoke test in mockResponseEditorActions with behavior-driven reducer tests for the exported actions, especially updateMockResponseRules and syncMockResponseEditorSaved, plus the related editor actions. Dispatch each action against representative state and assert the resulting observable state changes, following the style of tabs.spec.js.Source: Coding guidelines
packages/bruno-app/src/utils/mock-server/mock-responses.route-path.spec.js (1)
1-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd required semicolons across the new JavaScript specs.
packages/bruno-app/src/utils/mock-server/mock-responses.route-path.spec.js#L1-L24: terminate imports, assertions, and declarations with semicolons.packages/bruno-app/src/utils/mock-server/mock-responses.spec.js#L1-L111: terminate imports, mock setup, declarations, and assertions with semicolons.packages/bruno-app/src/utils/mock-server/mock-responses.sync.spec.js#L1-L56: terminate imports, mock setup, declarations, and assertions with semicolons.packages/bruno-app/src/utils/mock-server/mock-responses/editor.spec.js#L1-L61: terminate imports, declarations, mutations, and assertions with semicolons.As per coding guidelines, “Terminate statements with semicolons.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/utils/mock-server/mock-responses.route-path.spec.js` around lines 1 - 24, Terminate all statements with semicolons in the affected specs: packages/bruno-app/src/utils/mock-server/mock-responses.route-path.spec.js lines 1-24, mock-responses.spec.js lines 1-111, mock-responses.sync.spec.js lines 1-56, and mock-responses/editor.spec.js lines 1-61. Update imports, mock setup, declarations, mutations, and assertions without changing test behavior.Source: Coding guidelines
packages/bruno-electron/tests/mock-response-store.test.js (1)
107-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReuse the shared
workspacePathinstead of a second localmkdtempSync.Both tests create their own temp dir and only clean it up via an explicit
fs.rmSyncat the end of the test body (not inafterEach/finally). If anexpect()fails earlier in either test, the temp directory leaks.♻️ Proposed fix
it('stores mock server metadata in workspace mockserver.yml', () => { - const workspacePath = fs.mkdtempSync(path.join(os.tmpdir(), 'bruno-mock-meta-')); const instance = { uid: 'mock-1', name: 'Dog API Mock Server', port: 4001, sourceType: 'collection', collectionUid: 'collection-1', globalDelay: 0, workspaceUid: 'workspace-1' }; saveMockServer(workspacePath, instance); const store = readWorkspaceStore(workspacePath); expect(store.mockServers['mock-1'].name).toBe('Dog API Mock Server'); expect(store.mockServers['mock-1'].port).toBe(4001); expect(store.mockServers['mock-1'].collectionUid).toBe('collection-1'); expect(store.mockServers['mock-1'].responses).toEqual([]); const instances = listMockServers(workspacePath, 'workspace-1'); expect(instances).toHaveLength(1); expect(instances[0].name).toBe('Dog API Mock Server'); - - fs.rmSync(workspacePath, { recursive: true, force: true }); });(Same pattern applies to the
preserves mock server metadata when saving responsestest.)Also applies to: 134-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/tests/mock-response-store.test.js` around lines 107 - 132, Update the mock server metadata tests, including “stores mock server metadata in workspace mockserver.yml” and “preserves mock server metadata when saving responses,” to reuse the shared workspacePath fixture instead of creating local directories with mkdtempSync. Remove the per-test temporary directory creation and explicit cleanup so shared lifecycle handling performs cleanup even when assertions fail.tests/mock-server/collection-mock-server.spec.ts (1)
283-283: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace the fixed 500ms sleep with an auto-retrying assertion.
page.waitForTimeout(500)before reading.no-match-labelcount is exactly the flaky pattern the guide warns against.♻️ Proposed fix
- await page.getByTestId('mock-server-tab-log').click(); - await page.waitForTimeout(500); - const noMatchLabels = page.locator('.log-table-container .no-match-label'); - expect(await noMatchLabels.count()).toBeGreaterThan(0); + await page.getByTestId('mock-server-tab-log').click(); + await expect(noMatchLabels.first()).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/mock-server/collection-mock-server.spec.ts` at line 283, Replace the fixed page.waitForTimeout(500) delay in the test before reading the .no-match-label count with an auto-retrying assertion that waits until the expected UI state is reached, then perform the count check. Remove the arbitrary sleep while preserving the test’s existing validation.Source: Path instructions
packages/bruno-app/src/components/FilterDropdown/index.js (1)
74-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffOptions aren't keyboard/AT operable. The menu items are bare
<div onClick>— norole,tabIndex, or key handling — so the dropdown can only be driven by mouse. Considerrole="listbox"/role="option"withtabIndexand Enter/Space (and ideally arrow) handling, or render options as<button>.(The ast-grep "needs key" warnings on lines 96–104 are false positives — those elements are children of the already-keyed
<div key={option.value}>.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/FilterDropdown/index.js` around lines 74 - 108, Make the menu in FilterDropdown keyboard and assistive-technology operable by replacing the clickable option divs with accessible controls, preferably buttons, or by adding appropriate listbox/option roles, focusability, and Enter/Space activation. Update both the all-options control and each mapped option around handleSelect, preserving their selected-state styling and behavior.packages/bruno-app/src/components/MockServer/RenameMockServerModal/index.js (1)
51-58: 📐 Maintainability & Code Quality | 🔵 TrivialGeneric error message swallows the actual failure reason.
saveMockServerInstancethrows specific errors (e.g. missing workspace path, IPC failure reason) per its implementation, but the catch here always shows'Failed to rename mock server', discarding that detail from the user.♻️ Surface the actual error message
} catch { - toast.error('Failed to rename mock server'); + } catch (err) { + toast.error(err?.message || 'Failed to rename mock server'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/MockServer/RenameMockServerModal/index.js` around lines 51 - 58, Update the catch block in the RenameMockServerModal save flow to capture the thrown error and pass its actual message to toast.error, while retaining the generic rename failure text as a fallback when no usable message is available.packages/bruno-app/src/components/MockServer/MockResponse/MockResponseRules/index.js (1)
80-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a stable id instead of array index as the row key.
key={rule-${index}}(derived from array index) can cause React to misassociate DOM/input state when a condition in the middle of the list is removed. Since all fields are controlled, this is unlikely to cause visible bugs today, but a stable per-condition id (generated on add) would be more robust against future changes (e.g. drag-reorder).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/MockServer/MockResponse/MockResponseRules/index.js` around lines 80 - 124, Replace the index-based React key in the conditions map with a stable per-condition identifier, and ensure that identifier is generated when a condition is added and preserved through updates and removals. Use that identifier for the row key while continuing to pass the array index to updateCondition and removeCondition as needed.packages/bruno-app/src/providers/ReduxStore/middlewares/debug/middleware.js (1)
6-11: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the no-op listener.
Commenting out the effect body still runs this listener for every Redux action. Remove the registration, or enable it only behind an explicit debug flag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/providers/ReduxStore/middlewares/debug/middleware.js` around lines 6 - 11, Remove the no-op listener registration containing predicate and effect, or gate that registration behind an explicit debug flag so it does not run for every Redux action by default.Source: Coding guidelines
packages/bruno-electron/src/app/mock-server/mock-spec-routes.js (1)
38-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
pickResponseappears unused. Not referenced within the file nor exported (onlylistOperationResponsesis used). Drop it if it isn't consumed elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/app/mock-server/mock-spec-routes.js` around lines 38 - 53, Remove the unused pickResponse function, since it is neither referenced in the file nor exported and listOperationResponses is the consumed response-selection path. Preserve all remaining mock-server route behavior unchanged.packages/bruno-electron/src/app/mock-server/mock-server.js (1)
432-460: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
tryMockRequesthas no request timeout.
transport.request(...)only rejects on'error'. If the target accepts the socket but never responds, the promise never settles and the renderer's "Try" flow hangs indefinitely. Add a socket timeout that destroys the request and rejects.♻️ Proposed fix
const req = transport.request({ hostname: parsedUrl.hostname, port: parsedUrl.port || (parsedUrl.protocol === 'https:' ? 443 : 80), path: `${parsedUrl.pathname}${parsedUrl.search}`, method: method.toUpperCase(), - headers: requestHeaders + headers: requestHeaders, + timeout: 30000 }, (res) => { const chunks = []; res.on('data', (chunk) => chunks.push(chunk)); res.on('end', () => { resolve({ status: res.statusCode, statusText: res.statusMessage, headers: res.headers, body: Buffer.concat(chunks).toString('utf8'), url }); }); }); req.on('error', reject); + req.on('timeout', () => { + req.destroy(new Error('Try request timed out')); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/app/mock-server/mock-server.js` around lines 432 - 460, Update tryMockRequest to configure a request/socket timeout immediately after transport.request creates req. When the timeout fires, destroy the request with an appropriate timeout error so the existing req.on('error', reject) path settles the promise, while preserving the current response, payload, and completion handling.packages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorReducers.js (1)
60-69: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInconsistent cloning:
rulesis assigned by reference here, unlike everywhere else in this file.
initMockResponseEditor,syncMockResponseEditorSaved, andcancelMockResponseEditorEditallcloneDeepthe rules object before storing it;updateMockResponseRulesdoesn't. If the dispatching component keeps a reference to the samerulesobject and mutates it later, Immer's dev-mode auto-freeze will throw, or (in prod) it creates shared mutable state between the UI and the store.♻️ Proposed fix
+import { cloneDeep } from 'lodash'; + export const updateMockResponseRules = (state, action) => { const { responseUid, rules } = action.payload; const editor = state.mockResponseEditors[responseUid]; if (!editor) { return; } - editor.rules = rules; + editor.rules = cloneDeep(rules); };(
cloneDeepis already imported at the top of this file.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorReducers.js` around lines 60 - 69, Update updateMockResponseRules so it deep-clones the incoming rules before assigning them to editor.rules, matching the cloning behavior in initMockResponseEditor, syncMockResponseEditorSaved, and cancelMockResponseEditorEdit. Reuse the existing cloneDeep import.packages/bruno-electron/src/ipc/openapi-sync.js (1)
334-334: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame reserved-folder list duplicated 5x — use
RESERVED_FOLDER_NAMESeverywhere.Adding
'mocks'required touching 5 separate literal arrays instead of one constant. Next addition/removal risks drifting out of sync again.♻️ Proposed refactor
-const findRequestFileOnDisk = (dirPath, method, urlPath) => { +const RESERVED_FOLDER_NAMES = ['node_modules', '.git', 'environments', 'mocks']; + +const findRequestFileOnDisk = (dirPath, method, urlPath) => { if (!fs.existsSync(dirPath)) return null; const files = fs.readdirSync(dirPath); for (const file of files) { const filePath = path.join(dirPath, file); const stats = fs.statSync(filePath); - if (stats.isDirectory() && !['node_modules', '.git', 'environments', 'mocks'].includes(file)) { + if (stats.isDirectory() && !RESERVED_FOLDER_NAMES.includes(file)) {Then replace the remaining three literals (scanCollectionFiles, findAndResetRequest, findAndRemoveRequest) with
RESERVED_FOLDER_NAMES.includes(...), and drop the later duplicateconst RESERVED_FOLDER_NAMES = [...]declaration at line 704.Also applies to: 704-704, 1132-1132, 1400-1400, 1485-1485
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/ipc/openapi-sync.js` at line 334, Centralize the reserved-folder names in a single RESERVED_FOLDER_NAMES declaration and use it for every directory exclusion check, including the current condition and the checks in scanCollectionFiles, findAndResetRequest, and findAndRemoveRequest. Remove the later duplicate declaration so all five call sites reference the same constant.packages/bruno-app/src/utils/mock-server/mock-responses.js (1)
40-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrip host remnants in the URL parse fallback.
http://{{host}}/foois already handled, but malformed hosts such ashttps://{{host}}:{{port}}/bar/baz?x=1#horhttp://bad host/pathcan throw fromnew URL(); the current catch only removes the query string, leaving/https:/:host::port/bar/bazor/http:/bad host/pathas the route key. Reuse the scheme/host stripping pattern in the catch branch before building the mock route key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/utils/mock-server/mock-responses.js` around lines 40 - 48, Update the URL parse fallback in the mock response URL-cleaning logic to strip the scheme and host portion before removing query fragments. Reuse the existing scheme/host stripping pattern so malformed URLs such as templated or space-containing hosts produce only the path, then preserve fragment/query cleanup before the mock route key is built.packages/bruno-app/src/components/RequestTabPanel/index.js (1)
446-487: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
instanceCollectionresolution betweenmock-serverandmock-responsebranches.The
instance.sourceType === 'collection' ? find(...) : ...block is identical in both branches. Consider extracting a small helper (e.g.resolveInstanceCollection(instance, focusedTab, collections)) to avoid drift if the fallback logic changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/RequestTabPanel/index.js` around lines 446 - 487, Extract the duplicated instanceCollection resolution into a shared helper, such as resolveInstanceCollection, and reuse it in both the mock-server and mock-response branches of the focused tab rendering logic. Preserve the existing sourceType and focusedTab.collectionUid fallback behavior while removing the repeated conditional blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-app/src/components/MockServer/CreateMockServerModal/index.js`:
- Around line 303-321: Memoize the configured instance list returned by the
Redux selector so its reference changes only when the store contents change.
Update the effect using suggestAvailableMockServerPort to depend on that
memoized result (along with its other required dependencies), preventing
repeated setFieldValue calls caused by a newly created Object.values(...).flat()
array on each render.
In
`@packages/bruno-app/src/components/MockServer/MockResponse/RenameMockResponseModal/index.js`:
- Around line 57-61: Remove the local onKeyDown handler from the input in
RenameMockResponseModal so Modal remains the sole Enter-key handler and
handleConfirm cannot dispatch saveMockResponse twice. Keep handleConfirm guarded
by name.trim() as the only confirmation path.
In `@packages/bruno-app/src/components/MockServer/MockServerDashboard/index.js`:
- Around line 136-141: Update validatePort to return the freshly computed
validation error string or null, while retaining its existing state update
behavior. In handleStart, use the returned validation message for toast.error
instead of the rendered portError value, preserving the fallback message when no
specific error is returned.
In
`@packages/bruno-app/src/components/MockServer/MockServerDashboard/RequestLog/index.js`:
- Around line 219-224: Update the auto-expansion effect in the MockServer
request-log component so new matching logs only select the latest trace when no
trace is currently selected. Preserve the user’s existing expandedLogUid
selection, including when live traffic adds entries, and keep the latest-entry
auto-expansion for the unselected state.
In
`@packages/bruno-app/src/components/MockServer/MockServerDashboard/RouteTable/index.js`:
- Around line 72-84: Add an aria-label to the icon-only copy button in the route
table JSX, alongside its existing title, using a clear label describing that it
copies the route URL. Keep the current click handling and copied-state icon
rendering unchanged.
- Around line 42-54: Update handleCopyRouteUrl to track the pending copied-state
timeout with a useRef, clearing the previous timer before scheduling a new one.
Ensure each timer only clears the current copiedRouteUid after its own
1.5-second display period, so an earlier route cannot clear a newer route’s
state.
In
`@packages/bruno-app/src/components/MockServer/RequestTabs/MockResponseTab/index.js`:
- Around line 12-15: Update handleCloseClick in MockResponseTab to detect
unsaved edits in item.draft before dispatching closeTabs. Persist the draft
through the existing save flow by default, and only close without saving when
the user explicitly chooses discard, matching the draft-aware tab behavior; do
not dispatch immediate closure for an unsaved draft.
In `@packages/bruno-app/src/components/MockServer/Sidebar/MockServers/index.js`:
- Around line 194-203: The dashboard trigger around openDashboard is a
non-focusable span, preventing keyboard activation. Update this clickable
element to be keyboard accessible by adding an appropriate interactive role,
tabIndex, and keyboard handling that invokes openDashboard for standard
activation keys while preserving the existing click behavior.
In `@packages/bruno-app/src/components/ResponseExample/index.js`:
- Around line 27-28: Replace the content-based isNewExample heuristic in
ResponseExample with an explicit just-created signal propagated from the example
creation flow, such as the OPEN_EXAMPLE task queue. Initialize editMode from
that signal so JSON defaults and intentionally empty saved bodies retain correct
behavior, and preserve normal viewing mode for previously saved examples.
In `@packages/bruno-app/src/utils/beta-features.js`:
- Around line 7-14: The BETA_FEATURES shape is incompatible with the Beta
preferences consumer, which iterates it as an array and will fail at render.
Verify the current Beta component, then reconcile the shared contract: either
preserve the object-of-constants used by useBetaFeature, CollectionHeader, and
CollectionItem while updating Beta to iterate object entries and resolve
descriptor data appropriately, or convert all consumers consistently to an array
of feature descriptors keyed by id.
In `@packages/bruno-app/src/utils/snapshot/index.js`:
- Around line 498-511: Update the mock-response serialization branch in the
snapshot producer so it does not emit mockServerUid as null when
tab.mockServerUid is absent. Only include serialized.mockServerUid when a valid
value exists, while preserving responseUid and name serialization.
In `@packages/bruno-electron/src/app/collection-watcher.js`:
- Line 813: Update defaultIgnores in the collection watcher so mocks is excluded
only when it is the collection root’s mocks directory, not when mocks appears in
nested paths; preserve segment-agnostic ignoring for node_modules and .git as
appropriate, and adjust the ignore matching logic to distinguish the
collection-root path.
In `@packages/bruno-electron/src/app/mock-server/mock-example-generator.js`:
- Around line 97-103: Update the object branch in schemaToExample so each
sibling property recursion receives an independent copy of refStack, matching
the array branch’s new Set(refStack) behavior. Keep refStack path-local while
preserving generation of repeated references to the same component schema across
sibling properties.
In `@packages/bruno-electron/src/services/snapshot/index.js`:
- Around line 32-48: Update the snapshot validation schema’s mockServerUid field
to accept null values while remaining optional, and preserve nullable handling
wherever mockServerUid is consumed downstream. Ensure serializeTab-produced null
UIDs validate successfully so saveSnapshot can complete the write.
In `@packages/bruno-electron/src/store/preferences.js`:
- Around line 148-161: Update the mockServer.instances schema in the preferences
validation to include 'manual' in the sourceType allowed values, while
preserving the existing 'collection' and 'spec' values and required constraint
so legacy instances pass re-validation during hydrateMockServerInstances and
clearLegacyMockServerPrefs.
In `@tests/mock-server/collection-mock-server.spec.ts`:
- Around line 10-45: Move mock-server locators and UI interactions from
collection-mock-server.spec.ts into a dedicated tests/utils/page mock-server
module, including the flows currently implemented by openMockServerTab,
syncResponsesFromExamples, startMockServer, and stopMockServer, plus repeated
table, log, and filter interactions in the tests. Keep expect assertions in the
spec; page helpers should only perform actions and synchronization, returning
values such as the detected port when needed. Replace CSS selectors with
getByTestId or semantic locators wherever available, and update the spec to use
the new page-module methods.
---
Outside diff comments:
In `@packages/bruno-app/src/components/Preferences/Beta/index.js`:
- Around line 76-93: Memoize the schema returned by generateValidationSchema so
betaSchema remains stable across renders, importing useMemo from React if
needed. Preserve the existing debouncedSave callback and auto-save effect
behavior while ensuring their dependencies no longer recreate the debounce
wrapper on every form update.
In
`@packages/bruno-app/src/components/ResponseExample/ResponseExampleResponsePane/index.js`:
- Around line 44-97: Guard the 'try-result' branch in getTabPanel so it renders
MockResponseTryResult only when tryResult exists; otherwise fall back to the
response tab/content and ensure the active tab is synchronized with the visible
tabConfig entries when persisted responsePaneTab is stale.
---
Nitpick comments:
In `@packages/bruno-app/src/components/FilterDropdown/index.js`:
- Around line 74-108: Make the menu in FilterDropdown keyboard and
assistive-technology operable by replacing the clickable option divs with
accessible controls, preferably buttons, or by adding appropriate listbox/option
roles, focusability, and Enter/Space activation. Update both the all-options
control and each mapped option around handleSelect, preserving their
selected-state styling and behavior.
In
`@packages/bruno-app/src/components/MockServer/MockResponse/MockResponseRules/index.js`:
- Around line 80-124: Replace the index-based React key in the conditions map
with a stable per-condition identifier, and ensure that identifier is generated
when a condition is added and preserved through updates and removals. Use that
identifier for the row key while continuing to pass the array index to
updateCondition and removeCondition as needed.
In `@packages/bruno-app/src/components/MockServer/RenameMockServerModal/index.js`:
- Around line 51-58: Update the catch block in the RenameMockServerModal save
flow to capture the thrown error and pass its actual message to toast.error,
while retaining the generic rename failure text as a fallback when no usable
message is available.
In `@packages/bruno-app/src/components/RequestTabPanel/index.js`:
- Around line 446-487: Extract the duplicated instanceCollection resolution into
a shared helper, such as resolveInstanceCollection, and reuse it in both the
mock-server and mock-response branches of the focused tab rendering logic.
Preserve the existing sourceType and focusedTab.collectionUid fallback behavior
while removing the repeated conditional blocks.
In `@packages/bruno-app/src/providers/ReduxStore/middlewares/debug/middleware.js`:
- Around line 6-11: Remove the no-op listener registration containing predicate
and effect, or gate that registration behind an explicit debug flag so it does
not run for every Redux action by default.
In
`@packages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorActions.spec.js`:
- Around line 9-17: Replace the function-existence smoke test in
mockResponseEditorActions with behavior-driven reducer tests for the exported
actions, especially updateMockResponseRules and syncMockResponseEditorSaved,
plus the related editor actions. Dispatch each action against representative
state and assert the resulting observable state changes, following the style of
tabs.spec.js.
In
`@packages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorReducers.js`:
- Around line 60-69: Update updateMockResponseRules so it deep-clones the
incoming rules before assigning them to editor.rules, matching the cloning
behavior in initMockResponseEditor, syncMockResponseEditorSaved, and
cancelMockResponseEditorEdit. Reuse the existing cloneDeep import.
In `@packages/bruno-app/src/utils/mock-server/mock-responses.js`:
- Around line 40-48: Update the URL parse fallback in the mock response
URL-cleaning logic to strip the scheme and host portion before removing query
fragments. Reuse the existing scheme/host stripping pattern so malformed URLs
such as templated or space-containing hosts produce only the path, then preserve
fragment/query cleanup before the mock route key is built.
In `@packages/bruno-app/src/utils/mock-server/mock-responses.route-path.spec.js`:
- Around line 1-24: Terminate all statements with semicolons in the affected
specs:
packages/bruno-app/src/utils/mock-server/mock-responses.route-path.spec.js lines
1-24, mock-responses.spec.js lines 1-111, mock-responses.sync.spec.js lines
1-56, and mock-responses/editor.spec.js lines 1-61. Update imports, mock setup,
declarations, mutations, and assertions without changing test behavior.
In `@packages/bruno-electron/src/app/mock-server/mock-server.js`:
- Around line 432-460: Update tryMockRequest to configure a request/socket
timeout immediately after transport.request creates req. When the timeout fires,
destroy the request with an appropriate timeout error so the existing
req.on('error', reject) path settles the promise, while preserving the current
response, payload, and completion handling.
In `@packages/bruno-electron/src/app/mock-server/mock-spec-routes.js`:
- Around line 38-53: Remove the unused pickResponse function, since it is
neither referenced in the file nor exported and listOperationResponses is the
consumed response-selection path. Preserve all remaining mock-server route
behavior unchanged.
In `@packages/bruno-electron/src/ipc/openapi-sync.js`:
- Line 334: Centralize the reserved-folder names in a single
RESERVED_FOLDER_NAMES declaration and use it for every directory exclusion
check, including the current condition and the checks in scanCollectionFiles,
findAndResetRequest, and findAndRemoveRequest. Remove the later duplicate
declaration so all five call sites reference the same constant.
In `@packages/bruno-electron/tests/mock-response-store.test.js`:
- Around line 107-132: Update the mock server metadata tests, including “stores
mock server metadata in workspace mockserver.yml” and “preserves mock server
metadata when saving responses,” to reuse the shared workspacePath fixture
instead of creating local directories with mkdtempSync. Remove the per-test
temporary directory creation and explicit cleanup so shared lifecycle handling
performs cleanup even when assertions fail.
In `@tests/mock-server/collection-mock-server.spec.ts`:
- Line 283: Replace the fixed page.waitForTimeout(500) delay in the test before
reading the .no-match-label count with an auto-retrying assertion that waits
until the expected UI state is reached, then perform the count check. Remove the
arbitrary sleep while preserving the test’s existing validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 636122bd-8d89-44ae-8be5-31b015714752
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (101)
.gitignorepackage.jsonpackages/bruno-app/src/components/FilterDropdown/StyledWrapper.jspackages/bruno-app/src/components/FilterDropdown/index.jspackages/bruno-app/src/components/MockServer/CloneMockServerModal/index.jspackages/bruno-app/src/components/MockServer/CreateMockServerModal/index.jspackages/bruno-app/src/components/MockServer/DeleteMockServerModal/index.jspackages/bruno-app/src/components/MockServer/MockResponse/CreateMockResponsePanel/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockResponse/CreateMockResponsePanel/index.jspackages/bruno-app/src/components/MockServer/MockResponse/DeleteMockResponseModal/index.jspackages/bruno-app/src/components/MockServer/MockResponse/GenerateFromSpecModal/index.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponseRequestPane/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponseRequestPane/index.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponseRules/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponseRules/index.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponseTopBar/index.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponseTryResult/index.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponsesList/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockResponse/MockResponsesList/index.jspackages/bruno-app/src/components/MockServer/MockResponse/RenameMockResponseModal/index.jspackages/bruno-app/src/components/MockServer/MockResponse/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockResponse/SyncFromExamplesModal/index.jspackages/bruno-app/src/components/MockServer/MockResponse/SyncWithSpecModal/index.jspackages/bruno-app/src/components/MockServer/MockResponse/index.jspackages/bruno-app/src/components/MockServer/MockServerDashboard/RequestLog/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockServerDashboard/RequestLog/index.jspackages/bruno-app/src/components/MockServer/MockServerDashboard/RouteTable/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockServerDashboard/RouteTable/index.jspackages/bruno-app/src/components/MockServer/MockServerDashboard/StyledWrapper.jspackages/bruno-app/src/components/MockServer/MockServerDashboard/index.jspackages/bruno-app/src/components/MockServer/RenameMockServerModal/index.jspackages/bruno-app/src/components/MockServer/RequestTabs/MockResponseTab/index.jspackages/bruno-app/src/components/MockServer/Sidebar/MockServers/MockResponseSidebarItem/index.jspackages/bruno-app/src/components/MockServer/Sidebar/MockServers/index.jspackages/bruno-app/src/components/Preferences/Beta/index.jspackages/bruno-app/src/components/RequestTabPanel/index.jspackages/bruno-app/src/components/RequestTabs/CollectionHeader/index.jspackages/bruno-app/src/components/RequestTabs/RequestTab/SpecialTab.jspackages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/ResponseExample/CreateExampleModal/index.jspackages/bruno-app/src/components/ResponseExample/ResponseExampleResponsePane/index.jspackages/bruno-app/src/components/ResponseExample/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/index.jspackages/bruno-app/src/components/Sidebar/Sections/MockServersSection/index.jspackages/bruno-app/src/components/Sidebar/index.jspackages/bruno-app/src/providers/App/useIpcEvents.jspackages/bruno-app/src/providers/ReduxStore/index.jspackages/bruno-app/src/providers/ReduxStore/middlewares/debug/middleware.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/exampleReducers.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorActions.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorActions.spec.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/mockResponseEditorReducers.jspackages/bruno-app/src/providers/ReduxStore/slices/mock-server/index.jspackages/bruno-app/src/providers/ReduxStore/slices/tabs.jspackages/bruno-app/src/providers/ReduxStore/slices/tabs.spec.jspackages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.jspackages/bruno-app/src/utils/beta-features.jspackages/bruno-app/src/utils/mock-server/mock-responses.jspackages/bruno-app/src/utils/mock-server/mock-responses.route-path.spec.jspackages/bruno-app/src/utils/mock-server/mock-responses.spec.jspackages/bruno-app/src/utils/mock-server/mock-responses.sync.spec.jspackages/bruno-app/src/utils/mock-server/mock-responses/editor.jspackages/bruno-app/src/utils/mock-server/mock-responses/editor.spec.jspackages/bruno-app/src/utils/mock-server/mock-server-instances.jspackages/bruno-app/src/utils/mock-server/mock-server-instances.spec.jspackages/bruno-app/src/utils/mock-server/mock-server-log-subscription.jspackages/bruno-app/src/utils/snapshot/index.jspackages/bruno-app/src/utils/snapshot/index.spec.jspackages/bruno-electron/package.jsonpackages/bruno-electron/src/app/collection-watcher.jspackages/bruno-electron/src/app/mock-server/mock-example-generator.jspackages/bruno-electron/src/app/mock-server/mock-response-routes.jspackages/bruno-electron/src/app/mock-server/mock-response-store.jspackages/bruno-electron/src/app/mock-server/mock-rule-matcher.jspackages/bruno-electron/src/app/mock-server/mock-server-routing.jspackages/bruno-electron/src/app/mock-server/mock-server.jspackages/bruno-electron/src/app/mock-server/mock-spec-loader.jspackages/bruno-electron/src/app/mock-server/mock-spec-routes.jspackages/bruno-electron/src/index.jspackages/bruno-electron/src/ipc/mock-server/index.jspackages/bruno-electron/src/ipc/network/axios-instance.jspackages/bruno-electron/src/ipc/openapi-sync.jspackages/bruno-electron/src/services/snapshot/index.jspackages/bruno-electron/src/store/preferences.jspackages/bruno-electron/tests/mock-response-routes.test.jspackages/bruno-electron/tests/mock-response-store.test.jspackages/bruno-electron/tests/mock-rule-matcher.test.jspackages/bruno-electron/tests/mock-server-routing.test.jspackages/bruno-electron/tests/mock-spec-routes.test.jsplaywright.config.tstests/mock-server/collection-mock-server.spec.tstests/mock-server/fixtures/collections/mock-server-other-collection/bruno.jsontests/mock-server/fixtures/collections/mock-server-other-collection/health/healthcheck.brutests/mock-server/fixtures/collections/mock-server-test-collection/bruno.jsontests/mock-server/fixtures/collections/mock-server-test-collection/health/healthcheck.brutests/mock-server/fixtures/collections/mock-server-test-collection/users/create-user.brutests/mock-server/fixtures/collections/mock-server-test-collection/users/get-user.brutests/mock-server/init-user-data/preferences.json
| useEffect(() => { | ||
| if (isEditing) { | ||
| return; | ||
| } | ||
|
|
||
| let cancelled = false; | ||
|
|
||
| suggestAvailableMockServerPort(configuredInstances, { | ||
| excludeUid: editingInstance?.uid | ||
| }).then((port) => { | ||
| if (!cancelled) { | ||
| formik.setFieldValue('port', port); | ||
| } | ||
| }).catch(() => {}); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [isEditing, configuredInstances, editingInstance?.uid]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect getMockServerInstances to determine if it returns a stable/memoized reference.
fd -a 'mock-server-instances.js' packages/bruno-app/src/utils | while read -r f; do
echo "== $f =="
ast-grep run --pattern 'export const getMockServerInstances = $_' --lang javascript "$f" || \
rg -nP -A15 'getMockServerInstances\s*=' "$f"
doneRepository: usebruno/bruno
Length of output: 1106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file outline =="
ast-grep outline packages/bruno-app/src/components/MockServer/CreateMockServerModal/index.js --match 'CreateMockServerModal' --view expanded || true
echo "== target imports/useSelector lines =="
sed -n '1,120p' packages/bruno-app/src/components/MockServer/CreateMockServerModal/index.js
echo "== effect lines =="
sed -n '290,330p' packages/bruno-app/src/components/MockServer/CreateMockServerModal/index.js
echo "== mock-server-instances.js =="
sed -n '1,90p' packages/bruno-app/src/utils/mock-server/mock-server-instances.js
echo "== read-only behavioral probe of selector implementation =="
node - <<'JS'
function valuesKeys(x) {
return Object.fromEntries(Object.entries(x || {}).map(([k,v], i) => [i, v]));
}
const a = { x: 1 };
const b = { x: 1 };
console.log(JSON.stringify({
flatReturnsNewEachTime: JSON.stringify(valuesKeys({a:[1,2], b:[3,4]})) === JSON.stringify(valuesKeys({a:[1,2], b:[3,4]})),
directValuesIsNewReference: Object.values({a:[1,2]}) !== Object.values({a:[1,2]}),
flattenedValuesIsNewReference: ([1,2]).concat([3,4]) !== ([3,4]).concat([1,2]),
}));
JSRepository: usebruno/bruno
Length of output: 7898
🌐 Web query:
Redux Toolkit createSelector memoization array of objects selector getMockServerInstances
💡 Result:
In Redux Toolkit, createSelector provides memoization by caching the result of the output selector based on reference equality (===) of its input arguments [1][2]. When working with arrays of objects, memoization will only trigger efficiently if the references to those arrays and the objects within them remain stable [3][4]. Key Memoization Principles: - Input Selectors: These should only extract data from the state [1][2]. They are responsible for providing stable references to the result function [3][2]. If an input selector returns a new reference (e.g., creating a new array or object), the result function will re-run even if the underlying data is conceptually the same [3][4]. - Result Function: This function computes the derived data [1][5]. It only executes if the input selector results are different from their previous values based on strict reference equality [1][2]. - Default Behavior: Since Reselect 5.0.0, createSelector uses weakMapMemoize, which provides automatic memory management by cleaning up cached results when they are no longer referenced [1][2]. Regarding getMockServerInstances: There is no standard Redux Toolkit utility named getMockServerInstances. This appears to be a custom function or selector specific to your codebase, likely related to project-specific logic such as that found in the Hoppscotch codebase, which manages mock servers via backend services [6][7][8]. If you are creating a memoized selector for such instances: 1. Ensure the input selector to createSelector directly returns the state branch containing the mock servers [3][1]. 2. If you need to transform the data (e.g., mapping over instances), do this inside the result function [3][2]. 3. If the mock server instances are updated in the state as new object references, ensure your state management (e.g., reducers) maintains structural sharing to allow createSelector to correctly identify when specific items have changed [1][4]. If you find that your selector is re-calculating too frequently, use a debugging tool or console.log within your selectors to verify which input is triggering the re-run, and check if any upstream selectors are returning new object references on every execution [3][1].
Citations:
- 1: https://redux.js.org/usage/deriving-data-selectors
- 2: https://github.com/reduxjs/redux/blob/master/docs/usage/deriving-data-selectors.md
- 3: https://stackoverflow.com/questions/78813565/how-to-properly-use-createselector-to-memoize-a-selector-which-maps-over-an-obje
- 4: Understanding `createSlice` + `selectors` + memoization Current State reduxjs/redux-toolkit#4553
- 5: https://github.com/Reactjs/reselect
- 6: https://github.com/hoppscotch/hoppscotch/blob/d45903d2/packages/hoppscotch-common/src/newstore/mockServers.ts
- 7: https://github.com/hoppscotch/hoppscotch/blob/d45903d2/packages/hoppscotch-backend/src/mock-server/mock-server.service.ts
- 8: https://github.com/hoppscotch/hoppscotch/blob/d45903d2/packages/hoppscotch-backend/src/mock-server/mock-server.resolver.ts
Keep configuredInstances stable before adding it as an effect dependency. This effect reuses a new Object.values(...).flat() array from the Redux selector on each render unless the store contents change, which can trigger repeated setFieldValue('port', ...) calls. Memoize the selector result and include only that memoized result in the effect dependency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-app/src/components/MockServer/CreateMockServerModal/index.js`
around lines 303 - 321, Memoize the configured instance list returned by the
Redux selector so its reference changes only when the store contents change.
Update the effect using suggestAvailableMockServerPort to depend on that
memoized result (along with its other required dependencies), preventing
repeated setFieldValue calls caused by a newly created Object.values(...).flat()
array on each render.
| onKeyDown={(event) => { | ||
| if (event.key === 'Enter') { | ||
| handleConfirm(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm Modal's Enter handling for text inputs inside the modal
fd -t f 'index.js' packages/bruno-app/src/components/Modal --exec cat -n {}Repository: usebruno/bruno
Length of output: 6574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='packages/bruno-app/src/components/MockServer/MockResponse/RenameMockResponseModal/index.js'
if [ -f "$file" ]; then
wc -l "$file"
cat -n "$file"
else
fd -t f 'index.js' packages | grep -F 'MockResponse/RenameMockResponseModal/index.js' || true
fi
echo '--- imports/usages of RenameMockResponseModal ---'
rg -n "RenameMockResponseModal|saveMockResponse|setOpen\(false\)" packages/bruno-app/src/components/MockServer/MockResponse -S || trueRepository: usebruno/bruno
Length of output: 3334
Avoid double-invoking handleConfirm on Enter.
Modal already calls handleConfirm for Enter keys from text inputs inside the modal, and this input also calls it directly. One Enter will dispatch saveMockResponse twice; remove the local onKeyDown or call event.stopPropagation(), and keep handleConfirm only with a guard for name.trim().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/bruno-app/src/components/MockServer/MockResponse/RenameMockResponseModal/index.js`
around lines 57 - 61, Remove the local onKeyDown handler from the input in
RenameMockResponseModal so Modal remains the sole Enter-key handler and
handleConfirm cannot dispatch saveMockResponse twice. Keep handleConfirm guarded
by name.trim() as the only confirmation path.
| const handleStart = async () => { | ||
| const isValidPort = await validatePort(activePort); | ||
| if (!isValidPort) { | ||
| toast.error(portError || 'Fix the port before starting the mock server'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stale portError in the start toast. validatePort sets portError via setState, but the value read on Line 139 is the current render's (stale) portError, not the freshly-computed one — so the error toast can show the wrong/empty message. Return the message from validatePort and use it here.
Proposed fix
- const handleStart = async () => {
- const isValidPort = await validatePort(activePort);
- if (!isValidPort) {
- toast.error(portError || 'Fix the port before starting the mock server');
- return;
- }Have validatePort return the error string (or null) and toast that value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-app/src/components/MockServer/MockServerDashboard/index.js`
around lines 136 - 141, Update validatePort to return the freshly computed
validation error string or null, while retaining its existing state update
behavior. In handleStart, use the returned validation message for toast.error
instead of the rendered portError value, preserving the fallback message when no
specific error is returned.
| useEffect(() => { | ||
| const latestEntry = displayedLogs[0]; | ||
| if (latestEntry?.matchTrace) { | ||
| setExpandedLogUid(latestEntry.uid); | ||
| } | ||
| }, [displayedLogs.length, displayedLogs[0]?.uid]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Auto-expand on new logs overrides the user's manual trace selection.
This effect force-expands the newest entry's trace on every new request that has a matchTrace, even if the user manually expanded a different (older) entry to inspect it. Under live traffic this will keep yanking focus away from whatever the user is currently inspecting.
🐛 Proposed fix: don't override an existing selection
useEffect(() => {
const latestEntry = displayedLogs[0];
- if (latestEntry?.matchTrace) {
+ if (latestEntry?.matchTrace && !expandedLogUid) {
setExpandedLogUid(latestEntry.uid);
}
}, [displayedLogs.length, displayedLogs[0]?.uid]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| const latestEntry = displayedLogs[0]; | |
| if (latestEntry?.matchTrace) { | |
| setExpandedLogUid(latestEntry.uid); | |
| } | |
| }, [displayedLogs.length, displayedLogs[0]?.uid]); | |
| useEffect(() => { | |
| const latestEntry = displayedLogs[0]; | |
| if (latestEntry?.matchTrace && !expandedLogUid) { | |
| setExpandedLogUid(latestEntry.uid); | |
| } | |
| }, [displayedLogs.length, displayedLogs[0]?.uid]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/bruno-app/src/components/MockServer/MockServerDashboard/RequestLog/index.js`
around lines 219 - 224, Update the auto-expansion effect in the MockServer
request-log component so new matching logs only select the latest trace when no
trace is currently selected. Preserve the user’s existing expandedLogUid
selection, including when live traffic adds entries, and keep the latest-entry
auto-expansion for the unselected state.
| const handleCopyRouteUrl = async (routeUid, path) => { | ||
| if (!baseUrl) return; | ||
|
|
||
| const routePath = path.startsWith('/') ? path : `/${path}`; | ||
|
|
||
| try { | ||
| await navigator.clipboard.writeText(`${baseUrl}${routePath}`); | ||
| setCopiedRouteUid(routeUid); | ||
| setTimeout(() => setCopiedRouteUid(null), 1500); | ||
| } catch { | ||
| toast.error('Failed to copy URL'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stale timeout can prematurely clear a newer "copied" state.
Clicking route A then route B within 1.5s schedules two independent timers; A's timer fires later and unconditionally nulls copiedRouteUid, cutting short B's checkmark display early.
🐛 Proposed fix: track/clear the pending timeout
+ const copyTimeoutRef = useRef(null);
+
const handleCopyRouteUrl = async (routeUid, path) => {
if (!baseUrl) return;
const routePath = path.startsWith('/') ? path : `/${path}`;
try {
await navigator.clipboard.writeText(`${baseUrl}${routePath}`);
setCopiedRouteUid(routeUid);
- setTimeout(() => setCopiedRouteUid(null), 1500);
+ if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
+ copyTimeoutRef.current = setTimeout(() => setCopiedRouteUid(null), 1500);
} catch {
toast.error('Failed to copy URL');
}
};(Requires importing useRef from react.)
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 48-48: Avoid using the initial state variable in setState
Context: setCopiedRouteUid(routeUid)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/bruno-app/src/components/MockServer/MockServerDashboard/RouteTable/index.js`
around lines 42 - 54, Update handleCopyRouteUrl to track the pending
copied-state timeout with a useRef, clearing the previous timer before
scheduling a new one. Ensure each timer only clears the current copiedRouteUid
after its own 1.5-second display period, so an earlier route cannot clear a
newer route’s state.
| // Always ignore node_modules and .git, regardless of user config | ||
| // This prevents infinite loops with symlinked directories (e.g., npm workspaces) | ||
| const defaultIgnores = ['node_modules', '.git']; | ||
| const defaultIgnores = ['node_modules', '.git', 'mocks']; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any request files that live under a `mocks/` folder in fixtures/collections
fd -e bru . -x sh -c 'echo "$1" | grep -q "/mocks/" && echo "$1"' _ {}Repository: usebruno/bruno
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant code sections without running repository logic.
printf '--- collection-watcher relevant section ---\n'
sed -n '790,855p' packages/bruno-electron/src/app/collection-watcher.js | nl -ba -v790
printf '\n--- mock-example-generator relevant section ---\n'
file="$(fd 'mock-example-generator\.js$' packages | head -n1)"
if [ -n "${file:-}" ]; then
ast-grep outline "$file" --view expanded || true
rg -n "shouldSkipRequestFile|mocks|root" "$file" -C 4
fi
printf '\n--- all defaultIgnores / shouldSkip references ---\n'
rg -n "defaultIgnores|shouldSkipRequestFile|node_modules|mocks" packages -g '*.js' -g '*.ts' -g '*.tsx' -C 2
printf '\n--- package type-check setup ---\n'
fd 'package.json|tsconfig.json' packages/bruno-electron packages -x sh -c 'echo "=== $1 ==="; sed -n "1,120p" "$1"' sh {}Repository: usebruno/bruno
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe mirroring the segment-based defaultIgnores logic around the relevant code.
node - <<'JS'
const defaultIgnores = ['node_modules', '.git', 'mocks'];
// Mirrors likely match behavior: path.split by separator and checks any segment.
function shouldIgnore(relPath) {
const parts = relPath.split(/[\\/]/);
return parts.some(segment => defaultIgnores.includes(segment)) ||
parts.some(segment => segment.startsWith('.'));
}
const paths = [
'mocks/my-request.bru',
'collections/level/mocks/my-request.bru',
'src/mock/requests/mock.bru',
'data/node_modules/mynodefile.js',
'data/.git/config',
'requests/special.mocks/req.bru',
];
for (const p of paths) {
console.log(`${p} -> ${shouldIgnore(p)}`);
}
JSRepository: usebruno/bruno
Length of output: 369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant code sections without running repository logic.
printf '--- collection-watcher relevant section ---\n'
sed -n '790,855p' packages/bruno-electron/src/app/collection-watcher.js | nl -ba -v790
printf '\n--- mock-example-generator relevant section ---\n'
file="$(fd 'mock-example-generator\.js$' packages | head -n1)"
if [ -n "${file:-}" ]; then
ast-grep outline "$file" --view expanded || true
rg -n "shouldSkipRequestFile|mocks|root" "$file" -C 4
fi
printf '\n--- all defaultIgnores / shouldSkip references ---\n'
rg -n "defaultIgnores|shouldSkipRequestFile|node_modules|mocks" packages -g '*.js' -g '*.ts' -g '*.tsx' -C 2
printf '\n--- package type-check setup ---\n'
fd 'package.json|tsconfig.json' packages/bruno-electron packages -x sh -c 'echo "=== $1 ==="; sed -n "1,120p" "$1"' sh {}
# Behavioral probe mirroring the segment-based defaultIgnores logic around the relevant code.
printf '\n--- segment ignore probe ---\n'
node - <<'JS'
const defaultIgnores = ['node_modules', '.git', 'mocks'];
function shouldIgnore(relPath) {
const parts = relPath.split(/[\\/]/);
return parts.some(segment => defaultIgnores.includes(segment)) ||
parts.some(segment => segment.startsWith('.'));
}
const paths = [
'mocks/my-request.bru',
'collections/level/mocks/my-request.bru',
'src/mock/requests/mock.bru',
'data/node_modules/mynodefile.js',
'data/.git/config',
'requests/special.mocks/req.bru',
];
for (const p of paths) {
console.log(`${p} -> ${shouldIgnore(p)}`);
}
JSRepository: usebruno/bruno
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant code sections without running repository logic.
printf '%s\n' '--- collection-watcher relevant section ---'
sed -n '790,855p' packages/bruno-electron/src/app/collection-watcher.js | nl -ba -v790
printf '%s\n' '--- mock-example-generator relevant section ---'
file="$(fd 'mock-example-generator\.js$' packages | head -n1)"
if [ -n "${file:-}" ]; then
ast-grep outline "$file" --view expanded || true
rg -n "shouldSkipRequestFile|mocks|root" "$file" -C 4
fi
printf '%s\n' '--- all defaultIgnores / shouldSkip references ---'
rg -n "defaultIgnores|shouldSkipRequestFile|node_modules|mocks" packages -g '*.js' -g '*.ts' -g '*.tsx' -C 2
printf '%s\n' '--- package type-check setup ---'
fd 'package.json|tsconfig.json' packages/bruno-electron packages -x sh -c 'echo "=== $1 ==="; sed -n "1,120p" "$1"' sh {}
printf '%s\n' '--- segment ignore probe ---'
node - <<'JS'
const defaultIgnores = ['node_modules', '.git', 'mocks'];
function shouldIgnore(relPath) {
const parts = relPath.split(/[\\/]/);
return parts.some(segment => defaultIgnores.includes(segment)) ||
parts.some(segment => segment.startsWith('.'));
}
const paths = [
'mocks/my-request.bru',
'collections/level/mocks/my-request.bru',
'src/mock/requests/mock.bru',
'data/node_modules/mynodefile.js',
'data/.git/config',
'requests/special.mocks/req.bru',
];
for (const p of paths) {
console.log(`${p} -> ${shouldIgnore(p)}`);
}
JSRepository: usebruno/bruno
Length of output: 236
🏁 Script executed:
#!/bin/bash
set -u
# Locate and inspect the relevant code sections without running repository logic.
printf '%s\n' '--- collection-watcher relevant section ---'
sed -n '790,855p' packages/bruno-electron/src/app/collection-watcher.js | awk '{printf "%7d\t%s\n", NR+789, $0}'
printf '%s\n' '--- mock-example-generator relevant section ---'
file="$(fd 'mock-example-generator\.js$' packages 2>/dev/null | head -n1)"
if [ -n "${file:-}" ]; then
ast-grep outline "$file" --view expanded || true
rg -n "shouldSkipRequestFile|mocks|root" "$file" -C 4 || true
fi
printf '%s\n' '--- all defaultIgnores / shouldSkip references ---'
rg -n "defaultIgnores|shouldSkipRequestFile|node_modules|mocks" packages -g '*.js' -g '*.ts' -g '*.tsx' -C 2 || true
printf '%s\n' '--- package type-check setup ---'
fd 'package.json|tsconfig.json' packages/bruno-electron packages -x sh -c 'echo "=== $1 ==="; sed -n "1,120p" "$1"' sh {} || true
printf '%s\n' '--- segment ignore probe ---'
node - <<'JS'
const defaultIgnores = ['node_modules', '.git', 'mocks'];
function shouldIgnore(relPath) {
const parts = relPath.split(/[\\/]/);
return parts.some(segment => defaultIgnores.includes(segment)) ||
parts.some(segment => segment.startsWith('.'));
}
const paths = [
'mocks/my-request.bru',
'collections/level/mocks/my-request.bru',
'src/mock/requests/mock.bru',
'data/node_modules/mynodefile.js',
'data/.git/config',
'requests/special.mocks/req.bru',
];
for (const p of paths) {
console.log(`${p} -> ${shouldIgnore(p)}`);
}
JSRepository: usebruno/bruno
Length of output: 50370
Anchor the mocks ignore to the collection root.
defaultIgnores is applied as any path segment, so collections/level/mocks/my-request.bru is also ignored from watching. Keep the mocks exclude at the collection-level mocks directory only, while node_modules/.git can remain segment-agnostic if needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/app/collection-watcher.js` at line 813, Update
defaultIgnores in the collection watcher so mocks is excluded only when it is
the collection root’s mocks directory, not when mocks appears in nested paths;
preserve segment-agnostic ignoring for node_modules and .git as appropriate, and
adjust the ignore matching logic to distinguish the collection-root path.
| if (type === 'object' || resolvedSchema.properties) { | ||
| const result = {}; | ||
| for (const [key, value] of Object.entries(resolvedSchema.properties || {})) { | ||
| result[key] = schemaToExample(value, spec, depth + 1, refStack); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Shared refStack across sibling properties yields false cycle detection.
resolveOpenApiSchema permanently adds each $ref to refStack and never pops it. Because the object branch recurses with the same refStack reference, two sibling properties that reference the same component schema will drop the second one (resolved to null), producing incomplete examples. The array branch already isolates state with new Set(refStack) — mirror that here so refStack tracks the current path, not siblings.
🐛 Proposed fix
const result = {};
for (const [key, value] of Object.entries(resolvedSchema.properties || {})) {
- result[key] = schemaToExample(value, spec, depth + 1, refStack);
+ result[key] = schemaToExample(value, spec, depth + 1, new Set(refStack));
}
return result;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (type === 'object' || resolvedSchema.properties) { | |
| const result = {}; | |
| for (const [key, value] of Object.entries(resolvedSchema.properties || {})) { | |
| result[key] = schemaToExample(value, spec, depth + 1, refStack); | |
| } | |
| return result; | |
| } | |
| if (type === 'object' || resolvedSchema.properties) { | |
| const result = {}; | |
| for (const [key, value] of Object.entries(resolvedSchema.properties || {})) { | |
| result[key] = schemaToExample(value, spec, depth + 1, new Set(refStack)); | |
| } | |
| return result; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/app/mock-server/mock-example-generator.js` around
lines 97 - 103, Update the object branch in schemaToExample so each sibling
property recursion receives an independent copy of refStack, matching the array
branch’s new Set(refStack) behavior. Keep refStack path-local while preserving
generation of repeated references to the same component schema across sibling
properties.
| accessor: yup.string().oneOf([ | ||
| 'pathname', | ||
| 'pathname::exampleName', | ||
| 'pathname::exampleIndex', | ||
| 'type', | ||
| 'type::mockServerUid', | ||
| 'type::mockResponseUid' | ||
| ]).required(), | ||
| pathname: yup.string().nullable(), | ||
| permanent: yup.boolean().required(), | ||
| name: yup.string().optional(), | ||
| exampleName: yup.string().optional(), | ||
| exampleIndex: yup.number().integer().min(0).optional(), | ||
| exampleUid: yup.string().optional(), | ||
| mockServerUid: yup.string().optional(), | ||
| responseUid: yup.string().optional(), | ||
| tabName: yup.string().optional(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Yup treat null as invalid for a non-nullable string() schema even with strict: false?
💡 Result:
Yes, Yup treats null as invalid for a non-nullable string schema, even when strict mode is enabled. In Yup, schemas are non-nullable by default [1][2]. The string schema performs type checks to ensure that the input is indeed a string [3]. Because null is not a string, a non-nullable string schema will fail validation when it encounters a null value [1][4][2]. Using strict mode (strict: true) skips coercion and transformation attempts, validating the input "as-is" [1][4]. Enabling strict mode does not change the fundamental type requirement of the schema; rather, it prevents Yup from attempting to cast or coerce non-string values into strings [4][5]. Consequently, if the input is null, strict mode simply ensures that Yup evaluates the null value directly against the string type check, which fails because null is not a string [1][4]. To allow null values in a string schema, you must explicitly call.nullable [1][2]. If you want to accept null and treat it as a valid value, or if you need to handle nulls in specific ways (such as transforming them into empty strings), you would use methods like.nullable or.ensure [6][4].
Citations:
- 1: https://yup-docs.vercel.app/docs/Api/schema
- 2: https://github.com/jquense/yup
- 3: https://github.com/jquense/yup/blob/master/src/string.ts
- 4: https://github.com/jquense/yup/
- 5: Defined schemas validate and cast values as null jquense/yup#1059
- 6: https://yup-docs.vercel.app/docs/Api/types
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate relevant files:"
fd -a 'snapshot' . | sed 's#^\./##' | head -80
echo
echo "Search serializeTab and saveSnapshot:"
rg -n "serializeTab|mockServerUid|responseUid|saveSnapshot|validateSync|strict: false|tabs/collections" --glob '*.{js,jsx,ts,tsx}' . | head -200
echo
echo "Inspect electron snapshot service around schema:"
file=$(fd 'index.js|.*snapshot.*\.js' packages/bruno-electron/src/services | head -5)
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '1,120p' "$file"
sed -n '120,220p' "$file" || true
fi
echo
echo "Inspect app snapshot utility around serializeTab if available:"
app_file=$(fd 'index.js' packages/bruno-app/src/utils/snapshot | head -1)
if [ -n "${app_file:-}" ]; then
wc -l "$app_file"
sed -n '450,530p' "$app_file"
fiRepository: usebruno/bruno
Length of output: 26817
🏁 Script executed:
#!/bin/bash
set -u
echo "Inspect electron snapshot service around schema and saveSnapshot:"
sed -n '1,100p' packages/bruno-electronsrc/services/snapshot/index.js
sed -n '180,220p' packages/bruno-electronsrc/services/snapshot/index.js
echo
echo "Search app snapshot utility for tab serialization:"
rg -n "serializeTab|mockServerUid|mock-response" packages/bruno-app/src/utils/snapshot packages/bruno-auth/src/utils/snapshot packages/bruno-app/src --glob '*.{js,jsx,ts,tsx}' | head -160
echo
echo "Inspect snapshot util candidate files around matches:"
for f in $(rg -l "serializeTab" packages/bruno-app/src/utils/snapshot packages/bruno-app/src --glob '*.{js,jsx,ts,tsx}' | head -10); do
echo "--- $f"
wc -l "$f"
rg -n "serializeTab|mockServerUid|mock-response|mockServerUid:" "$f" -A5 -B5
doneRepository: usebruno/bruno
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Read correct snapshot service sections:"
sed -n '1,90p' packages/bruno-electron/src/services/snapshot/index.js
sed -n '180,220p' packages/bruno-electron/src/services/snapshot/index.js
sed -n '650,695p' packages/bruno-electron/src/services/snapshot/index.js
echo
echo "Determine package build/runtime TypeScript status:"
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('packages/bruno-electron/package.json', 'utf8'));
const deps = {...(pkg.dependencies || {}), ...(pkg.devDependencies || {})};
console.log({typescript: deps.typescript, scripts: pkg.scripts || {}});
JSRepository: usebruno/bruno
Length of output: 6086
Make mockServerUid nullable in the snapshot schema.
serializeTab sets mockServerUid to null for mock-response tabs when the source tab has no UID. Because this field is yup.string().optional() without .nullable(), validation fails and saveSnapshot returns false, dropping the snapshot write.
🩹 Proposed fix
pathname: yup.string().nullable(),
permanent: yup.boolean().required(),
name: yup.string().optional(),,
exampleName: yup.string().optional(),,
exampleIndex: yup.number().integer().min(0).optional(),
exampleUid: yup.string().optional(),
- mockServerUid: yup.string().optional(),
+ mockServerUid: yup.string().nullable().optional(),
responseUid: yup.string().optional(),
tabName: yup.string().optional(),Also keep this nullable where mockServerUid is consumed downstream as a possibly-null value.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| accessor: yup.string().oneOf([ | |
| 'pathname', | |
| 'pathname::exampleName', | |
| 'pathname::exampleIndex', | |
| 'type', | |
| 'type::mockServerUid', | |
| 'type::mockResponseUid' | |
| ]).required(), | |
| pathname: yup.string().nullable(), | |
| permanent: yup.boolean().required(), | |
| name: yup.string().optional(), | |
| exampleName: yup.string().optional(), | |
| exampleIndex: yup.number().integer().min(0).optional(), | |
| exampleUid: yup.string().optional(), | |
| mockServerUid: yup.string().optional(), | |
| responseUid: yup.string().optional(), | |
| tabName: yup.string().optional(), | |
| accessor: yup.string().oneOf([ | |
| 'pathname', | |
| 'pathname::exampleName', | |
| 'pathname::exampleIndex', | |
| 'type', | |
| 'type::mockServerUid', | |
| 'type::mockResponseUid' | |
| ]).required(), | |
| pathname: yup.string().nullable(), | |
| permanent: yup.boolean().required(), | |
| name: yup.string().optional(), | |
| exampleName: yup.string().optional(), | |
| exampleIndex: yup.number().integer().min(0).optional(), | |
| exampleUid: yup.string().optional(), | |
| mockServerUid: yup.string().nullable().optional(), | |
| responseUid: yup.string().optional(), | |
| tabName: yup.string().optional(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/services/snapshot/index.js` around lines 32 - 48,
Update the snapshot validation schema’s mockServerUid field to accept null
values while remaining optional, and preserve nullable handling wherever
mockServerUid is consumed downstream. Ensure serializeTab-produced null UIDs
validate successfully so saveSnapshot can complete the write.
| mockServer: Yup.object({ | ||
| instances: Yup.array().of(Yup.object({ | ||
| uid: Yup.string().required(), | ||
| name: Yup.string().required(), | ||
| sourceType: Yup.string().oneOf(['collection', 'spec']).required(), | ||
| collectionUid: Yup.string().nullable(), | ||
| specUid: Yup.string().nullable(), | ||
| specPath: Yup.string().nullable(), | ||
| specName: Yup.string().nullable(), | ||
| port: Yup.number().min(1).max(65535).required(), | ||
| globalDelay: Yup.number().min(0).required(), | ||
| workspaceUid: Yup.string().required() | ||
| })).optional() | ||
| }), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
sourceType enum is missing 'manual', breaking legacy-instance re-save.
createMockServerInstance and resolveMockServerStartPayload in packages/bruno-app/src/utils/mock-server/mock-server-instances.js treat 'manual' as a valid sourceType (it's even the default fallback). But this Yup schema only allows 'collection'/'spec'. Since hydrateMockServerInstances/clearLegacyMockServerPrefs re-validates the entire preferences object (including any not-yet-migrated mockServer.instances for other workspaces) via savePreferences, a lingering 'manual' legacy instance will throw a validation error during migration.
🐛 Proposed fix
- sourceType: Yup.string().oneOf(['collection', 'spec']).required(),
+ sourceType: Yup.string().oneOf(['collection', 'spec', 'manual']).required(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mockServer: Yup.object({ | |
| instances: Yup.array().of(Yup.object({ | |
| uid: Yup.string().required(), | |
| name: Yup.string().required(), | |
| sourceType: Yup.string().oneOf(['collection', 'spec']).required(), | |
| collectionUid: Yup.string().nullable(), | |
| specUid: Yup.string().nullable(), | |
| specPath: Yup.string().nullable(), | |
| specName: Yup.string().nullable(), | |
| port: Yup.number().min(1).max(65535).required(), | |
| globalDelay: Yup.number().min(0).required(), | |
| workspaceUid: Yup.string().required() | |
| })).optional() | |
| }), | |
| mockServer: Yup.object({ | |
| instances: Yup.array().of(Yup.object({ | |
| uid: Yup.string().required(), | |
| name: Yup.string().required(), | |
| sourceType: Yup.string().oneOf(['collection', 'spec', 'manual']).required(), | |
| collectionUid: Yup.string().nullable(), | |
| specUid: Yup.string().nullable(), | |
| specPath: Yup.string().nullable(), | |
| specName: Yup.string().nullable(), | |
| port: Yup.number().min(1).max(65535).required(), | |
| globalDelay: Yup.number().min(0).required(), | |
| workspaceUid: Yup.string().required() | |
| })).optional() | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/store/preferences.js` around lines 148 - 161,
Update the mockServer.instances schema in the preferences validation to include
'manual' in the sourceType allowed values, while preserving the existing
'collection' and 'spec' values and required constraint so legacy instances pass
re-validation during hydrateMockServerInstances and clearLegacyMockServerPrefs.
| const openMockServerTab = async (page: Page) => { | ||
| await page.locator('#sidebar-collection-name').filter({ hasText: new RegExp(`^${COLLECTION_NAME}$`) }).click(); | ||
| await page.getByTestId('mock-server').click(); | ||
| await page.getByTestId('mock-server-dashboard').waitFor({ state: 'visible' }); | ||
| }; | ||
|
|
||
| const syncResponsesFromExamples = async (page: Page) => { | ||
| await openMockServerTab(page); | ||
| await page.getByTestId('mock-server-tab-responses').click(); | ||
| await expect(page.getByTestId('mock-response-sync-examples-btn')).toBeVisible({ timeout: 10000 }); | ||
| await page.getByTestId('mock-response-sync-examples-btn').click(); | ||
| await expect(page.getByTestId('sync-mock-examples-modal')).toBeVisible({ timeout: 10000 }); | ||
| await page.getByTestId('sync-mock-examples-modal-submit-btn').click(); | ||
| await expect(page.getByText('Mock responses synced with collection examples')).toBeVisible({ timeout: 10000 }); | ||
| }; | ||
|
|
||
| const startMockServer = async (page: Page) => { | ||
| await page.getByTestId('mock-server-start-btn').click(); | ||
| const statusText = page.getByTestId('mock-server-status-text'); | ||
| await expect(statusText).toContainText('Running on port', { timeout: 15000 }); | ||
| const text = await statusText.innerText(); | ||
| const portMatch = text.match(/Running on port (\d+)/); | ||
| currentMockPort = portMatch ? portMatch[1] : DEFAULT_MOCK_PORT; | ||
| }; | ||
|
|
||
| const stopMockServer = async (page: Page) => { | ||
| await page.getByTestId('mock-server-stop-btn').click(); | ||
| await expect(page.getByTestId('mock-server-status-text')).toContainText('Stopped', { timeout: 15000 }); | ||
| }; | ||
|
|
||
| const mockFetch = async (urlPath: string, options?: RequestInit) => { | ||
| const res = await fetch(`${getMockBase()}${urlPath}`, options); | ||
| const contentType = res.headers.get('content-type') || ''; | ||
| const body = contentType.includes('json') ? await res.json() : await res.text(); | ||
| return { status: res.status, headers: res.headers, body }; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Extract locators/actions into tests/utils/page/* and keep assertions in the spec.
openMockServerTab, syncResponsesFromExamples, startMockServer, and stopMockServer inline raw page.locator(...)/page.getByTestId(...) calls directly here, and the same pattern repeats in every test body below (e.g. .table-container table tbody tr, .log-table-container .method-badge/.no-match-label/.status-code, .filter-option-label). These should live in a mock-server page module. Additionally, syncResponsesFromExamples (19, 21, 23), startMockServer (29), and stopMockServer (37) embed expect(...) assertions inside the helpers — the guide requires assertions to stay in the spec while helpers only wait/synchronize. CSS class selectors should also be swapped for getByTestId/semantic locators where test ids exist.
As per path instructions, "put locators and UI interactions in tests/utils/page/* page modules—don't inline raw page.locator(...)/page.getByTestId(...) in specs; every new section should have its own page-module file" and "Put expect(...) assertions in the spec (actions/helpers should only wait/synchronize)."
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 10-10: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${COLLECTION_NAME}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/mock-server/collection-mock-server.spec.ts` around lines 10 - 45, Move
mock-server locators and UI interactions from collection-mock-server.spec.ts
into a dedicated tests/utils/page mock-server module, including the flows
currently implemented by openMockServerTab, syncResponsesFromExamples,
startMockServer, and stopMockServer, plus repeated table, log, and filter
interactions in the tests. Keep expect assertions in the spec; page helpers
should only perform actions and synchronization, returning values such as the
detected port when needed. Replace CSS selectors with getByTestId or semantic
locators wherever available, and update the spec to use the new page-module
methods.
Source: Path instructions
1 participant
Description
Mock Server Implementation
Contribution Checklist:
Summary by CodeRabbit