feat(request): warn when binary body file is missing on disk#8753
Open
sanish-bruno wants to merge 5 commits into
Open
feat(request): warn when binary body file is missing on disk#8753sanish-bruno wants to merge 5 commits into
sanish-bruno wants to merge 5 commits into
Conversation
…istence validation and improve warning display. Introduced new hook for checking missing files and updated tests to reflect changes in file picker behavior.
Contributor
WalkthroughThe file picker now checks selected paths against the collection directory, displays warning styling and tooltip content for missing files, and adds an end-to-end test with a binary request fixture. ChangesMissing file warning
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RequestUI
participant FilePickerEditor
participant useMissingFileCheck
participant FilesystemHelpers
RequestUI->>FilePickerEditor: load selected binary path
FilePickerEditor->>useMissingFileCheck: check path against collection directory
useMissingFileCheck->>FilesystemHelpers: test resolved file existence
FilesystemHelpers-->>useMissingFileCheck: report missing path
useMissingFileCheck-->>FilePickerEditor: return missingPaths
FilePickerEditor-->>RequestUI: render warning icon and tooltip
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 |
sanish-bruno
changed the title
sanish-bruno
marked this pull request as ready for review
July 23, 2026 13:42
sanish-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
July 23, 2026 13:42
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/FilePickerEditor/index.js`:
- Around line 106-111: Update the warning icon rendering in FilePickerEditor so
the missing-file warning is discoverable without hover: wrap the icon in a
focusable semantic trigger or add equivalent accessible text/ARIA, while
preserving the existing warningTooltipId tooltip behavior. Apply the same
accessibility treatment to the related warning rendering at the additional
location.
In `@packages/bruno-app/src/hooks/useMissingFileCheck/index.js`:
- Around line 31-39: Update the early idle-return branch in the useEffect of
useMissingFileCheck to increment seqRef.current before returning when paths are
empty or basePath is falsy. Keep the existing idle state reset unchanged so any
in-flight check is invalidated and cannot overwrite it with stale results.
In `@tests/request/binary-file/binary-file-missing-warning.spec.ts`:
- Around line 42-54: The binary-file warning spec should stop using inline
page.locator selectors. Add selected, fileName, and warningIcon helpers under
locators.filePicker in the page locator module, then update the spec to use
those helpers while preserving the existing visibility, text, class, hover, and
tooltip assertions.
🪄 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: 228e0025-7d16-4b77-9657-eb6da29e529e
📒 Files selected for processing (6)
packages/bruno-app/src/components/FilePickerEditor/StyledWrapper.jspackages/bruno-app/src/components/FilePickerEditor/index.jspackages/bruno-app/src/hooks/useMissingFileCheck/index.jstests/request/binary-file/binary-file-missing-warning.spec.tstests/request/binary-file/fixtures/binary-file-missing-warning.ymltests/utils/page/locators.ts
Comment on lines
+106
to
+111
| {hasMissingFiles ? ( | ||
| <IconAlertTriangleFilled | ||
| data-tooltip-id={warningTooltipId} | ||
| className="warning-icon" | ||
| size={16} | ||
| /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the warning discoverable without hover.
The warning is attached to a plain, non-focusable SVG with a mouse-triggered tooltip, so keyboard and screen-reader users cannot reliably discover why the file is highlighted. Use a focusable semantic trigger or provide equivalent accessible text/ARIA.
Also applies to: 128-138
🤖 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/FilePickerEditor/index.js` around lines 106
- 111, Update the warning icon rendering in FilePickerEditor so the missing-file
warning is discoverable without hover: wrap the icon in a focusable semantic
trigger or add equivalent accessible text/ARIA, while preserving the existing
warningTooltipId tooltip behavior. Apply the same accessibility treatment to the
related warning rendering at the additional location.
Comment on lines
+31
to
+39
| useEffect(() => { | ||
| const currentPaths = pathsRef.current; | ||
| if (!currentPaths.length || !basePath) { | ||
| setState({ status: 'idle', missingPaths: [] }); | ||
| return; | ||
| } | ||
|
|
||
| const seq = ++seqRef.current; | ||
| setState({ status: 'checking', missingPaths: [] }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalidate in-flight checks when entering idle.
When paths becomes empty or basePath becomes falsy, the effect sets idle without advancing seqRef. A prior Promise.all can then resolve with the same sequence and overwrite the idle state with stale ready/missingPaths, allowing an obsolete warning to reappear. Increment the sequence before the early return.
Proposed fix
useEffect(() => {
+ const seq = ++seqRef.current;
const currentPaths = pathsRef.current;
if (!currentPaths.length || !basePath) {
setState({ status: 'idle', missingPaths: [] });
return;
}
- const seq = ++seqRef.current;
setState({ status: 'checking', missingPaths: [] });📝 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
| useEffect(() => { | |
| const currentPaths = pathsRef.current; | |
| if (!currentPaths.length || !basePath) { | |
| setState({ status: 'idle', missingPaths: [] }); | |
| return; | |
| } | |
| const seq = ++seqRef.current; | |
| setState({ status: 'checking', missingPaths: [] }); | |
| useEffect(() => { | |
| const seq = ++seqRef.current; | |
| const currentPaths = pathsRef.current; | |
| if (!currentPaths.length || !basePath) { | |
| setState({ status: 'idle', missingPaths: [] }); | |
| return; | |
| } | |
| setState({ status: 'checking', missingPaths: [] }); |
🤖 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/hooks/useMissingFileCheck/index.js` around lines 31 -
39, Update the early idle-return branch in the useEffect of useMissingFileCheck
to increment seqRef.current before returning when paths are empty or basePath is
falsy. Keep the existing idle state reset unchanged so any in-flight check is
invalidated and cannot overwrite it with stale results.
Comment on lines
+42
to
+54
| const filePicker = page.locator('.file-picker-selected').first(); | ||
| await expect(filePicker).toBeVisible({ timeout: 5000 }); | ||
| await expect(filePicker.locator('.file-name')).toHaveText('missing-payload.bin'); | ||
|
|
||
| // Warning: the missing-file styling, icon and tooltip must be present. | ||
| await expect(filePicker).toHaveClass(/has-warning/); | ||
| const warningIcon = filePicker.locator('.warning-icon'); | ||
| await expect(warningIcon).toBeVisible(); | ||
|
|
||
| await warningIcon.hover(); | ||
| const tooltip = locators.filePicker.warningTooltip(); | ||
| await expect(tooltip).toBeVisible(); | ||
| await expect(tooltip).toContainText(MISSING_FILE_WARNING); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move these selectors into the page locator module.
This spec directly uses page.locator(...) for .file-picker-selected, .file-name, and .warning-icon. Add selected, fileName, and warningIcon helpers under locators.filePicker and consume them here.
As per path instructions, specs must not inline raw selectors; selectors belong in tests/utils/page/* page modules.
🤖 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/request/binary-file/binary-file-missing-warning.spec.ts` around lines
42 - 54, The binary-file warning spec should stop using inline page.locator
selectors. Add selected, fileName, and warningIcon helpers under
locators.filePicker in the page locator module, then update the spec to use
those helpers while preserving the existing visibility, text, class, hover, and
tooltip assertions.
Source: Path instructions
…ilenames and improve warning tooltip layout with icon integration. Adjust StyledWrapper to utilize theme-based border radius for consistency.
6 tasks
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.
JIRA: BRU-3118
Extension of #8276
Description
Warn when a request's binary body points to a file that no longer exists on disk.
FilePickerEditorshows a warning icon and tooltip when the selected file is missing.useMissingFileCheck(paths, basePath)hook returns{ status, missingPaths }and uses a sequence counter to discard superseded results.getBasenamefor correct handling of trailing separators,./.., and mixed separators on Windows.IconAlertTriangleFilledcomponent.tests/request/binary-file/binary-file-missing-warning.spec.ts.Contribution Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Tests