Merged
Conversation
…le requests in IPC module
…rocess * add action to store transient directory paths in Redux state * update IPC handler to create and return a temporary directory for collections * modify collection mount action to dispatch transient directory addition
…uests * implement CreateTransientRequest component to facilitate the creation of HTTP, GraphQL, gRPC, and WebSocket transient requests * integrate the component into RequestTabs for user interaction * update collection and request handling to differentiate between transient and non-transient requests * enhance Redux actions to support transient request creation and management
* refactor Redux actions for HTTP, gRPC, and WebSocket requests to utilize a unified task queue for transient requests * implement a new helper function to retrieve collection paths from temporary directory metadata * add functionality to watch transient directories for file changes, excluding metadata.json * integrate transient directory watcher into the collection mounting process
…ation * update generateTransientRequestName function to accept tempDirectory parameter for improved request naming * modify CreateTransientRequest component to utilize tempDirectory for transient request creation * adjust middleware to check for transient status based on tempDirectory * implement transient file and directory identification in collections slice for better state management
* implement SaveTransientRequest component to facilitate saving transient requests to selected folders * create StyledWrapper for component styling * introduce useCollectionFolderTree hook for managing folder navigation and state * update Redux actions to handle saving requests from transient state
…ement * add SaveTransientRequestContainer to manage multiple transient request modals * refactor SaveTransientRequest component to utilize Redux for modal state management * implement open and close actions for transient request modals in Redux slice * update Bruno page to include SaveTransientRequestContainer for improved UI integration
… functionality * add input for creating new folders within the SaveTransientRequest component * implement validation for new folder names and filesystem names * integrate folder creation logic with Redux actions for better state management * update styling for new folder input elements in StyledWrapper * improve modal behavior to reset state when opened
Contributor
WalkthroughAdds transient-request support: UI to create transient requests, save modal(s) and container, Redux state/actions/middleware for transient directories and modal queuing, a folder-tree hook, Electron IPC/watcher to manage temp dirs and persist moves, and Playwright tests and helpers for transient flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as CreateTransientRequest / SaveModal
participant Redux
participant Electron
participant FS as FileSystem
User->>UI: create transient request (click)
UI->>Redux: dispatch new*Request(..., isTransient: true)
Redux->>Electron: mount-collection (returns tempDirectoryPath)
Electron->>FS: write file under /tmp/bruno-<id>/ and metadata.json
Electron->>Redux: emit collectionAddFileEvent (mapped to real collection path)
Redux->>UI: open tab (preview set based on isTransient)
User->>UI: open Save modal and confirm
UI->>Electron: renderer:save-transient-request(sourcePath, targetDir, filename, request, format)
Electron->>FS: validate/write target file, remove transient source
Electron->>Redux: notify saved, remove save-modal entry
Redux->>UI: show success toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
… request URLs * Refactored CreateTransientRequest component to retrieve request URLs from collection presets. * Enhanced request handling by dynamically setting request URLs based on the selected collection's configuration.
…ections actions * Removed unused imports from actions.js to streamline the code. * Updated the saveRequest function to reject the modal instead of resolving it when handling transient requests. * Cleaned up comments in index.js for better clarity.
…tionality * Removed success toast notifications from CreateTransientRequest component to simplify user feedback. * Enhanced SaveTransientRequest component to handle transient requests more effectively, including improved filename resolution and validation. * Added IPC handler for saving transient requests, ensuring proper file management and error handling. * Updated Redux actions to check for duplicate transient request names within the temporary directory.
* Added logic to differentiate between transient and non-transient drafts, ensuring transient requests are saved individually before closing the collection. * Improved user feedback by displaying unsaved changes for both regular and transient requests. * Updated save and discard functionality to handle all drafts appropriately, enhancing overall user experience.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/bruno-electron/src/ipc/collection.js (1)
1674-1701: Add cleanup for temp directory and watcher on collection removal.The temp directory created in
renderer:mount-collectionis never cleaned up. Whenrenderer:remove-collectionis called, only the collection watcher is removed—the temp directory watcher (stored undertempDirectoryPathkey) and the temp directory itself remain. No cleanup occurs on app exit either.Add a
removeTempDirectoryWatchermethod to the watcher module and call it when the collection is removed, then delete the temp directory.packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js (1)
2574-2639: PropagateisTransientto request items from transient file events.Line 2607 and Line 2623:
isTransientFileis computed but only applied to folders. Request items added/updated from the transient directory won’t carryisTransient, which can break filtering/UI logic that expects that flag.💡 Suggested patch
- currentItem.pathname = file.meta.pathname; + currentItem.pathname = file.meta.pathname; + currentItem.isTransient = isTransientFile; ... - currentSubItems.push({ + currentSubItems.push({ uid: file.data.uid, name: file.data.name, type: file.data.type, + isTransient: isTransientFile, seq: file.data.seq, tags: file.data.tags, request: file.data.request,packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
1257-1334: Guard against missing transient directory before usingpath.join.Line 1257 (and similar in gRPC/WS paths): if
isTransientis true buttempDirectoryisn’t in state yet,path.join(tempDirectory, ...)can throw and break request creation. Add an early guard and fail gracefully.💡 Suggested guard (apply to HTTP/GRPC/WS)
const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; + if (isTransient && !tempDirectory) { + return reject(new Error('Transient directory is not available yet. Please retry.')); + }Also applies to: 1426-1524, 1549-1649
138-158: Defer transient saves without rejecting the promise.Lines 158-160: The transient save flow dispatches a modal but then rejects the promise with no error. Since most callers use bare
dispatch(saveRequest(...))without.catch(), this could surface as unhandled promise rejections. The transient flow is intentional (user must confirm via modal), not a failure.Consider resolving with a sentinel value instead:
Suggested fix
dispatch(openSaveTransientRequestModal({ modalId, item, collection, isOpen: true })); - return reject(); + return resolve({ deferred: true });This signals to callers that the save is deferred to the modal, avoiding unhandled rejections while preserving the intended behavior.
🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/components/CreateTransientRequest/index.js`:
- Around line 88-175: The handlers handleCreateHttpRequest,
handleCreateGraphQLRequest, handleCreateWebSocketRequest and
handleCreateGrpcRequest capture tempDirectory (via generateTransientRequestName)
and collectionPresets.requestUrl but only list [dispatch, collection]; update
each useCallback dependency array to include tempDirectory and
collectionPresets.requestUrl so the closures refresh when those values change
(i.e., change the dependency arrays for handleCreateHttpRequest,
handleCreateGraphQLRequest, handleCreateWebSocketRequest,
handleCreateGrpcRequest to [dispatch, collection, tempDirectory,
collectionPresets.requestUrl]).
In `@packages/bruno-app/src/components/SaveTransientRequest/index.js`:
- Around line 91-114: In handleConfirm, guard against requestName collapsing to
an empty sanitized filename: after computing sanitizedFilename =
sanitizeName(requestName.trim()) (and before calling resolveRequestFilename or
any IPC save), check that sanitizedFilename is non-empty and handle the error
path (e.g., show validation/notification and return) to avoid constructing
targetFilename or attempting the IPC save; update the logic around sanitizeName,
resolveRequestFilename, and any use of targetFilename so they only run when
sanitizedFilename has content.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/RemoveCollection/ConfirmCollectionCloseDrafts.js`:
- Around line 25-43: Both currentDrafts and currentTransientDrafts call
item.pathname.startsWith(tempDirectory) which will throw if tempDirectory is
falsy; update the predicate in both React.useMemo blocks (the filter callbacks)
to guard tempDirectory (e.g. tempDirectory &&
item.pathname.startsWith(tempDirectory)) and for transient detection fall back
to item.isTransient when tempDirectory is not set. Keep the rest of the mapping
(including adding collectionUid) the same and reference symbols: currentDrafts,
currentTransientDrafts, flattenItems, filter, isItemARequest, hasRequestChanges,
item.pathname, item.isTransient, collectionUid.
In `@packages/bruno-app/src/providers/ReduxStore/middlewares/tasks/middleware.js`:
- Around line 25-39: The transient detection currently uses "tempDirectory &&
task.itemPathname.startsWith(tempDirectory)" which treats items as non‑transient
when tempDirectory is not set; change this so missing tempDirectory defaults to
treating items as transient (i.e., set isTransient = tempDirectory ?
task.itemPathname.startsWith(tempDirectory) : true) before calling
listenerApi.dispatch(addTab(...)) so preview remains false for uncertain cases;
update the isTransient variable in the block that uses findCollectionByUid,
findItemInCollectionByPathname, getDefaultRequestPaneTab and addTab.
In `@packages/bruno-electron/src/app/collection-watcher.js`:
- Around line 871-931: The temp watcher is stored under
this.watchers[tempDirectoryPath] in addTempDirectoryWatcher but removeWatcher
only closes this.watchers[collectionPath], causing temp watchers to leak; fix by
ensuring removeWatcher also closes the tempDirectory watcher: either (A) when
adding a temp watcher, also store a mapping from collectionUid/collectionPath to
its tempDirectoryPath (or push tempDirectoryPath into
this.watchers[collectionPath] metadata) and update removeWatcher to look up and
close this.watchers[tempDirectoryPath] and delete that key, or (B) change
removeWatcher signature to accept tempDirectoryPath (or accept collectionUid and
resolve tempDirectoryPath using getCollectionPathFromTempDirectory / reverse
lookup) and explicitly call this.watchers[tempDirectoryPath].close() and delete
the entry; update addTempDirectoryWatcher to ensure the tempDirectoryPath key is
created and documented so removeWatcher can find and cleanup it.
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 388-429: The handler for 'renderer:save-transient-request'
currently constructs targetPathname and writes to it without verifying
collection boundaries; call validatePathIsInsideCollection(targetPathname) (or
validatePathIsInsideCollection(targetDirname, targetFilename) if that overload
exists) right after computing targetPathname and before checking for conflicts
or performing any writes, and throw or reject when validation fails so the
subsequent steps in this function (the conflict check,
writeFile(targetPathname), and removePath(sourcePathname)) only run for
validated in-collection paths.
- Around line 86-107: The transient-detection logic in
findCollectionPathByItemPath treats any path under os.tmpdir() as transient and
returns null, breaking real collections stored in temp; change the detection so
a path is only treated as transient if it is under os.tmpdir() AND contains a
'bruno-' segment. Concretely, in findCollectionPathByItemPath locate the path
parts and the index for parts.findIndex(p => p.startsWith('bruno-')), then only
run the transient-handling block if filePath.startsWith(tmpDir) && index !== -1;
if there is no 'bruno-' segment, do not return null and allow normal collection
resolution to continue; keep the existing metadata read/JSON parse and its
catch-return-null behavior inside the bruno-specific branch.
In `@packages/bruno-electron/src/utils/collection.js`:
- Around line 406-410: The current transient detection marks any path under
os.tmpdir() as transient; change the check so request.isTransient is set true
only when the pathname is inside the temp dir AND uses the Bruno transient
prefix (e.g., directory or filename starting with "bruno-"); update the block
that references pathname and os.tmpdir() to perform a more specific test (for
example verify path.relative(os.tmpdir(), pathname) does not escape and that the
first path segment or basename startsWith "bruno-") and set request.isTransient
accordingly.
...c/components/Sidebar/Collections/Collection/RemoveCollection/ConfirmCollectionCloseDrafts.js
Outdated
Show resolved
Hide resolved
Comment on lines
+25
to
+39
| const tempDirectory = state.collections.tempDirectories?.[collectionUid]; | ||
| const openRequestTasks = filter(state.app.taskQueue, { type: taskTypes.OPEN_REQUEST }); | ||
| each(openRequestTasks, (task) => { | ||
| if (collectionUid === task.collectionUid) { | ||
| const collection = findCollectionByUid(state.collections.collections, collectionUid); | ||
| if (collection && collection.mountStatus === 'mounted' && !collection.isLoading) { | ||
| const item = findItemInCollectionByPathname(collection, task.itemPathname); | ||
| const isTransient = tempDirectory && task.itemPathname.startsWith(tempDirectory); | ||
| if (item) { | ||
| listenerApi.dispatch( | ||
| addTab({ | ||
| uid: item.uid, | ||
| collectionUid: collection.uid, | ||
| requestPaneTab: getDefaultRequestPaneTab(item) | ||
| requestPaneTab: getDefaultRequestPaneTab(item), | ||
| preview: !isTransient |
Contributor
There was a problem hiding this comment.
Guard transient detection when tempDirectory isn’t available.
If the temp path isn’t populated yet, transient items can be treated as non‑transient and opened with preview. Consider a fallback flag to avoid misclassification.
🔧 Suggested fix
- const isTransient = tempDirectory && task.itemPathname.startsWith(tempDirectory);
+ const isTransient = tempDirectory
+ ? task.itemPathname.startsWith(tempDirectory)
+ : item?.isTransient;📝 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.
Suggested change
| const tempDirectory = state.collections.tempDirectories?.[collectionUid]; | |
| const openRequestTasks = filter(state.app.taskQueue, { type: taskTypes.OPEN_REQUEST }); | |
| each(openRequestTasks, (task) => { | |
| if (collectionUid === task.collectionUid) { | |
| const collection = findCollectionByUid(state.collections.collections, collectionUid); | |
| if (collection && collection.mountStatus === 'mounted' && !collection.isLoading) { | |
| const item = findItemInCollectionByPathname(collection, task.itemPathname); | |
| const isTransient = tempDirectory && task.itemPathname.startsWith(tempDirectory); | |
| if (item) { | |
| listenerApi.dispatch( | |
| addTab({ | |
| uid: item.uid, | |
| collectionUid: collection.uid, | |
| requestPaneTab: getDefaultRequestPaneTab(item) | |
| requestPaneTab: getDefaultRequestPaneTab(item), | |
| preview: !isTransient | |
| const tempDirectory = state.collections.tempDirectories?.[collectionUid]; | |
| const openRequestTasks = filter(state.app.taskQueue, { type: taskTypes.OPEN_REQUEST }); | |
| each(openRequestTasks, (task) => { | |
| if (collectionUid === task.collectionUid) { | |
| const collection = findCollectionByUid(state.collections.collections, collectionUid); | |
| if (collection && collection.mountStatus === 'mounted' && !collection.isLoading) { | |
| const item = findItemInCollectionByPathname(collection, task.itemPathname); | |
| const isTransient = tempDirectory | |
| ? task.itemPathname.startsWith(tempDirectory) | |
| : item?.isTransient; | |
| if (item) { | |
| listenerApi.dispatch( | |
| addTab({ | |
| uid: item.uid, | |
| collectionUid: collection.uid, | |
| requestPaneTab: getDefaultRequestPaneTab(item), | |
| preview: !isTransient |
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/providers/ReduxStore/middlewares/tasks/middleware.js`
around lines 25 - 39, The transient detection currently uses "tempDirectory &&
task.itemPathname.startsWith(tempDirectory)" which treats items as non‑transient
when tempDirectory is not set; change this so missing tempDirectory defaults to
treating items as transient (i.e., set isTransient = tempDirectory ?
task.itemPathname.startsWith(tempDirectory) : true) before calling
listenerApi.dispatch(addTab(...)) so preview remains false for uncertain cases;
update the isTransient variable in the block that uses findCollectionByUid,
findItemInCollectionByPathname, getDefaultRequestPaneTab and addTab.
Comment on lines
+871
to
+931
| // Add watcher for transient directory | ||
| addTempDirectoryWatcher(win, tempDirectoryPath, collectionUid, collectionPath) { | ||
| if (this.watchers[tempDirectoryPath]) { | ||
| this.watchers[tempDirectoryPath].close(); | ||
| } | ||
|
|
||
| // Ignore metadata.json file | ||
| const ignored = (filepath) => { | ||
| const basename = path.basename(filepath); | ||
| return basename === 'metadata.json'; | ||
| }; | ||
|
|
||
| setTimeout(() => { | ||
| const watcher = chokidar.watch(tempDirectoryPath, { | ||
| ignoreInitial: true, // Don't process existing files | ||
| usePolling: isWSLPath(tempDirectoryPath) ? true : false, | ||
| ignored, | ||
| persistent: true, | ||
| ignorePermissionErrors: true, | ||
| awaitWriteFinish: { | ||
| stabilityThreshold: 80, | ||
| pollInterval: 10 | ||
| }, | ||
| depth: 1, // Only watch the temp directory itself, not subdirectories | ||
| disableGlobbing: true | ||
| }); | ||
|
|
||
| // Wrapper function to handle temp directory files | ||
| const addTempFile = async (pathname) => { | ||
| // Skip metadata.json | ||
| if (path.basename(pathname) === 'metadata.json') { | ||
| return; | ||
| } | ||
|
|
||
| // Get the actual collection path from metadata | ||
| const actualCollectionPath = this.getCollectionPathFromTempDirectory(tempDirectoryPath); | ||
| if (!actualCollectionPath) { | ||
| console.error(`Could not determine collection path for temp directory: ${tempDirectoryPath}`); | ||
| return; | ||
| } | ||
|
|
||
| // Use the collection format from the actual collection | ||
| const format = getCollectionFormat(actualCollectionPath); | ||
|
|
||
| // Only process request files | ||
| if (hasRequestExtension(pathname, format)) { | ||
| // Call the regular add function with the actual collection path | ||
| // This will hydrate and send the file to the renderer | ||
| await add(win, pathname, collectionUid, actualCollectionPath, false, this); | ||
| } | ||
| }; | ||
|
|
||
| watcher | ||
| .on('add', (pathname) => addTempFile(pathname)) | ||
| .on('error', (error) => { | ||
| console.error(`An error occurred in the temp directory watcher for: ${tempDirectoryPath}`, error); | ||
| }); | ||
|
|
||
| this.watchers[tempDirectoryPath] = watcher; | ||
| }, 100); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where temp directory watchers are removed / temp dirs cleaned up
rg -n "addTempDirectoryWatcher|getCollectionPathFromTempDirectory|tempDirectoryPath" packages/bruno-electron/src -g'*.js'
rg -n "removeWatcher|removePath|mkdtempSync" packages/bruno-electron/src -g'*.js'Repository: usebruno/bruno
Length of output: 4128
🏁 Script executed:
#!/bin/bash
# Get removeWatcher implementation and context around where addTempDirectoryWatcher is called
sed -n '820,860p' packages/bruno-electron/src/app/collection-watcher.js
# Check what happens around the addTempDirectoryWatcher call
sed -n '1690,1710p' packages/bruno-electron/src/ipc/collection.js
# Check what happens in the remove collection flow
sed -n '910,920p' packages/bruno-electron/src/ipc/collection.js
# Look for cleanup of temp directories (rm, unlink, rmSync patterns)
rg -n "rm\(|rmSync|unlink|unlinktempDirectory" packages/bruno-electron/src/ipc/collection.js -A 2 -B 2Repository: usebruno/bruno
Length of output: 3165
🏁 Script executed:
#!/bin/bash
# Search for any code that might close temp watchers or clean up temp directories
rg -n "this\.watchers\[tempDirectoryPath\]" packages/bruno-electron/src -A 1 -B 1
rg -n "tempDirectoryPath.*close\|tempDirectoryPath.*unlink\|tempDirectoryPath.*rm" packages/bruno-electron/src -A 2 -B 2
rg -n "fs\.rm\(.*tempo\|fs\.rmSync\(.*tempo" packages/bruno-electron/src
rg -n "mainWindow.*closed\|app.*quit" packages/bruno-electron/src/app/collection-watcher.js -A 5 -B 2Repository: usebruno/bruno
Length of output: 711
Fix temp directory watcher cleanup—watchers leak on collection removal.
addTempDirectoryWatcher stores watchers under tempDirectoryPath key, but removeWatcher (called during collection removal) closes watchers under collectionPath key. These keys don't match, so temp watchers are never closed and temp directories created via fs.mkdtempSync() are never deleted. This causes a file descriptor and disk space leak.
Either:
- Pass
tempDirectoryPathtoremoveWatcherand close it there, or - Track and clean up temp watchers separately when collections are removed.
🤖 Prompt for AI Agents
In `@packages/bruno-electron/src/app/collection-watcher.js` around lines 871 -
931, The temp watcher is stored under this.watchers[tempDirectoryPath] in
addTempDirectoryWatcher but removeWatcher only closes
this.watchers[collectionPath], causing temp watchers to leak; fix by ensuring
removeWatcher also closes the tempDirectory watcher: either (A) when adding a
temp watcher, also store a mapping from collectionUid/collectionPath to its
tempDirectoryPath (or push tempDirectoryPath into this.watchers[collectionPath]
metadata) and update removeWatcher to look up and close
this.watchers[tempDirectoryPath] and delete that key, or (B) change
removeWatcher signature to accept tempDirectoryPath (or accept collectionUid and
resolve tempDirectoryPath using getCollectionPathFromTempDirectory / reverse
lookup) and explicitly call this.watchers[tempDirectoryPath].close() and delete
the entry; update addTempDirectoryWatcher to ensure the tempDirectoryPath key is
created and documented so removeWatcher can find and cleanup it.
…ogic * Updated CreateTransientRequest to utilize useMemo for improved performance and prevent unnecessary re-renders. * Refactored generateTransientRequestName to focus solely on transient requests, removing tempDirectory dependency. * Streamlined SaveTransientRequest by consolidating form reset logic and removing unused state variables. * Improved ConfirmCollectionCloseDrafts to differentiate between transient and non-transient drafts more effectively. * Cleaned up imports and optimized Redux actions for better maintainability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/components/CreateTransientRequest/index.js`:
- Around line 58-60: The memoized "collection" computed with useMemo uses
collectionUid inside the find callback but only depends on collections; update
the dependency array of the useMemo that defines collection to include
collectionUid as well as collections so the memo recalculates when collectionUid
changes (i.e., in the useMemo surrounding the find call that sets collection).
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`:
- Around line 144-166: saveRequest currently computes isTransient by calling
item.pathname.startsWith(tempDirectory) without first ensuring tempDirectory
exists, causing exceptions or misclassification when the temp directory isn't
ready; update the isTransient calculation in saveRequest to first check
tempDirectory is truthy before using startsWith and otherwise fall back to
item.isTransient (e.g., compute isTransient = tempDirectory ?
item.pathname.startsWith(tempDirectory) : !!item.isTransient), keeping
references to item, tempDirectory, saveRequest, findItemInCollection,
openSaveTransientRequestModal and uuid unchanged so the transient modal flow
still dispatches correctly when transient is detected.
- Around line 1000-1087: The pasteItem action uses an async function as the
Promise executor (in the returned new Promise(async (resolve, reject) => { ...
})), which is unsafe; change pasteItem to avoid an async Promise executor by
returning a normal Promise that runs an immediately-invoked async function
(IIFE) or by making the outer function async and awaiting the operations
directly: move the try/catch body into an async IIFE (or top-level async
function) and call it, then resolve/reject based on its result; update the same
pattern in importCollection (the other occurrence) and keep the internal logic
(findCollectionByUid, generateUniqueName, ipcRenderer.invoke,
itemSchema.validate, dispatch insertTaskIntoQueue, etc.) identical while
removing async from the Promise executor to ensure resolver callbacks are not
blocked.
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js`:
- Around line 3158-3163: The clear operation
collectionClearOauth2CredentialsByUrl currently filters
collectionOauth2Credentials by url and collectionUid only, while
collectionGetOauth2CredentialsByUrl still checks credentialsId; make these
behaviors consistent by either (A) updating
collectionClearOauth2CredentialsByUrl to also check creds.credentialsId ===
credentialsId when removing entries, or (B) changing
collectionGetOauth2CredentialsByUrl to ignore credentialsId (matching the
current clear behavior); if the asymmetry is intentional, add a clarifying
comment near collectionClearOauth2CredentialsByUrl and
collectionGetOauth2CredentialsByUrl explaining why one removes all creds for a
URL but the getter requires credentialsId.
Comment on lines
+58
to
+60
| const collection = useMemo(() => { | ||
| return collections?.find((c) => c.uid === collectionUid); | ||
| }, [collections]); |
Contributor
There was a problem hiding this comment.
Missing collectionUid in useMemo dependency array.
The memo uses collectionUid in the find callback but only lists collections as a dependency. This can lead to stale closures where collection doesn't update when collectionUid changes.
🔧 Suggested fix
const collection = useMemo(() => {
return collections?.find((c) => c.uid === collectionUid);
- }, [collections]);
+ }, [collections, collectionUid]);🤖 Prompt for AI Agents
In `@packages/bruno-app/src/components/CreateTransientRequest/index.js` around
lines 58 - 60, The memoized "collection" computed with useMemo uses
collectionUid inside the find callback but only depends on collections; update
the dependency array of the useMemo that defines collection to include
collectionUid as well as collections so the memo recalculates when collectionUid
changes (i.e., in the useMemo surrounding the find call that sets collection).
Comment on lines
+144
to
+166
| export const saveRequest | ||
| = (itemUid, collectionUid, silent = false) => | ||
| (dispatch, getState) => { | ||
| const state = getState(); | ||
| const collection = findCollectionByUid(state.collections.collections, collectionUid); | ||
| const tempDirectory = state.collections.tempDirectories?.[collectionUid]; | ||
| return new Promise((resolve, reject) => { | ||
| if (!collection) { | ||
| return reject(new Error('Collection not found')); | ||
| } | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| if (!collection) { | ||
| return reject(new Error('Collection not found')); | ||
| } | ||
| const collectionCopy = cloneDeep(collection); | ||
| const item = findItemInCollection(collectionCopy, itemUid); | ||
| if (!item) { | ||
| return reject(new Error('Not able to locate item')); | ||
| } | ||
|
|
||
| const collectionCopy = cloneDeep(collection); | ||
| const item = findItemInCollection(collectionCopy, itemUid); | ||
| if (!item) { | ||
| return reject(new Error('Not able to locate item')); | ||
| } | ||
| const isTransient = tempDirectory && item.pathname.startsWith(tempDirectory); | ||
| if (isTransient) { | ||
| const modalId = uuid(); | ||
| dispatch(openSaveTransientRequestModal({ modalId, item, collection, isOpen: true })); | ||
| return reject(); | ||
| } |
Contributor
There was a problem hiding this comment.
Guard transient flows when the temp directory isn’t ready.
If isTransient is true but tempDirectory is missing, path.join will throw and startsWith can misclassify items. Add a guard and use item.isTransient as a fallback.
🛡️ Suggested guard
- const isTransient = tempDirectory && item.pathname.startsWith(tempDirectory);
+ const isTransient = item.isTransient || (tempDirectory && item.pathname && item.pathname.startsWith(tempDirectory));- const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null;
+ const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null;
+ if (isTransient && !tempDirectory) {
+ return reject(new Error('Transient directory is not ready yet. Please retry.'));
+ }Also applies to: 1324-1752
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`
around lines 144 - 166, saveRequest currently computes isTransient by calling
item.pathname.startsWith(tempDirectory) without first ensuring tempDirectory
exists, causing exceptions or misclassification when the temp directory isn't
ready; update the isTransient calculation in saveRequest to first check
tempDirectory is truthy before using startsWith and otherwise fall back to
item.isTransient (e.g., compute isTransient = tempDirectory ?
item.pathname.startsWith(tempDirectory) : !!item.isTransient), keeping
references to item, tempDirectory, saveRequest, findItemInCollection,
openSaveTransientRequestModal and uuid unchanged so the transient modal flow
still dispatches correctly when transient is detected.
Comment on lines
+1000
to
+1087
| export const pasteItem | ||
| = (targetCollectionUid, targetItemUid = null) => | ||
| (dispatch, getState) => { | ||
| const state = getState(); | ||
|
|
||
| const clipboardResult = brunoClipboard.read(); | ||
| const clipboardResult = brunoClipboard.read(); | ||
|
|
||
| if (!clipboardResult.hasData) { | ||
| return Promise.reject(new Error('No item in clipboard')); | ||
| } | ||
| if (!clipboardResult.hasData) { | ||
| return Promise.reject(new Error('No item in clipboard')); | ||
| } | ||
|
|
||
| const targetCollection = findCollectionByUid(state.collections.collections, targetCollectionUid); | ||
| const targetCollection = findCollectionByUid(state.collections.collections, targetCollectionUid); | ||
|
|
||
| if (!targetCollection) { | ||
| return Promise.reject(new Error('Target collection not found')); | ||
| } | ||
| if (!targetCollection) { | ||
| return Promise.reject(new Error('Target collection not found')); | ||
| } | ||
|
|
||
| return new Promise(async (resolve, reject) => { | ||
| try { | ||
| for (const clipboardItem of clipboardResult.items) { | ||
| const copiedItem = cloneDeep(clipboardItem); | ||
|
|
||
| const targetCollectionCopy = cloneDeep(targetCollection); | ||
| let targetItem = null; | ||
| let targetParentPathname = targetCollection.pathname; | ||
|
|
||
| // If targetItemUid is provided, we're pasting into a folder | ||
| if (targetItemUid) { | ||
| targetItem = findItemInCollection(targetCollectionCopy, targetItemUid); | ||
| if (!targetItem) { | ||
| return reject(new Error('Target folder not found')); | ||
| } | ||
| if (!isItemAFolder(targetItem)) { | ||
| return reject(new Error('Target must be a folder or collection')); | ||
| } | ||
| targetParentPathname = targetItem.pathname; | ||
| } | ||
| return new Promise(async (resolve, reject) => { | ||
| try { | ||
| for (const clipboardItem of clipboardResult.items) { | ||
| const copiedItem = cloneDeep(clipboardItem); | ||
|
|
||
| const targetCollectionCopy = cloneDeep(targetCollection); | ||
| let targetItem = null; | ||
| let targetParentPathname = targetCollection.pathname; | ||
|
|
||
| // If targetItemUid is provided, we're pasting into a folder | ||
| if (targetItemUid) { | ||
| targetItem = findItemInCollection(targetCollectionCopy, targetItemUid); | ||
| if (!targetItem) { | ||
| return reject(new Error('Target folder not found')); | ||
| } | ||
| if (!isItemAFolder(targetItem)) { | ||
| return reject(new Error('Target must be a folder or collection')); | ||
| } | ||
| targetParentPathname = targetItem.pathname; | ||
| } | ||
|
|
||
| const existingItems = targetItem ? targetItem.items : targetCollection.items; | ||
| const existingItems = targetItem ? targetItem.items : targetCollection.items; | ||
|
|
||
| // Handle folder pasting | ||
| if (isItemAFolder(copiedItem)) { | ||
| // Generate unique name for folder | ||
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, true); | ||
| // Handle folder pasting | ||
| if (isItemAFolder(copiedItem)) { | ||
| // Generate unique name for folder | ||
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, true); | ||
|
|
||
| set(copiedItem, 'name', newName); | ||
| set(copiedItem, 'filename', newFilename); | ||
| set(copiedItem, 'root.meta.name', newName); | ||
| set(copiedItem, 'root.meta.seq', (existingItems?.length ?? 0) + 1); | ||
| set(copiedItem, 'name', newName); | ||
| set(copiedItem, 'filename', newFilename); | ||
| set(copiedItem, 'root.meta.name', newName); | ||
| set(copiedItem, 'root.meta.seq', (existingItems?.length ?? 0) + 1); | ||
|
|
||
| const fullPathname = path.join(targetParentPathname, newFilename); | ||
| const { ipcRenderer } = window; | ||
| const fullPathname = path.join(targetParentPathname, newFilename); | ||
| const { ipcRenderer } = window; | ||
|
|
||
| await ipcRenderer.invoke('renderer:clone-folder', copiedItem, fullPathname, targetCollection.pathname); | ||
| } else { | ||
| // Handle request pasting | ||
| // Generate unique name for request | ||
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, false); | ||
| await ipcRenderer.invoke('renderer:clone-folder', copiedItem, fullPathname, targetCollection.pathname); | ||
| } else { | ||
| // Handle request pasting | ||
| // Generate unique name for request | ||
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, false); | ||
|
|
||
| const filename = resolveRequestFilename(newFilename, targetCollection.format); | ||
| const itemToSave = refreshUidsInItem(transformRequestToSaveToFilesystem(copiedItem)); | ||
| set(itemToSave, 'name', trim(newName)); | ||
| set(itemToSave, 'filename', trim(filename)); | ||
| const filename = resolveRequestFilename(newFilename, targetCollection.format); | ||
| const itemToSave = refreshUidsInItem(transformRequestToSaveToFilesystem(copiedItem)); | ||
| set(itemToSave, 'name', trim(newName)); | ||
| set(itemToSave, 'filename', trim(filename)); | ||
|
|
||
| const fullPathname = path.join(targetParentPathname, filename); | ||
| const { ipcRenderer } = window; | ||
| const requestItems = filter(existingItems, (i) => i.type !== 'folder'); | ||
| itemToSave.seq = requestItems ? requestItems.length + 1 : 1; | ||
| const fullPathname = path.join(targetParentPathname, filename); | ||
| const { ipcRenderer } = window; | ||
| const requestItems = filter(existingItems, (i) => i.type !== 'folder'); | ||
| itemToSave.seq = requestItems ? requestItems.length + 1 : 1; | ||
|
|
||
| await itemSchema.validate(itemToSave); | ||
| await ipcRenderer.invoke('renderer:new-request', fullPathname, itemToSave, targetCollection.format); | ||
| await itemSchema.validate(itemToSave); | ||
| await ipcRenderer.invoke('renderer:new-request', fullPathname, itemToSave, targetCollection.format); | ||
|
|
||
| dispatch(insertTaskIntoQueue({ | ||
| uid: uuid(), | ||
| type: 'OPEN_REQUEST', | ||
| collectionUid: targetCollectionUid, | ||
| itemPathname: fullPathname | ||
| })); | ||
| } | ||
| } | ||
| dispatch( | ||
| insertTaskIntoQueue({ | ||
| uid: uuid(), | ||
| type: 'OPEN_REQUEST', | ||
| collectionUid: targetCollectionUid, | ||
| itemPathname: fullPathname | ||
| }) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| resolve(); | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); | ||
| }; | ||
| resolve(); | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file
fd -type f -name "actions.js" | grep -E "collections"Repository: usebruno/bruno
Length of output: 228
🏁 Script executed:
# Check the file length and get line counts around the mentioned ranges
wc -l packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jsRepository: usebruno/bruno
Length of output: 136
🏁 Script executed:
# Extract the code around lines 1000-1087 to verify the async executor pattern
sed -n '1000,1087p' packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js | head -100Repository: usebruno/bruno
Length of output: 3650
🏁 Script executed:
# Extract the code around lines 2727-2761 to check the second instance
sed -n '2727,2761p' packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jsRepository: usebruno/bruno
Length of output: 1175
Wrap async work in an IIFE instead of using async Promise executor.
Both pasteItem (lines 1000–1087) and importCollection (lines 2727–2761) use async Promise executors, which blocks the resolver callbacks. Refactor to use an IIFE:
♻️ Refactor pattern
-return new Promise(async (resolve, reject) => {
+return new Promise((resolve, reject) => {
+ (async () => {
try {
// async work
- } catch (error) {
- reject(error);
- }
- });
+ } catch (error) {
+ reject(error);
+ }
+ })();
+});📝 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.
Suggested change
| export const pasteItem | |
| = (targetCollectionUid, targetItemUid = null) => | |
| (dispatch, getState) => { | |
| const state = getState(); | |
| const clipboardResult = brunoClipboard.read(); | |
| const clipboardResult = brunoClipboard.read(); | |
| if (!clipboardResult.hasData) { | |
| return Promise.reject(new Error('No item in clipboard')); | |
| } | |
| if (!clipboardResult.hasData) { | |
| return Promise.reject(new Error('No item in clipboard')); | |
| } | |
| const targetCollection = findCollectionByUid(state.collections.collections, targetCollectionUid); | |
| const targetCollection = findCollectionByUid(state.collections.collections, targetCollectionUid); | |
| if (!targetCollection) { | |
| return Promise.reject(new Error('Target collection not found')); | |
| } | |
| if (!targetCollection) { | |
| return Promise.reject(new Error('Target collection not found')); | |
| } | |
| return new Promise(async (resolve, reject) => { | |
| try { | |
| for (const clipboardItem of clipboardResult.items) { | |
| const copiedItem = cloneDeep(clipboardItem); | |
| const targetCollectionCopy = cloneDeep(targetCollection); | |
| let targetItem = null; | |
| let targetParentPathname = targetCollection.pathname; | |
| // If targetItemUid is provided, we're pasting into a folder | |
| if (targetItemUid) { | |
| targetItem = findItemInCollection(targetCollectionCopy, targetItemUid); | |
| if (!targetItem) { | |
| return reject(new Error('Target folder not found')); | |
| } | |
| if (!isItemAFolder(targetItem)) { | |
| return reject(new Error('Target must be a folder or collection')); | |
| } | |
| targetParentPathname = targetItem.pathname; | |
| } | |
| return new Promise(async (resolve, reject) => { | |
| try { | |
| for (const clipboardItem of clipboardResult.items) { | |
| const copiedItem = cloneDeep(clipboardItem); | |
| const targetCollectionCopy = cloneDeep(targetCollection); | |
| let targetItem = null; | |
| let targetParentPathname = targetCollection.pathname; | |
| // If targetItemUid is provided, we're pasting into a folder | |
| if (targetItemUid) { | |
| targetItem = findItemInCollection(targetCollectionCopy, targetItemUid); | |
| if (!targetItem) { | |
| return reject(new Error('Target folder not found')); | |
| } | |
| if (!isItemAFolder(targetItem)) { | |
| return reject(new Error('Target must be a folder or collection')); | |
| } | |
| targetParentPathname = targetItem.pathname; | |
| } | |
| const existingItems = targetItem ? targetItem.items : targetCollection.items; | |
| const existingItems = targetItem ? targetItem.items : targetCollection.items; | |
| // Handle folder pasting | |
| if (isItemAFolder(copiedItem)) { | |
| // Generate unique name for folder | |
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, true); | |
| // Handle folder pasting | |
| if (isItemAFolder(copiedItem)) { | |
| // Generate unique name for folder | |
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, true); | |
| set(copiedItem, 'name', newName); | |
| set(copiedItem, 'filename', newFilename); | |
| set(copiedItem, 'root.meta.name', newName); | |
| set(copiedItem, 'root.meta.seq', (existingItems?.length ?? 0) + 1); | |
| set(copiedItem, 'name', newName); | |
| set(copiedItem, 'filename', newFilename); | |
| set(copiedItem, 'root.meta.name', newName); | |
| set(copiedItem, 'root.meta.seq', (existingItems?.length ?? 0) + 1); | |
| const fullPathname = path.join(targetParentPathname, newFilename); | |
| const { ipcRenderer } = window; | |
| const fullPathname = path.join(targetParentPathname, newFilename); | |
| const { ipcRenderer } = window; | |
| await ipcRenderer.invoke('renderer:clone-folder', copiedItem, fullPathname, targetCollection.pathname); | |
| } else { | |
| // Handle request pasting | |
| // Generate unique name for request | |
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, false); | |
| await ipcRenderer.invoke('renderer:clone-folder', copiedItem, fullPathname, targetCollection.pathname); | |
| } else { | |
| // Handle request pasting | |
| // Generate unique name for request | |
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, false); | |
| const filename = resolveRequestFilename(newFilename, targetCollection.format); | |
| const itemToSave = refreshUidsInItem(transformRequestToSaveToFilesystem(copiedItem)); | |
| set(itemToSave, 'name', trim(newName)); | |
| set(itemToSave, 'filename', trim(filename)); | |
| const filename = resolveRequestFilename(newFilename, targetCollection.format); | |
| const itemToSave = refreshUidsInItem(transformRequestToSaveToFilesystem(copiedItem)); | |
| set(itemToSave, 'name', trim(newName)); | |
| set(itemToSave, 'filename', trim(filename)); | |
| const fullPathname = path.join(targetParentPathname, filename); | |
| const { ipcRenderer } = window; | |
| const requestItems = filter(existingItems, (i) => i.type !== 'folder'); | |
| itemToSave.seq = requestItems ? requestItems.length + 1 : 1; | |
| const fullPathname = path.join(targetParentPathname, filename); | |
| const { ipcRenderer } = window; | |
| const requestItems = filter(existingItems, (i) => i.type !== 'folder'); | |
| itemToSave.seq = requestItems ? requestItems.length + 1 : 1; | |
| await itemSchema.validate(itemToSave); | |
| await ipcRenderer.invoke('renderer:new-request', fullPathname, itemToSave, targetCollection.format); | |
| await itemSchema.validate(itemToSave); | |
| await ipcRenderer.invoke('renderer:new-request', fullPathname, itemToSave, targetCollection.format); | |
| dispatch(insertTaskIntoQueue({ | |
| uid: uuid(), | |
| type: 'OPEN_REQUEST', | |
| collectionUid: targetCollectionUid, | |
| itemPathname: fullPathname | |
| })); | |
| } | |
| } | |
| dispatch( | |
| insertTaskIntoQueue({ | |
| uid: uuid(), | |
| type: 'OPEN_REQUEST', | |
| collectionUid: targetCollectionUid, | |
| itemPathname: fullPathname | |
| }) | |
| ); | |
| } | |
| } | |
| resolve(); | |
| } catch (error) { | |
| reject(error); | |
| } | |
| }); | |
| }; | |
| resolve(); | |
| } catch (error) { | |
| reject(error); | |
| } | |
| }); | |
| export const pasteItem | |
| = (targetCollectionUid, targetItemUid = null) => | |
| (dispatch, getState) => { | |
| const state = getState(); | |
| const clipboardResult = brunoClipboard.read(); | |
| if (!clipboardResult.hasData) { | |
| return Promise.reject(new Error('No item in clipboard')); | |
| } | |
| const targetCollection = findCollectionByUid(state.collections.collections, targetCollectionUid); | |
| if (!targetCollection) { | |
| return Promise.reject(new Error('Target collection not found')); | |
| } | |
| return new Promise((resolve, reject) => { | |
| (async () => { | |
| try { | |
| for (const clipboardItem of clipboardResult.items) { | |
| const copiedItem = cloneDeep(clipboardItem); | |
| const targetCollectionCopy = cloneDeep(targetCollection); | |
| let targetItem = null; | |
| let targetParentPathname = targetCollection.pathname; | |
| // If targetItemUid is provided, we're pasting into a folder | |
| if (targetItemUid) { | |
| targetItem = findItemInCollection(targetCollectionCopy, targetItemUid); | |
| if (!targetItem) { | |
| return reject(new Error('Target folder not found')); | |
| } | |
| if (!isItemAFolder(targetItem)) { | |
| return reject(new Error('Target must be a folder or collection')); | |
| } | |
| targetParentPathname = targetItem.pathname; | |
| } | |
| const existingItems = targetItem ? targetItem.items : targetCollection.items; | |
| // Handle folder pasting | |
| if (isItemAFolder(copiedItem)) { | |
| // Generate unique name for folder | |
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, true); | |
| set(copiedItem, 'name', newName); | |
| set(copiedItem, 'filename', newFilename); | |
| set(copiedItem, 'root.meta.name', newName); | |
| set(copiedItem, 'root.meta.seq', (existingItems?.length ?? 0) + 1); | |
| const fullPathname = path.join(targetParentPathname, newFilename); | |
| const { ipcRenderer } = window; | |
| await ipcRenderer.invoke('renderer:clone-folder', copiedItem, fullPathname, targetCollection.pathname); | |
| } else { | |
| // Handle request pasting | |
| // Generate unique name for request | |
| const { newName, newFilename } = generateUniqueName(copiedItem.name, existingItems, false); | |
| const filename = resolveRequestFilename(newFilename, targetCollection.format); | |
| const itemToSave = refreshUidsInItem(transformRequestToSaveToFilesystem(copiedItem)); | |
| set(itemToSave, 'name', trim(newName)); | |
| set(itemToSave, 'filename', trim(filename)); | |
| const fullPathname = path.join(targetParentPathname, filename); | |
| const { ipcRenderer } = window; | |
| const requestItems = filter(existingItems, (i) => i.type !== 'folder'); | |
| itemToSave.seq = requestItems ? requestItems.length + 1 : 1; | |
| await itemSchema.validate(itemToSave); | |
| await ipcRenderer.invoke('renderer:new-request', fullPathname, itemToSave, targetCollection.format); | |
| dispatch( | |
| insertTaskIntoQueue({ | |
| uid: uuid(), | |
| type: 'OPEN_REQUEST', | |
| collectionUid: targetCollectionUid, | |
| itemPathname: fullPathname | |
| }) | |
| ); | |
| } | |
| } | |
| resolve(); | |
| } catch (error) { | |
| reject(error); | |
| } | |
| })(); | |
| }); | |
| }; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1017-1087: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`
around lines 1000 - 1087, The pasteItem action uses an async function as the
Promise executor (in the returned new Promise(async (resolve, reject) => { ...
})), which is unsafe; change pasteItem to avoid an async Promise executor by
returning a normal Promise that runs an immediately-invoked async function
(IIFE) or by making the outer function async and awaiting the operations
directly: move the try/catch body into an async IIFE (or top-level async
function) and call it, then resolve/reject based on its result; update the same
pattern in importCollection (the other occurrence) and keep the internal logic
(findCollectionByUid, generateUniqueName, ipcRenderer.invoke,
itemSchema.validate, dispatch insertTaskIntoQueue, etc.) identical while
removing async from the Promise executor to ensure resolver callbacks are not
blocked.
Comment on lines
3158
to
3163
| const filteredOauth2Credentials = filter( | ||
| collectionOauth2Credentials, | ||
| (creds) => | ||
| !(creds.url === url && creds.collectionUid === collectionUid) | ||
| (creds) => !(creds.url === url && creds.collectionUid === collectionUid) | ||
| ); | ||
| collection.oauth2Credentials = filteredOauth2Credentials; | ||
| } |
Contributor
There was a problem hiding this comment.
Inconsistent filter condition between clear and get operations.
collectionClearOauth2CredentialsByUrl filters without credentialsId, but collectionGetOauth2CredentialsByUrl at Line 3171 still uses credentialsId in its lookup. This asymmetry means clearing credentials will remove all credentials for a URL regardless of credentialsId, while getting still requires it.
If intentional, consider adding a comment explaining the design choice.
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js`
around lines 3158 - 3163, The clear operation
collectionClearOauth2CredentialsByUrl currently filters
collectionOauth2Credentials by url and collectionUid only, while
collectionGetOauth2CredentialsByUrl still checks credentialsId; make these
behaviors consistent by either (A) updating
collectionClearOauth2CredentialsByUrl to also check creds.credentialsId ===
credentialsId when removing entries, or (B) changing
collectionGetOauth2CredentialsByUrl to ignore credentialsId (matching the
current clear behavior); if the asymmetry is intentional, add a clarifying
comment near collectionClearOauth2CredentialsByUrl and
collectionGetOauth2CredentialsByUrl explaining why one removes all creds for a
URL but the getter requires credentialsId.
* Refactored SaveTransientRequest and its container to improve modal handling for unsaved transient requests. * Introduced state management for opening specific modals and added functionality to discard all unsaved requests. * Updated Redux actions to manage transient request modals more effectively, ensuring no duplicates are added. * Enhanced user interface to display a list of unsaved requests with options to save or discard them.
* Added useEffect to reset openItemUid when the corresponding modal is no longer present. * Implemented functionality to close all tabs associated with transient requests and show a success message upon discarding them. * Removed unnecessary modal close handler and streamlined modal opening logic for better clarity and performance.
Contributor
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/providers/ReduxStore/slices/collections/actions.js (1)
1323-1414: Fail fast when the transient temp directory is missing.
path.join(tempDirectory, ...)will throw if the temp directory isn’t hydrated yet, which can happen before mount completes. Add a guard in all three creators.🛡️ Suggested guard (apply in newHttpRequest/newGrpcRequest/newWsRequest)
- const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; + const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; + if (isTransient && !tempDirectory) { + return reject(new Error('Transient directory is not ready yet. Please retry.')); + }Also applies to: 1500-1577, 1638-1715
🤖 Fix all issues with AI agents
In `@packages/bruno-electron/src/app/collection-watcher.js`:
- Around line 866-877: The watcher can crash on invalid/stale metadata because
getCollectionFormat (and subsequent add/unlink logic) may throw; wrap the format
resolution and any downstream operations that act on the temp directory (calls
to getCollectionFormat and the add/unlink handlers referenced in the watcher
flow) in a try/catch inside the async handler so exceptions are caught, logged
(including the tempDirectoryPath and error), and the handler returns/continues
without rethrowing; apply the same defensive try/catch pattern around the
related block that processes temp directories (the code range handling format
resolution and add/unlink between the getCollectionPathFromTempDirectory call
and the end of that handler).
🧹 Nitpick comments (1)
packages/bruno-app/src/components/SaveTransientRequest/index.js (1)
109-113: Preserve the user-facing name; only sanitize the filename.Right now the saved request name becomes the sanitized filename, which can strip spaces/symbols from what the user typed. Consider using the trimmed name for
itemToSave.nameand keepsanitizedFilenamejust for the file path.♻️ Suggested tweak
- const itemToSave = latestItem.draft ? { ...latestItem, ...latestItem.draft } : { ...latestItem }; - itemToSave.name = sanitizedFilename; + const itemToSave = latestItem.draft ? { ...latestItem, ...latestItem.draft } : { ...latestItem }; + itemToSave.name = trimmedName;
Comment on lines
+866
to
+877
| // Helper function to get collection path from temp directory metadata | ||
| getCollectionPathFromTempDirectory(tempDirectoryPath) { | ||
| const metadataPath = path.join(tempDirectoryPath, 'metadata.json'); | ||
| try { | ||
| const metadataContent = fs.readFileSync(metadataPath, 'utf8'); | ||
| const metadata = JSON.parse(metadataContent); | ||
| return metadata.collectionPath; | ||
| } catch (error) { | ||
| console.error(`Error reading metadata from temp directory ${tempDirectoryPath}:`, error); | ||
| return null; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Guard temp file processing against format/metadata failures.
getCollectionFormat(actualCollectionPath) can throw (missing/invalid config or stale metadata), which will reject the async handler and can surface as unhandled rejections in the watcher. Wrapping format resolution (and downstream add/unlink) in a try/catch keeps the watcher resilient.
🛠️ Suggested hardening
const addTempFile = async (pathname) => {
// Skip metadata.json
if (path.basename(pathname) === 'metadata.json') {
return;
}
// Get the actual collection path from metadata
const actualCollectionPath = this.getCollectionPathFromTempDirectory(tempDirectoryPath);
if (!actualCollectionPath) {
console.error(`Could not determine collection path for temp directory: ${tempDirectoryPath}`);
return;
}
- // Use the collection format from the actual collection
- const format = getCollectionFormat(actualCollectionPath);
+ // Use the collection format from the actual collection
+ let format;
+ try {
+ format = getCollectionFormat(actualCollectionPath);
+ } catch (error) {
+ console.error('Could not determine collection format for', actualCollectionPath, error);
+ return;
+ }
// Only process request files
if (hasRequestExtension(pathname, format)) {
// Call the regular add function with the actual collection path
// This will hydrate and send the file to the renderer
await add(win, pathname, collectionUid, actualCollectionPath, false, this);
}
};
const unlinkTempFile = async (pathname) => {
// Skip metadata.json
if (path.basename(pathname) === 'metadata.json') {
return;
}
// Get the actual collection path from metadata
const actualCollectionPath = this.getCollectionPathFromTempDirectory(tempDirectoryPath);
if (!actualCollectionPath) {
console.error(`Could not determine collection path for temp directory: ${tempDirectoryPath}`);
return;
}
- // Use the collection format from the actual collection
- const format = getCollectionFormat(actualCollectionPath);
+ // Use the collection format from the actual collection
+ let format;
+ try {
+ format = getCollectionFormat(actualCollectionPath);
+ } catch (error) {
+ console.error('Could not determine collection format for', actualCollectionPath, error);
+ return;
+ }
// Only process request files
if (hasRequestExtension(pathname, format)) {
// Call the regular unlink function with the actual collection path
await unlink(win, pathname, collectionUid, actualCollectionPath);
}
};Also applies to: 879-966
🤖 Prompt for AI Agents
In `@packages/bruno-electron/src/app/collection-watcher.js` around lines 866 -
877, The watcher can crash on invalid/stale metadata because getCollectionFormat
(and subsequent add/unlink logic) may throw; wrap the format resolution and any
downstream operations that act on the temp directory (calls to
getCollectionFormat and the add/unlink handlers referenced in the watcher flow)
in a try/catch inside the async handler so exceptions are caught, logged
(including the tempDirectoryPath and error), and the handler returns/continues
without rethrowing; apply the same defensive try/catch pattern around the
related block that processes temp directories (the code range handling format
resolution and add/unlink between the getCollectionPathFromTempDirectory call
and the end of that handler).
sid-bruno
reviewed
Jan 28, 2026
| const handleSaveAll = () => { | ||
| // If there are transient drafts, we can't proceed with batch save | ||
| if (currentTransientDrafts.length > 0) { | ||
| toast.error('Please save or discard transient requests first'); |
Collaborator
There was a problem hiding this comment.
Not sure about this functionality, I'm guessing this was discussed with product but I remember saying we save all into the root if they select save
sid-bruno
reviewed
Jan 28, 2026
Comment on lines
+143
to
+151
| {saveTransientRequestModals.length === 1 ? ( | ||
| <SaveTransientRequest | ||
| item={saveTransientRequestModals[0].item} | ||
| collection={saveTransientRequestModals[0].collection} | ||
| isOpen={true} | ||
| /> | ||
| ) : saveTransientRequestModals.length > 1 ? ( | ||
| <SaveTransientRequestContainer /> | ||
| ) : null} |
Collaborator
There was a problem hiding this comment.
Wrap into a sub component with a cleaner if else condition, this will become unreadable quickly
sid-bruno
reviewed
Jan 28, 2026
sid-bruno
reviewed
Jan 28, 2026
sid-bruno
reviewed
Jan 28, 2026
| return basename === 'metadata.json'; | ||
| }; | ||
|
|
||
| setTimeout(() => { |
Collaborator
There was a problem hiding this comment.
Don't see the point of the setTimeout but if the delay is irrelevant (considering 100 was passed), just use a setImmediate instead to push this into the timer thread
Collaborator
Author
There was a problem hiding this comment.
Removing the timeout entirely. The caller of addTempDirectoryWatcher creates the directory and the metadata file before calling addTempDirectoryWatcher. Adding timeout or immediate is of no use and creates an unnecessary async call. Removing it.
sid-bruno
reviewed
Jan 28, 2026
sid-bruno
reviewed
Jan 28, 2026
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
Show resolved
Hide resolved
sid-bruno
reviewed
Jan 28, 2026
Collaborator
There was a problem hiding this comment.
General review, there's a lot of whitespace changes that will need to be either considered during sync or reset and re-added into the diffs.
I've cc'd @bijin-bruno for references in files that conflict during sync because of this, please take @chirag-bruno into it when doing the sync to be safe.
There's some minor comments for additional calls and work being done, which should be fixed.
There's 0 tests in the PR, please add in basic request tests for all 3 types (http, grpc, ws) and make sure they work
You can add these tests directly in their respective tests files with the modification being the creation of these transient requests
…ction actions * Consolidated multiple lines of code into single lines for better readability in ConfirmCollectionCloseDrafts and actions.js. * Enhanced consistency in the formatting of function parameters and return statements across the collections slice. * Removed unnecessary line breaks and improved the structure of the code for easier maintenance.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/RemoveCollection/ConfirmCollectionCloseDrafts.js`:
- Around line 60-72: The current catch for
dispatch(saveMultipleRequests(currentDrafts)) duplicates an existing toast from
saveMultipleRequests; keep the catch to handle rejections but remove the
redundant toast.error call so it doesn't show the same message twice.
Specifically, inside the block that calls
dispatch(saveMultipleRequests(currentDrafts)) → .catch(...), delete the
toast.error('Failed to save requests!') but still ensure the promise is handled
(e.g., leave an empty catch or rethrow/log silently) and preserve the subsequent
flow that dispatches removeCollection(collectionUid), toast.success and onClose
in the success path.
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`:
- Around line 2643-2655: The catch handler in openMultipleCollections drops the
original error by calling reject() with no argument; update the catch to call
reject(err) so the original error is propagated to callers (or alternatively
return ipcRenderer.invoke(...) directly from openMultipleCollections to
naturally propagate the promise rejection); locate the function
openMultipleCollections in the Redux store collections actions and change the
.catch((err) => { reject(); }) to .catch((err) => { reject(err); }) (or remove
the manual Promise wrapper and return
ipcRenderer.invoke('renderer:open-multiple-collections', collectionPaths,
options)).
...c/components/Sidebar/Collections/Collection/RemoveCollection/ConfirmCollectionCloseDrafts.js
Show resolved
Hide resolved
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
Outdated
Show resolved
Hide resolved
…tions * Consolidated multiple lines of code into single lines for better readability in middleware.js and actions.js. * Enhanced consistency in formatting function parameters and return statements across the collections slice. * Removed unnecessary line breaks and improved the structure of the code for easier maintenance. * Streamlined dispatch calls for better clarity and performance.
Contributor
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/providers/ReduxStore/slices/collections/actions.js (1)
1246-1348: Guard transient creation when the temp directory isn’t ready.When
isTransientis true buttempDirectoryis missing,path.joinwill throw and duplicate checks become unreliable. Add an early guard before using the temp path (apply the same pattern innewHttpRequest,newGrpcRequest, andnewWsRequest).🛠️ Suggested fix (apply in each transient branch)
- const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; + const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; + if (isTransient && !tempDirectory) { + return reject(new Error('Transient directory is not ready yet. Please retry.')); + }Also applies to: 1424-1537, 1551-1664
🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/providers/ReduxStore/middlewares/tasks/middleware.js`:
- Around line 6-8: Remove the unused imports by editing the import statements:
delete closeAllCollectionTabs from the import from
'providers/ReduxStore/slices/tabs' and delete flattenItems from the import from
'utils/collections/index' so only actually used symbols (e.g., addTab,
closeTabs, collectionAddFileEvent, collectionChangeFileEvent,
findCollectionByUid, findItemInCollectionByPathname, getDefaultRequestPaneTab,
findItemInCollectionByItemUid, findItemInCollection) remain.
packages/bruno-app/src/providers/ReduxStore/middlewares/tasks/middleware.js
Show resolved
Hide resolved
…actions * Improved formatting and structure in middleware.js for dispatch calls. * Streamlined comments and indentation in actions.js for better clarity. * Consolidated multiple lines into single lines where appropriate to enhance readability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`:
- Around line 1552-1554: The code computes tempDirectory with const
tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid]
: null but doesn't guard for undefined before using it in the surrounding
action; update the same action (the function that handles creating requests in
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js — the
blocks that mirror newHttpRequest and newGrpcRequest) to early-return or handle
the case when tempDirectory is undefined (e.g., if isTransient && !tempDirectory
then abort/throw/log and skip further processing), referencing the same symbols:
tempDirectory, isTransient, state.collections.tempDirectories, and collectionUid
so the behavior matches newHttpRequest/newGrpcRequest; apply the same guard
changes to the other occurrence noted (~lines 1596-1628).
- Around line 1258-1260: The code assumes tempDirectory is a string when
isTransient is true but it can be undefined; update the logic around the
tempDirectory lookup (state.collections.tempDirectories?.[collectionUid]) and
all uses (the startsWith check and path.join calls) to guard for a non-empty
string before using string methods or path.join: check that tempDirectory is
truthy and of type string (e.g., tempDirectory && typeof tempDirectory ===
'string') before calling tempDirectory.startsWith(...) or
path.join(tempDirectory, filename), and if it's missing, take the safe
alternative (skip the transient-path branch or return/null fallback) so no
"undefined" is coerced into paths or comparisons.
- Around line 1424-1427: The code sets const tempDirectory = isTransient ?
state.collections.tempDirectories?.[collectionUid] : null but doesn't guard
against tempDirectory being undefined before it's used, which can produce
malformed paths; update the logic in the affected functions (e.g., where
tempDirectory is read for the current HTTP request handling — referenced as
newHttpRequest and the block around lines 1469-1501) to treat undefined the same
as null: check that tempDirectory is truthy before constructing paths or call
early-return/throw when isTransient && !tempDirectory, or explicitly set
tempDirectory = isTransient ?
(state.collections.tempDirectories?.[collectionUid] ?? null) : null so
downstream path-building only runs when tempDirectory is non-null. Ensure the
same guard is applied both at the newHttpRequest site and the separate block
covering 1469-1501.
Comment on lines
+1258
to
+1260
| // Get temp directory if isTransient is true | ||
| const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; | ||
|
|
Contributor
There was a problem hiding this comment.
Guard against undefined tempDirectory before using it.
If isTransient is true but the temp directory hasn't been created yet (collection not fully mounted), tempDirectory will be undefined. This causes:
- Line 1321:
startsWith(undefined)coerces to"undefined"string — incorrect match logic - Line 1328:
path.join(undefined, filename)produces"undefined/filename.bru"
🛠️ Suggested fix
// Get temp directory if isTransient is true
const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null;
+
+ if (isTransient && !tempDirectory) {
+ return reject(new Error('Transient directory is not ready. Please try again.'));
+ }
const parts = splitOnFirst(requestUrl, '?');Also applies to: 1315-1348
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`
around lines 1258 - 1260, The code assumes tempDirectory is a string when
isTransient is true but it can be undefined; update the logic around the
tempDirectory lookup (state.collections.tempDirectories?.[collectionUid]) and
all uses (the startsWith check and path.join calls) to guard for a non-empty
string before using string methods or path.join: check that tempDirectory is
truthy and of type string (e.g., tempDirectory && typeof tempDirectory ===
'string') before calling tempDirectory.startsWith(...) or
path.join(tempDirectory, filename), and if it's missing, take the safe
alternative (skip the transient-path branch or return/null fallback) so no
"undefined" is coerced into paths or comparisons.
Comment on lines
+1424
to
+1427
|
|
||
| // Get temp directory if isTransient is true | ||
| const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; | ||
|
|
Contributor
There was a problem hiding this comment.
Same tempDirectory guard needed here.
Identical to newHttpRequest: if the transient directory isn't ready, tempDirectory is undefined, leading to malformed paths.
🛠️ Suggested fix
// Get temp directory if isTransient is true
const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null;
+
+ if (isTransient && !tempDirectory) {
+ return reject(new Error('Transient directory is not ready. Please try again.'));
+ }
// do we need to handle query, path params for grpc requests?Also applies to: 1469-1501
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`
around lines 1424 - 1427, The code sets const tempDirectory = isTransient ?
state.collections.tempDirectories?.[collectionUid] : null but doesn't guard
against tempDirectory being undefined before it's used, which can produce
malformed paths; update the logic in the affected functions (e.g., where
tempDirectory is read for the current HTTP request handling — referenced as
newHttpRequest and the block around lines 1469-1501) to treat undefined the same
as null: check that tempDirectory is truthy before constructing paths or call
early-return/throw when isTransient && !tempDirectory, or explicitly set
tempDirectory = isTransient ?
(state.collections.tempDirectories?.[collectionUid] ?? null) : null so
downstream path-building only runs when tempDirectory is non-null. Ensure the
same guard is applied both at the newHttpRequest site and the separate block
covering 1469-1501.
Comment on lines
+1552
to
+1554
| // Get temp directory if isTransient is true | ||
| const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null; | ||
|
|
Contributor
There was a problem hiding this comment.
Same tempDirectory guard needed here.
Consistent with newHttpRequest and newGrpcRequest, add a guard for undefined tempDirectory.
🛠️ Suggested fix
// Get temp directory if isTransient is true
const tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid] : null;
+
+ if (isTransient && !tempDirectory) {
+ return reject(new Error('Transient directory is not ready. Please try again.'));
+ }
const item = {Also applies to: 1596-1628
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`
around lines 1552 - 1554, The code computes tempDirectory with const
tempDirectory = isTransient ? state.collections.tempDirectories?.[collectionUid]
: null but doesn't guard for undefined before using it in the surrounding
action; update the same action (the function that handles creating requests in
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js — the
blocks that mirror newHttpRequest and newGrpcRequest) to early-return or handle
the case when tempDirectory is undefined (e.g., if isTransient && !tempDirectory
then abort/throw/log and skip further processing), referencing the same symbols:
tempDirectory, isTransient, state.collections.tempDirectories, and collectionUid
so the behavior matches newHttpRequest/newGrpcRequest; apply the same guard
changes to the other occurrence noted (~lines 1596-1628).
* Improved the modal handling logic for removing collections to differentiate between regular and drafts confirmation modals. * Added new tests for creating and saving transient requests (HTTP, GraphQL, gRPC, WebSocket) ensuring they do not appear in the sidebar until saved. * Introduced utility functions for creating transient requests and filling request URLs, improving code reusability and clarity.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/transient-requests/transient-requests.spec.ts`:
- Around line 124-128: The test uses page.keyboard.press('Meta+s') which only
works on macOS; update the keyboard press to use the cross-platform modifier
'ControlOrMeta+s' in the test step that triggers the save action (the block
labeled "Trigger save action using keyboard shortcut" that calls
page.keyboard.press) and make the same change in the other test step where
page.keyboard.press('Meta+s') is used so both save shortcuts use
'ControlOrMeta+s'.
In `@tests/utils/page/actions.ts`:
- Around line 159-161: Update the inconsistent WebSocket casing: change the
union member 'Websocket' to 'WebSocket' in the CreateTransientRequestOptions
type so it matches CreateUntitledRequestOptions and UI/protocol naming, and also
update any tests that assert/create transient requests (the test referenced as
transient-requests.spec.ts uses 'Websocket') to use 'WebSocket' instead.
🧹 Nitpick comments (5)
tests/onboarding/sample-collection.spec.ts (1)
95-108: Consider extracting duplicated modal handling logic.This modal-handling pattern (checking for "Discard All and Remove" vs regular submit) is duplicated in
tests/utils/page/actions.ts(closeAllCollectionsandremoveCollection). Extract this to a shared helper function to improve maintainability and reduce duplication.💡 Example helper extraction
// In tests/utils/page/actions.ts const handleRemoveCollectionModal = async (page: Page) => { const removeModal = page.locator('.bruno-modal').filter({ hasText: 'Remove Collection' }); await removeModal.waitFor({ state: 'visible', timeout: 5000 }); const hasDiscardButton = await page.getByRole('button', { name: 'Discard All and Remove' }).isVisible().catch(() => false); if (hasDiscardButton) { await page.getByRole('button', { name: 'Discard All and Remove' }).click(); } else { await page.locator('.bruno-modal-footer .submit').click(); } await removeModal.waitFor({ state: 'hidden', timeout: 5000 }); };tests/transient-requests/transient-requests.spec.ts (3)
3-3: Remove unused import.
buildWebsocketCommonLocatorsis imported but never used in this file.🧹 Proposed fix
-import { buildCommonLocators, buildWebsocketCommonLocators } from '../utils/page/locators'; +import { buildCommonLocators } from '../utils/page/locators';
143-145: Replace fixed timeout with proper wait.Instead of
page.waitForTimeout(1000), wait for an observable state change such as the modal closing or a success toast appearing.⏱️ Proposed fix
await saveModal.getByRole('button', { name: 'Save' }).click(); - // Wait for success toast - await page.waitForTimeout(1000); + // Wait for modal to close + await saveModal.waitFor({ state: 'hidden', timeout: 5000 });Apply similar fix at line 187.
41-50: Replace timeout with assertion-based wait.The
waitForTimeout(300)can be eliminated sinceexpect(collectionItems).toHaveCount(0)already has built-in auto-waiting.⏱️ Proposed fix
await test.step('Verify request is NOT in sidebar', async () => { // Click on the collection to ensure it's expanded await locators.sidebar.collection('transient-requests-test').click(); - await page.waitForTimeout(300); // Check that there are no requests in the collection // Transient requests should not appear in the sidebar const collectionItems = page.locator('.collection-item-name'); await expect(collectionItems).toHaveCount(0); });tests/utils/page/actions.ts (1)
380-396: Duplicated modal handling logic with closeAllCollections.This is the same pattern as lines 20-37. Extract to a shared helper function (e.g.,
handleRemoveCollectionModal) to reduce duplication and simplify maintenance.
Comment on lines
+124
to
+128
| await test.step('Trigger save action using keyboard shortcut', async () => { | ||
| // Try to save using Cmd+S (Mac) or Ctrl+S (other platforms) | ||
| await page.keyboard.press('Meta+s'); | ||
| await page.waitForTimeout(500); | ||
| }); |
Contributor
There was a problem hiding this comment.
Use cross-platform keyboard shortcut.
Meta+s only works on macOS. For cross-platform compatibility, use the ControlOrMeta modifier.
🔧 Proposed fix
- await page.keyboard.press('Meta+s');
+ await page.keyboard.press('ControlOrMeta+s');Apply the same fix at line 174.
📝 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.
Suggested change
| await test.step('Trigger save action using keyboard shortcut', async () => { | |
| // Try to save using Cmd+S (Mac) or Ctrl+S (other platforms) | |
| await page.keyboard.press('Meta+s'); | |
| await page.waitForTimeout(500); | |
| }); | |
| await test.step('Trigger save action using keyboard shortcut', async () => { | |
| // Try to save using Cmd+S (Mac) or Ctrl+S (other platforms) | |
| await page.keyboard.press('ControlOrMeta+s'); | |
| await page.waitForTimeout(500); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/transient-requests/transient-requests.spec.ts` around lines 124 - 128,
The test uses page.keyboard.press('Meta+s') which only works on macOS; update
the keyboard press to use the cross-platform modifier 'ControlOrMeta+s' in the
test step that triggers the save action (the block labeled "Trigger save action
using keyboard shortcut" that calls page.keyboard.press) and make the same
change in the other test step where page.keyboard.press('Meta+s') is used so
both save shortcuts use 'ControlOrMeta+s'.
…ection watcher logic * Introduced a new TransientRequestModalsRenderer component to streamline modal rendering based on the number of transient requests. * Refactored the collection watcher logic to enhance readability by removing unnecessary setTimeout and consolidating file handling functions. * Improved error handling and logging for the temp directory watcher.
…and tests * Updated the spelling of 'Websocket' to 'WebSocket' in CreateTransientRequest component, transient requests test, and action type definitions for consistency and accuracy.
This was referenced Feb 5, 2026
This was referenced Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces Transient Requests, a new way to quickly create requests within a collection without requiring a name upfront. These requests are intended to be disposable and are not automatically saved to the collection. Users can choose to persist a transient request either when closing it or by explicitly saving it via the Save button or
Ctrl/Cmd + S.This change addresses the issue tracked in BRU-2114: https://usebruno.atlassian.net/browse/BRU-2114
Transient requests are stored in a temporary directory and are excluded from the collection directory. This improves the user experience by allowing users to create requests instantly using the
+icon in the tab bar. Right-clicking the+icon opens a dropdown to select the request type, while a left-click creates a request using the collection’s preset request type (defaulting to HTTP if no preset is configured).The PR also introduces an enhanced save flow that allows users to rename a request at the time of saving and choose a target directory, which defaults to the collection root. From the save modal, users can additionally create a new folder and immediately save the request within that newly created directory.
Contribution Checklist:
Summary by CodeRabbit
New Features
UX / Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.