feat: improve OpenAPI Sync tab UX and fix sync flow bugs#7467
Merged
bijin-bruno merged 11 commits intousebruno:mainfrom Mar 13, 2026
Merged
feat: improve OpenAPI Sync tab UX and fix sync flow bugs#7467bijin-bruno merged 11 commits intousebruno:mainfrom
bijin-bruno merged 11 commits intousebruno:mainfrom
Conversation
…alidation Updated error messages in ConnectSpecForm and ConnectionSettingsModal to clarify that only OpenAPI 3.x specifications are valid. Enhanced useOpenAPISync hook to reflect the same specificity in error handling for invalid URLs.
…c viewer Implemented a new function to pretty-print JSON content for improved readability in the OpenAPISpecTab component. This enhancement ensures that JSON specifications are displayed in a more user-friendly format while leaving YAML content unchanged.
…local sources Added functionality to resolve relative file paths to absolute paths for better user experience in the OpenAPISyncHeader component. Implemented state management and side effects to handle path resolution based on the source URL, enhancing the display of local file paths.
…int counting utility Refactored the CollectionStatusSection to streamline the display of collection drift status, integrating loading states and improved messaging for initial sync scenarios. Introduced a new utility function to count HTTP endpoints in OpenAPI specifications, enhancing the overall functionality of the OpenAPISyncTab. Additionally, updated the OpenAPISyncHeader and OverviewSection to utilize stored specification metadata for better user experience.
- Enhanced the logic for adding new requests by ensuring existing files are verified before removal to prevent accidental deletions. - Streamlined the process of adding new endpoints, including checks for existing files and merging requests to maintain user customizations. - Added comments for clarity on the purpose of changes, particularly regarding filename collision prevention and file content verification.
- Changed background color for the 'type-spec-modified' class to a warning color for better distinction. - Updated text color and background for the SyncReviewPage to enhance readability and visual hierarchy. - Adjusted default expanded states for endpoint sections to improve user experience during sync reviews.
abhishek-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
WalkthroughAdds spec pretty-printing, per-collection stored spec metadata and endpoint counting, safer IPC file operations with content verification, broader loading/empty-state handling across OpenAPI sync UI, and several styling and copy adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Renderer
participant Redux
participant IPCMain
participant FS as FileSystem
Client->>Renderer: Open Sync tab / trigger "Check for updates"
Renderer->>Redux: dispatch start check (isLoading = true)
Renderer->>IPCMain: request remote spec / resolve local path
IPCMain->>FS: read remote/local spec files
FS-->>IPCMain: return spec bytes / file contents
IPCMain-->>Renderer: send spec result, diff, metadata
Renderer->>Redux: dispatch setStoredSpecMeta / updateStoredSpec
Redux-->>Renderer: storedSpecMeta available
Renderer->>Client: render UI (specMeta, endpoint counts, or loader)
Note right of IPCMain: deletions guarded — parse file -> match method/path -> delete if match
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/bruno-app/src/components/OpenAPISyncTab/utils.js (1)
10-13: HardencountEndpointsagainst malformedpathsentries.
Object.keys(path)can throw if a path item is null/non-object. A small guard makes this utility safer for imperfect spec payloads.♻️ Proposed defensive update
export const countEndpoints = (spec) => { if (!spec?.paths) return null; let count = 0; - for (const path of Object.values(spec.paths)) { - for (const key of Object.keys(path)) { + for (const pathItem of Object.values(spec.paths)) { + if (!pathItem || typeof pathItem !== 'object') continue; + for (const key of Object.keys(pathItem)) { if (HTTP_METHODS.includes(key.toLowerCase())) count++; } } return count; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/OpenAPISyncTab/utils.js` around lines 10 - 13, The loop in countEndpoints can throw when iterating Object.keys(path) if a path entry is null or not an object; add a guard before using Object.keys to ensure each path is a plain object (e.g. check path !== null && typeof path === "object") and skip non-objects, then proceed to iterate keys and test against HTTP_METHODS; update countEndpoints to perform that check (referencing the countEndpoints function and HTTP_METHODS constant) so malformed spec.paths entries are safely ignored.packages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.js (1)
5-6: Unused import:IconLoader2.
IconLoader2is imported but not used in this file. The loading state appears to be handled bySyncReviewPagewhich receives theisLoadingprop.🧹 Proposed fix
import { IconCheck, - IconRefresh, - IconLoader2 + IconRefresh } from '@tabler/icons';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.js` around lines 5 - 6, The import list in SpecStatusSection includes an unused symbol IconLoader2; remove IconLoader2 from the import statement (the import that currently lists IconRefresh and IconLoader2) so only IconRefresh is imported, and ensure no other references to IconLoader2 remain in the file (the loading UI is already handled via the isLoading prop passed to SyncReviewPage).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/OpenAPISyncTab/CollectionStatusSection/index.js`:
- Around line 228-239: Fix the double-period typo in the user-facing messages
inside the CollectionStatusSection component: update the conditional message
strings that reference lastSyncDate (the two places rendering 'Last synced spec
is required...' and 'Restore the latest spec from the source to track future
changes..') to end with a single period; check both the top span message and the
<p> message that uses lastSyncDate and remove the extra '.' so each sentence
ends with one period.
In `@packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js`:
- Line 264: The displayed no-update message in the SyncReviewPage component
contains incorrect grammar ("The spec endpoints has not been updated...");
update the string used in the JSX (the paragraph text in SyncReviewPage) to use
correct plural agreement, e.g. change "The spec endpoints has not been updated
since the last sync." to "The spec endpoints have not been updated since the
last sync." so the message reads grammatically correct.
---
Nitpick comments:
In `@packages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.js`:
- Around line 5-6: The import list in SpecStatusSection includes an unused
symbol IconLoader2; remove IconLoader2 from the import statement (the import
that currently lists IconRefresh and IconLoader2) so only IconRefresh is
imported, and ensure no other references to IconLoader2 remain in the file (the
loading UI is already handled via the isLoading prop passed to SyncReviewPage).
In `@packages/bruno-app/src/components/OpenAPISyncTab/utils.js`:
- Around line 10-13: The loop in countEndpoints can throw when iterating
Object.keys(path) if a path entry is null or not an object; add a guard before
using Object.keys to ensure each path is a plain object (e.g. check path !==
null && typeof path === "object") and skip non-objects, then proceed to iterate
keys and test against HTTP_METHODS; update countEndpoints to perform that check
(referencing the countEndpoints function and HTTP_METHODS constant) so malformed
spec.paths entries are safely ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d7ef166-6b97-4680-8dc6-ae4ee3a04750
📒 Files selected for processing (14)
packages/bruno-app/src/components/OpenAPISpecTab/index.jspackages/bruno-app/src/components/OpenAPISyncTab/CollectionStatusSection/index.jspackages/bruno-app/src/components/OpenAPISyncTab/ConnectSpecForm/index.jspackages/bruno-app/src/components/OpenAPISyncTab/ConnectionSettingsModal/index.jspackages/bruno-app/src/components/OpenAPISyncTab/OpenAPISyncHeader/index.jspackages/bruno-app/src/components/OpenAPISyncTab/OverviewSection/index.jspackages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.jspackages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.jspackages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.jspackages/bruno-app/src/components/OpenAPISyncTab/hooks/useOpenAPISync.jspackages/bruno-app/src/components/OpenAPISyncTab/index.jspackages/bruno-app/src/components/OpenAPISyncTab/utils.jspackages/bruno-app/src/providers/ReduxStore/slices/openapi-sync.jspackages/bruno-electron/src/ipc/openapi-sync.js
- Added new entries to .gitignore for agent-related files and skills-lock.json. - Modified StyledWrapper to improve overflow handling and added sticky headers for better visibility. - Introduced loading state in SpecDiffModal with a spinner for improved user feedback during rendering.
…ndering - Replaced the JSON parsing and stringifying logic with fast-json-format for better performance in pretty-printing API specifications. - Updated StyledWrapper in OpenAPISyncTab to change background and text colors for enhanced visual consistency. - Modified DisconnectSyncModal button to include a secondary color for improved visibility during user interactions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js (1)
1740-1747: Consider honoring reduced-motion for spinner animation.Line 1745-1747 introduces a new spinner animation; adding a reduced-motion fallback would improve accessibility with minimal cost.
♿ Optional tweak
.spinner-icon { animation: spin 1s linear infinite; } + + `@media` (prefers-reduced-motion: reduce) { + .spinner-icon { + animation: none; + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js` around lines 1740 - 1747, Add a reduced-motion fallback for the spinner by wrapping the .spinner-icon animation rule in a prefers-reduced-motion media query (or add a separate `@media` (prefers-reduced-motion: reduce) block) and set animation: none (or animation-duration: 0s) for .spinner-icon there; update the spinner/keyframes usage in StyledWrapper (the .spinner-icon rule and any spin keyframes) so users who prefer reduced motion won’t see the continuous spin.packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js (1)
20-39: Loading state may not be visible due to synchronous execution.The
setIsRendering(true)→ synchronous DOM mutation →setIsRendering(false)sequence runs within a single useEffect callback. Since React batches state updates, the intermediateisRendering=truestate may never actually trigger a re-render before it's immediately set back tofalse.For fast diffs, this is fine. For large specs where
Diff2Html.html()takes noticeable time, the loading indicator still won't appear because the DOM won't repaint until the synchronous work completes. If you want the spinner to be visible for heavy diffs, you'd need to defer the rendering work (e.g.,requestAnimationFrameorsetTimeout(..., 0)).That said, this is a minor UX consideration — the current implementation is functional and unlikely to cause issues in practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js` around lines 20 - 39, The useEffect currently sets setIsRendering(true), runs the synchronous heavy Diff2Html.html(...) and immediately calls setIsRendering(false) so the loading state may never render; modify the effect to defer the expensive work (e.g., wrap the Diff2Html.html(...) call and diffRef.current.innerHTML assignment in requestAnimationFrame or setTimeout(..., 0)) after calling setIsRendering(true), and ensure setIsRendering(false) runs after the DOM mutation completes (or in the same deferred callback) so the spinner can actually appear for large diffs; update the effect that uses Diff2Html.html, diffRef.current.innerHTML, and setIsRendering accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 54-56: The new .gitignore entries (.agents, .agent,
skills-lock.json) look unrelated to the OpenAPI Sync work and need
clarification; either remove them from this PR or document why they belong here:
if they are incidental housekeeping, move them to a separate housekeeping PR,
otherwise update the PR description and commit message to explain their purpose
and scope (mentioning .agents, .agent, skills-lock.json) so reviewers understand
they are intentional and what tooling or artifacts they ignore.
---
Nitpick comments:
In `@packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js`:
- Around line 20-39: The useEffect currently sets setIsRendering(true), runs the
synchronous heavy Diff2Html.html(...) and immediately calls
setIsRendering(false) so the loading state may never render; modify the effect
to defer the expensive work (e.g., wrap the Diff2Html.html(...) call and
diffRef.current.innerHTML assignment in requestAnimationFrame or setTimeout(...,
0)) after calling setIsRendering(true), and ensure setIsRendering(false) runs
after the DOM mutation completes (or in the same deferred callback) so the
spinner can actually appear for large diffs; update the effect that uses
Diff2Html.html, diffRef.current.innerHTML, and setIsRendering accordingly.
In `@packages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js`:
- Around line 1740-1747: Add a reduced-motion fallback for the spinner by
wrapping the .spinner-icon animation rule in a prefers-reduced-motion media
query (or add a separate `@media` (prefers-reduced-motion: reduce) block) and set
animation: none (or animation-duration: 0s) for .spinner-icon there; update the
spinner/keyframes usage in StyledWrapper (the .spinner-icon rule and any spin
keyframes) so users who prefer reduced motion won’t see the continuous spin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 343a2515-e89b-4023-8ffe-29d14b5d1fae
📒 Files selected for processing (5)
.gitignorepackages/bruno-app/src/components/OpenAPISpecTab/index.jspackages/bruno-app/src/components/OpenAPISyncTab/DisconnectSyncModal/index.jspackages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.jspackages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/OpenAPISpecTab/index.js
…tles - Removed unnecessary trailing periods in messages related to syncing and restoring specifications across CollectionStatusSection and OverviewSection components. - Updated SyncReviewPage to correct grammatical error in the description of spec updates.
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/components/OpenAPISyncTab/SpecStatusSection/index.js (1)
125-137:⚠️ Potential issue | 🟠 MajorHide
SyncReviewPagefor the explicit error states above.Line 125 now falls through to
SyncReviewPageeven for file-not-found, invalid-spec, or comparison-error cases. SinceremoteDriftis only populated after a successful compare inpackages/bruno-app/src/components/OpenAPISyncTab/hooks/useOpenAPISync.js:168-178, those failures end up showing the "No updates from the spec" empty state under a danger banner.Suggested fix
- ) : ( + ) : fileNotFound || error || specDrift?.isValid === false ? null : ( <div className="mt-5"> <SyncReviewPage specDrift={specDrift}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.js` around lines 125 - 137, The SyncReviewPage is rendered even when explicit error states (file-not-found, invalid-spec, comparison-error) occurred; update the JSX so SyncReviewPage only renders when there are no error flags and a successful compare result exists (e.g., remoteDrift is populated). Concretely, in the component rendering block that currently renders <SyncReviewPage ... />, guard it with a combined condition such as "remoteDrift && !fileNotFound && !invalidSpec && !comparisonError" (or whatever error boolean names are used in this file) so that SyncReviewPage (props: specDrift, remoteDrift, collectionDrift, collectionPath, collectionUid, newSpec, isSyncing, isLoading, onApplySync/handleApplySync) is hidden for those explicit error states described in useOpenAPISync.
🧹 Nitpick comments (2)
packages/bruno-app/src/components/OpenAPISyncTab/OverviewSection/index.js (2)
123-123: Minor copy polish for subtitle wording.Line 123 reads more naturally as “missing in storage” (instead of “missing in the storage”).
✍️ Copy tweak
- subtitle: 'The last synced spec is missing in the storage. Restore the latest spec from the source to track future changes.', + subtitle: 'The last synced spec is missing in storage. Restore the latest spec from the source to track future changes.',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/OpenAPISyncTab/OverviewSection/index.js` at line 123, Update the subtitle text in the OverviewSection component so it reads "missing in storage" instead of "missing in the storage": locate the subtitle property (key: subtitle) inside the OverviewSection (or the OpenAPISyncTab OverviewSection component) and change the string to "The last synced spec is missing in storage. Restore the latest spec from the source to track future changes." to apply the copy polish.
50-51: Make endpoint fallback explicit whenstoredSpecis absent.On Line 51, fallback to
specMeta?.endpointCountcurrently depends on whatcountEndpoints()returns for missing input. Guarding onstoredSpecdirectly avoids accidental0masking fallback metadata.♻️ Suggested tweak
- const endpointCount = countEndpoints(storedSpec) ?? specMeta?.endpointCount ?? null; + const endpointCount = storedSpec + ? countEndpoints(storedSpec) + : (specMeta?.endpointCount ?? null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/OpenAPISyncTab/OverviewSection/index.js` around lines 50 - 51, The endpoint fallback currently calls countEndpoints(storedSpec) and then falls back to specMeta?.endpointCount, which can incorrectly use the fallback when countEndpoints returns 0; update the endpointCount logic to first check storedSpec presence and only call countEndpoints when storedSpec exists (so a returned 0 from countEndpoints is preserved), otherwise use specMeta?.endpointCount; adjust the expression around endpointCount (referencing storedSpec, countEndpoints, and specMeta?.endpointCount) so the fallback is explicit when storedSpec is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/OpenAPISyncTab/CollectionStatusSection/index.js`:
- Around line 43-45: The code currently treats a null collectionDrift as "no
stored spec" because hasStoredSpec is computed as collectionDrift &&
!collectionDrift.noStoredSpec; change hasStoredSpec to explicitly check for a
non-null result (e.g., collectionDrift !== null &&
!collectionDrift.noStoredSpec) and update hasDrift to guard similarly (e.g.,
collectionDrift !== null && (collectionDrift.modified?.length > 0 ||
collectionDrift.missing?.length > 0)); ensure any rendering logic that branches
on !hasStoredSpec instead distinguishes between "loading/unknown"
(collectionDrift === null), "noStoredSpec" (collectionDrift?.noStoredSpec ===
true), and "hasStoredSpec" using the new guards so the
initial-loading/failed-to-load state is not rendered as missing-spec.
---
Outside diff comments:
In `@packages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.js`:
- Around line 125-137: The SyncReviewPage is rendered even when explicit error
states (file-not-found, invalid-spec, comparison-error) occurred; update the JSX
so SyncReviewPage only renders when there are no error flags and a successful
compare result exists (e.g., remoteDrift is populated). Concretely, in the
component rendering block that currently renders <SyncReviewPage ... />, guard
it with a combined condition such as "remoteDrift && !fileNotFound &&
!invalidSpec && !comparisonError" (or whatever error boolean names are used in
this file) so that SyncReviewPage (props: specDrift, remoteDrift,
collectionDrift, collectionPath, collectionUid, newSpec, isSyncing, isLoading,
onApplySync/handleApplySync) is hidden for those explicit error states described
in useOpenAPISync.
---
Nitpick comments:
In `@packages/bruno-app/src/components/OpenAPISyncTab/OverviewSection/index.js`:
- Line 123: Update the subtitle text in the OverviewSection component so it
reads "missing in storage" instead of "missing in the storage": locate the
subtitle property (key: subtitle) inside the OverviewSection (or the
OpenAPISyncTab OverviewSection component) and change the string to "The last
synced spec is missing in storage. Restore the latest spec from the source to
track future changes." to apply the copy polish.
- Around line 50-51: The endpoint fallback currently calls
countEndpoints(storedSpec) and then falls back to specMeta?.endpointCount, which
can incorrectly use the fallback when countEndpoints returns 0; update the
endpointCount logic to first check storedSpec presence and only call
countEndpoints when storedSpec exists (so a returned 0 from countEndpoints is
preserved), otherwise use specMeta?.endpointCount; adjust the expression around
endpointCount (referencing storedSpec, countEndpoints, and
specMeta?.endpointCount) so the fallback is explicit when storedSpec is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26d6f599-e823-4286-87d4-ebfcbf8c49de
📒 Files selected for processing (4)
packages/bruno-app/src/components/OpenAPISyncTab/CollectionStatusSection/index.jspackages/bruno-app/src/components/OpenAPISyncTab/OverviewSection/index.jspackages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.jspackages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js
- Replaced isValidUrl with isHttpUrl in ConnectSpecForm and ConnectionSettingsModal components to ensure only valid HTTP URLs are accepted. - Updated the logic for enabling the save button based on the new URL validation method.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/OpenAPISyncTab/ConnectionSettingsModal/index.js`:
- Line 11: Trim/normalize the incoming sourceUrl once before you infer modal
mode so leading/trailing whitespace won't make a valid HTTP URL be treated as a
file; specifically, introduce a normalizedSourceUrl (e.g., trimmed string or
fallback) and use that when calling isHttpUrl (where isUrl is computed) and
anywhere else you inspect sourceUrl for mode initialization (the same places
referenced around isUrl and the modal mode inference logic) so all checks
operate on the trimmed value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 180f1b7c-9b04-4ebe-b8cf-87f10f6d0569
📒 Files selected for processing (2)
packages/bruno-app/src/components/OpenAPISyncTab/ConnectSpecForm/index.jspackages/bruno-app/src/components/OpenAPISyncTab/ConnectionSettingsModal/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/OpenAPISyncTab/ConnectSpecForm/index.js
| const ConnectionSettingsModal = ({ collection, sourceUrl, onSave, onDisconnect, onClose }) => { | ||
| const openApiSyncConfig = collection?.brunoConfig?.openapi?.[0]; | ||
| const isUrl = isValidUrl(sourceUrl); | ||
| const isUrl = isHttpUrl(sourceUrl); |
Contributor
There was a problem hiding this comment.
Normalize sourceUrl once before mode inference to avoid whitespace edge cases.
If persisted sourceUrl has leading/trailing spaces, modal mode can initialize as file even for valid HTTP URLs.
💡 Proposed fix
- const isUrl = isHttpUrl(sourceUrl);
+ const normalizedSourceUrl = (sourceUrl || '').trim();
+ const isUrl = isHttpUrl(normalizedSourceUrl);
const initialMode = isUrl ? 'url' : 'file';
const [mode, setMode] = useState(initialMode);
- const [url, setUrl] = useState(isUrl ? (sourceUrl || '') : '');
+ const [url, setUrl] = useState(isUrl ? normalizedSourceUrl : '');
const [filePath, setFilePath] = useState(isUrl ? '' : sourceUrl);
@@
- const effectiveSource = mode === 'file' ? filePath : url.trim();
- const canSave = mode === 'file' ? !!effectiveSource : isHttpUrl(effectiveSource.trim());
+ const normalizedUrl = url.trim();
+ const effectiveSource = mode === 'file' ? filePath : normalizedUrl;
+ const canSave = mode === 'file' ? !!effectiveSource : isHttpUrl(normalizedUrl);Also applies to: 24-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/bruno-app/src/components/OpenAPISyncTab/ConnectionSettingsModal/index.js`
at line 11, Trim/normalize the incoming sourceUrl once before you infer modal
mode so leading/trailing whitespace won't make a valid HTTP URL be treated as a
file; specifically, introduce a normalizedSourceUrl (e.g., trimmed string or
fallback) and use that when calling isHttpUrl (where isUrl is computed) and
anywhere else you inspect sourceUrl for mode initialization (the same places
referenced around isUrl and the modal mode inference logic) so all checks
operate on the trimmed value.
- Trimmed the source URL in ConnectionSettingsModal to ensure consistent validation with isHttpUrl. - Updated state initialization for URL and filePath to use the normalized source URL, improving handling of user input.
bijin-bruno
approved these changes
Mar 13, 2026
lohit-bruno
pushed a commit
to lohit-bruno/bruno
that referenced
this pull request
Mar 18, 2026
* fix: specify OpenAPI 3.x in error messages for file uploads and URL validation Updated error messages in ConnectSpecForm and ConnectionSettingsModal to clarify that only OpenAPI 3.x specifications are valid. Enhanced useOpenAPISync hook to reflect the same specificity in error handling for invalid URLs. * feat(OpenAPISpecTab): add pretty-printing for JSON content in API spec viewer Implemented a new function to pretty-print JSON content for improved readability in the OpenAPISpecTab component. This enhancement ensures that JSON specifications are displayed in a more user-friendly format while leaving YAML content unchanged. * feat(OpenAPISyncHeader): resolve and display absolute file paths for local sources Added functionality to resolve relative file paths to absolute paths for better user experience in the OpenAPISyncHeader component. Implemented state management and side effects to handle path resolution based on the source URL, enhancing the display of local file paths. * feat(OpenAPISyncTab): enhance collection status display and add endpoint counting utility Refactored the CollectionStatusSection to streamline the display of collection drift status, integrating loading states and improved messaging for initial sync scenarios. Introduced a new utility function to count HTTP endpoints in OpenAPI specifications, enhancing the overall functionality of the OpenAPISyncTab. Additionally, updated the OpenAPISyncHeader and OverviewSection to utilize stored specification metadata for better user experience. * refactor: improve OpenAPI Sync endpoint handling - Enhanced the logic for adding new requests by ensuring existing files are verified before removal to prevent accidental deletions. - Streamlined the process of adding new endpoints, including checks for existing files and merging requests to maintain user customizations. - Added comments for clarity on the purpose of changes, particularly regarding filename collision prevention and file content verification. * style(OpenAPISyncTab): update styles for improved visual feedback - Changed background color for the 'type-spec-modified' class to a warning color for better distinction. - Updated text color and background for the SyncReviewPage to enhance readability and visual hierarchy. - Adjusted default expanded states for endpoint sections to improve user experience during sync reviews. * chore: update .gitignore and enhance OpenAPISyncTab components - Added new entries to .gitignore for agent-related files and skills-lock.json. - Modified StyledWrapper to improve overflow handling and added sticky headers for better visibility. - Introduced loading state in SpecDiffModal with a spinner for improved user feedback during rendering. * feat(OpenAPISpecTab): integrate fast-json-format for improved JSON rendering - Replaced the JSON parsing and stringifying logic with fast-json-format for better performance in pretty-printing API specifications. - Updated StyledWrapper in OpenAPISyncTab to change background and text colors for enhanced visual consistency. - Modified DisconnectSyncModal button to include a secondary color for improved visibility during user interactions. * fix(OpenAPISyncTab): correct punctuation in status messages and subtitles - Removed unnecessary trailing periods in messages related to syncing and restoring specifications across CollectionStatusSection and OverviewSection components. - Updated SyncReviewPage to correct grammatical error in the description of spec updates. * fix(OpenAPISyncTab): update URL validation to use isHttpUrl - Replaced isValidUrl with isHttpUrl in ConnectSpecForm and ConnectionSettingsModal components to ensure only valid HTTP URLs are accepted. - Updated the logic for enabling the save button based on the new URL validation method. * fix(OpenAPISyncTab): normalize source URL before validation - Trimmed the source URL in ConnectionSettingsModal to ensure consistent validation with isHttpUrl. - Updated state initialization for URL and filePath to use the normalized source URL, improving handling of user input.
12 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.
Description
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements