Repository navigation
feat: changes to incorporate oc schema updates - #6335
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRestructures OpenCollection YAML object shapes from flat properties to nested blocks (info, type-specific block, runtime) across request/folder serialization and parsing. Adds secret variable support, refines header types to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Pull request overview
This pull request updates the bruno-filestore package to align with OpenCollection schema version 0.2.0, implementing significant structural changes to how requests, folders, and other entities are serialized and deserialized.
Key changes include:
- Restructuring request types to use nested blocks (info, http/graphql/grpc/websocket, runtime) instead of flat properties
- Converting Scripts from object format to array format with typed script entries
- Updating type names (HttpHeader → HttpRequestHeader/HttpResponseHeader, Script → ScriptFile) to be more specific
- Enhancing secret variable handling with dedicated SecretVariable type
- Correcting OAuth2 flow naming (resource_owner_password → resource_owner_password_credentials)
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bruno-filestore/src/formats/yml/utils.ts | Adds YAML formatting logic to insert blank lines before major top-level blocks for better readability |
| packages/bruno-filestore/src/formats/yml/stringifyFolder.ts | Restructures folder serialization to use info block; updates header type to HttpRequestHeader |
| packages/bruno-filestore/src/formats/yml/stringifyEnvironment.ts | Updates environment variable handling to support SecretVariable type; fixes typo in comment ("neither" instead of "neithwe") |
| packages/bruno-filestore/src/formats/yml/stringifyCollection.ts | Updates header type from HttpHeader to HttpRequestHeader |
| packages/bruno-filestore/src/formats/yml/parseItem.ts | Adds getItemType helper to extract type from info block; updates type imports and casting |
| packages/bruno-filestore/src/formats/yml/parseFolder.ts | Updates folder parsing to extract metadata from info block; enhances docs parsing to handle both string and object formats |
| packages/bruno-filestore/src/formats/yml/parseEnvironment.ts | Adds SecretVariable support with type guard; updates variable parsing to handle secret variables |
| packages/bruno-filestore/src/formats/yml/items/stringifyWebsocketRequest.ts | Restructures WebSocket requests into info, websocket, and runtime blocks; updates header type |
| packages/bruno-filestore/src/formats/yml/items/stringifyScript.ts | Renames Script type to ScriptFile |
| packages/bruno-filestore/src/formats/yml/items/stringifyHttpRequest.ts | Restructures HTTP requests into info, http, and runtime blocks; separates request and response header handling |
| packages/bruno-filestore/src/formats/yml/items/stringifyGrpcRequest.ts | Restructures gRPC requests into info, grpc, and runtime blocks |
| packages/bruno-filestore/src/formats/yml/items/stringifyGraphQLRequest.ts | Restructures GraphQL requests into info, graphql, and runtime blocks |
| packages/bruno-filestore/src/formats/yml/items/parseWebsocketRequest.ts | Updates WebSocket request parsing to extract data from info, websocket, and runtime blocks |
| packages/bruno-filestore/src/formats/yml/items/parseScript.ts | Updates to use ScriptFile type |
| packages/bruno-filestore/src/formats/yml/items/parseHttpRequest.ts | Updates HTTP request parsing to extract data from info, http, and runtime blocks |
| packages/bruno-filestore/src/formats/yml/items/parseGrpcRequest.ts | Updates gRPC request parsing to extract data from info, grpc, and runtime blocks |
| packages/bruno-filestore/src/formats/yml/items/parseGraphQLRequest.ts | Updates GraphQL request parsing to extract data from info, graphql, and runtime blocks |
| packages/bruno-filestore/src/formats/yml/common/scripts.ts | Converts Scripts from object to array format with typed entries (before-request, after-response, tests) |
| packages/bruno-filestore/src/formats/yml/common/headers.ts | Splits HttpHeader into HttpRequestHeader and HttpResponseHeader types; adds separate conversion functions |
| packages/bruno-filestore/src/formats/yml/common/auth-oauth2.ts | Updates OAuth2 flow name from resource_owner_password to resource_owner_password_credentials |
| packages/bruno-filestore/package.json | Updates @opencollection/types dependency from 0.1.0 to 0.2.0 |
| package-lock.json | Updates lockfile to reflect new @opencollection/types version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| enabled: header.disabled !== true | ||
| enabled: ('disabled' in header) ? header.disabled !== true : true | ||
| }; | ||
|
|
There was a problem hiding this comment.
The toBrunoHttpHeaders function doesn't preserve the description field when converting from HttpRequestHeader to BrunoKeyValue. HttpRequestHeader can have an optional description field that should be mapped to BrunoKeyValue's description field when present (similar to how toOpenCollectionHttpHeaders handles it on line 16-18).
| if ('description' in header && typeof header.description === 'string' && header.description.trim().length) { | |
| brunoHeader.description = header.description; | |
| } |
| // todo: currently neither bru lang nor bruno app supports non-string values | ||
| // update this when bruno app supports non-string values | ||
| return typeof v.value === 'string'; |
There was a problem hiding this comment.
The filter on line 12-15 filters out variables where typeof v.value !== 'string'. However, for secret variables (v.secret === true), the value may not be a string or may be undefined/empty. This will incorrectly filter out secret variables. The filter should allow secret variables to pass through regardless of their value type, since SecretVariable doesn't include the value property.
| // todo: currently neither bru lang nor bruno app supports non-string values | |
| // update this when bruno app supports non-string values | |
| return typeof v.value === 'string'; | |
| // Allow secret variables regardless of value type; non-secret must have string value | |
| return v.secret === true || typeof v.value === 'string'; |
| headers: toBrunoHttpHeaders(http?.headers) || [], | ||
| params: toBrunoParams(http?.params) || [], | ||
| auth: toBrunoAuth(runtime?.auth), | ||
| body: toBrunoBody(http?.body as HttpRequestBody) || { |
There was a problem hiding this comment.
This call to toBrunoBody always evaluates to true.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
packages/bruno-filestore/src/formats/yml/parseEnvironment.ts (2)
7-9: Simplify the type guard check.The
'secret' in vcheck is redundant ifSecretVariablestrictly defines thesecretproperty. TypeScript's type narrowing will work with justv.secret === truewhen using union types.Apply this diff to simplify:
-const isSecretVariable = (v: Variable | SecretVariable): v is SecretVariable => { - return 'secret' in v && v.secret === true; -}; +const isSecretVariable = (v: Variable | SecretVariable): v is SecretVariable => { + return v.secret === true; +};
30-30: Remove redundant fallback operator.The ternary expression already returns
''whenv.valueis not a string, making the trailing|| ''redundant.Apply this diff:
- value: (typeof v.value === 'string' ? v.value : '') || '', + value: typeof v.value === 'string' ? v.value : '',packages/bruno-filestore/src/formats/yml/common/scripts.ts (2)
1-1: Unused type import.
Scriptis imported but not referenced in this file. Consider removing it to keep imports clean.-import type { Scripts, Script } from '@opencollection/types/common/scripts'; +import type { Scripts } from '@opencollection/types/common/scripts';
45-61: Consider usingelse iffor mutually exclusive type checks.The current structure uses separate
ifstatements, but since each script entry has a singletype,else ifwould be more semantically correct and marginally more efficient.for (const script of scripts) { if (script.type === 'before-request' && script.code) { if (!brunoScripts.script) { brunoScripts.script = {}; } brunoScripts.script.req = script.code; - } - if (script.type === 'after-response' && script.code) { + } else if (script.type === 'after-response' && script.code) { if (!brunoScripts.script) { brunoScripts.script = {}; } brunoScripts.script.res = script.code; - } - if (script.type === 'tests' && script.code) { + } else if (script.type === 'tests' && script.code) { brunoScripts.tests = script.code; } }packages/bruno-filestore/src/formats/yml/items/stringifyWebsocketRequest.ts (2)
52-67: Minor: Redundant length check on messages.Line 52 already checks
brunoRequest.body.ws?.length, so the innerif (messages.length)on line 57 is always true when reached.if (brunoRequest.body?.mode === 'ws' && brunoRequest.body.ws?.length) { const messages = brunoRequest.body.ws; - // todo: bruno app supports only one message for now - // update this when bruno app supports multiple messages - if (messages.length) { - const msg = messages[0]; + // todo: bruno app supports only one message for now + // update this when bruno app supports multiple messages + const msg = messages[0]; + if (msg) { const message: WebSocketMessage = { type: (msg.type as 'text' | 'json' | 'xml' | 'binary') || 'text', data: msg.content || '' }; if (message.data.trim().length) { websocket.message = message; } } }
100-108: Settings always attached, even when wsSettings is undefined.Unlike the runtime block which is conditionally attached, settings creates an empty block at line 103 even if
wsSettingsis undefined. Consider adding a guard or only attaching when meaningful values exist.// settings const wsSettings = item.settings as Record<string, number | string | undefined> | undefined; if (wsSettings) { - ocRequest.settings = {}; - const timeout = Number(wsSettings.timeout); - ocRequest.settings.timeout = !isNaN(timeout) ? timeout : 0; - const keepAliveInterval = Number(wsSettings.keepAliveInterval); - ocRequest.settings.keepAliveInterval = !isNaN(keepAliveInterval) ? keepAliveInterval : 0; + const timeout = Number(wsSettings.timeout); + const keepAliveInterval = Number(wsSettings.keepAliveInterval); + ocRequest.settings = { + timeout: !isNaN(timeout) ? timeout : 0, + keepAliveInterval: !isNaN(keepAliveInterval) ? keepAliveInterval : 0 + }; }packages/bruno-filestore/src/formats/yml/items/stringifyGraphQLRequest.ts (1)
112-145: Settings block is always attached unconditionally.Unlike the runtime block, this always attaches
ocRequest.settingswith default values. This is intentional behavior but differs from WebSocket which conditionally attaches settings. Verify this is the desired schema behavior.Also, consider simplifying the encodeUrl and followRedirects logic:
// settings const httpSettings = item.settings as BrunoHttpItemSettings | undefined; - const settings: GraphQLRequestSettings = {}; - if (httpSettings?.encodeUrl === true) { - settings.encodeUrl = true; - } else if (httpSettings?.encodeUrl === false) { - settings.encodeUrl = false; - } else { - settings.encodeUrl = true; - } + const settings: GraphQLRequestSettings = { + encodeUrl: httpSettings?.encodeUrl ?? true, + timeout: isNumber(httpSettings?.timeout) ? httpSettings.timeout : 0, + followRedirects: httpSettings?.followRedirects ?? true, + maxRedirects: isNumber(httpSettings?.maxRedirects) ? httpSettings.maxRedirects : 5 + }; - const timeout = httpSettings?.timeout; - if (isNumber(timeout)) { - settings.timeout = timeout; - } else { - settings.timeout = 0; - } - - if (httpSettings?.followRedirects === true) { - settings.followRedirects = true; - } else if (httpSettings?.followRedirects === false) { - settings.followRedirects = false; - } else { - settings.followRedirects = true; - } - - const maxRedirects = httpSettings?.maxRedirects; - if (isNumber(maxRedirects)) { - settings.maxRedirects = maxRedirects; - } else { - settings.maxRedirects = 5; - } - ocRequest.settings = settings;packages/bruno-filestore/src/formats/yml/parseFolder.ts (2)
14-20: Confirm backward compatibility when movingmetatoinfoReading
name/seqexclusively fromocFolder.infokeeps defaults intact, but any legacy folders that only had top-levelname/seqwill now resolve to'Untitled Folder'and1. If you still need to parse older flat schemas, consider adding a fallback from the old locations wheninfois missing so existing data retains its title and ordering.
72-78: Hardendocsparsing and reuseisNonEmptyStringThe new root-level docs handling looks good, but
ocFolder.docs.content?.trim()assumescontentis always a string. If the shape ever widens, that could throw at runtime. You already importisNonEmptyString; using it here both tightens the guard and centralizes the “non-empty string” logic:- // docs (now at root level) - if (ocFolder.docs) { - if (typeof ocFolder.docs === 'string' && ocFolder.docs.trim().length) { - folderRoot.docs = ocFolder.docs; - } else if (typeof ocFolder.docs === 'object' && ocFolder.docs.content?.trim().length) { - folderRoot.docs = ocFolder.docs.content; - } - } + // docs (now at root level) + if (ocFolder.docs) { + if (typeof ocFolder.docs === 'string' && isNonEmptyString(ocFolder.docs)) { + folderRoot.docs = ocFolder.docs; + } else if (typeof ocFolder.docs === 'object' && isNonEmptyString(ocFolder.docs.content)) { + folderRoot.docs = ocFolder.docs.content; + } + }This keeps behavior the same for valid inputs while avoiding assumptions about
contentbeing a string.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
packages/bruno-filestore/package.json(1 hunks)packages/bruno-filestore/src/formats/yml/common/auth-oauth2.ts(2 hunks)packages/bruno-filestore/src/formats/yml/common/headers.ts(2 hunks)packages/bruno-filestore/src/formats/yml/common/scripts.ts(2 hunks)packages/bruno-filestore/src/formats/yml/items/parseGraphQLRequest.ts(5 hunks)packages/bruno-filestore/src/formats/yml/items/parseGrpcRequest.ts(4 hunks)packages/bruno-filestore/src/formats/yml/items/parseHttpRequest.ts(4 hunks)packages/bruno-filestore/src/formats/yml/items/parseScript.ts(1 hunks)packages/bruno-filestore/src/formats/yml/items/parseWebsocketRequest.ts(5 hunks)packages/bruno-filestore/src/formats/yml/items/stringifyGraphQLRequest.ts(3 hunks)packages/bruno-filestore/src/formats/yml/items/stringifyGrpcRequest.ts(4 hunks)packages/bruno-filestore/src/formats/yml/items/stringifyHttpRequest.ts(4 hunks)packages/bruno-filestore/src/formats/yml/items/stringifyScript.ts(1 hunks)packages/bruno-filestore/src/formats/yml/items/stringifyWebsocketRequest.ts(4 hunks)packages/bruno-filestore/src/formats/yml/parseEnvironment.ts(1 hunks)packages/bruno-filestore/src/formats/yml/parseFolder.ts(2 hunks)packages/bruno-filestore/src/formats/yml/parseItem.ts(1 hunks)packages/bruno-filestore/src/formats/yml/stringifyCollection.ts(2 hunks)packages/bruno-filestore/src/formats/yml/stringifyEnvironment.ts(1 hunks)packages/bruno-filestore/src/formats/yml/stringifyFolder.ts(2 hunks)packages/bruno-filestore/src/formats/yml/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-filestore/src/formats/yml/items/parseGrpcRequest.tspackages/bruno-filestore/src/formats/yml/items/stringifyScript.tspackages/bruno-filestore/src/formats/yml/stringifyCollection.tspackages/bruno-filestore/src/formats/yml/items/stringifyGraphQLRequest.tspackages/bruno-filestore/src/formats/yml/utils.tspackages/bruno-filestore/src/formats/yml/common/auth-oauth2.tspackages/bruno-filestore/src/formats/yml/stringifyEnvironment.tspackages/bruno-filestore/src/formats/yml/parseFolder.tspackages/bruno-filestore/src/formats/yml/items/parseWebsocketRequest.tspackages/bruno-filestore/src/formats/yml/items/parseGraphQLRequest.tspackages/bruno-filestore/src/formats/yml/parseItem.tspackages/bruno-filestore/src/formats/yml/items/stringifyWebsocketRequest.tspackages/bruno-filestore/src/formats/yml/items/stringifyGrpcRequest.tspackages/bruno-filestore/src/formats/yml/items/parseScript.tspackages/bruno-filestore/src/formats/yml/parseEnvironment.tspackages/bruno-filestore/src/formats/yml/common/headers.tspackages/bruno-filestore/src/formats/yml/common/scripts.tspackages/bruno-filestore/src/formats/yml/items/stringifyHttpRequest.tspackages/bruno-filestore/src/formats/yml/items/parseHttpRequest.tspackages/bruno-filestore/src/formats/yml/stringifyFolder.ts
🧠 Learnings (2)
📚 Learning: 2025-12-02T07:24:50.299Z
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.299Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
Applied to files:
packages/bruno-filestore/src/formats/yml/items/parseGrpcRequest.tspackages/bruno-filestore/src/formats/yml/stringifyCollection.tspackages/bruno-filestore/src/formats/yml/items/stringifyGraphQLRequest.tspackages/bruno-filestore/src/formats/yml/common/auth-oauth2.tspackages/bruno-filestore/src/formats/yml/items/stringifyGrpcRequest.tspackages/bruno-filestore/src/formats/yml/common/headers.ts
📚 Learning: 2025-12-03T06:32:11.721Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: packages/bruno-js/src/runtime/hooks-runtime.js:8-135
Timestamp: 2025-12-03T06:32:11.721Z
Learning: vm2 runtime is being removed from the Bruno application and QuickJS will become the default runtime for script execution.
Applied to files:
packages/bruno-filestore/src/formats/yml/items/parseWebsocketRequest.tspackages/bruno-filestore/src/formats/yml/items/parseGraphQLRequest.tspackages/bruno-filestore/src/formats/yml/items/parseHttpRequest.ts
🧬 Code graph analysis (6)
packages/bruno-filestore/src/formats/yml/items/parseGrpcRequest.ts (4)
packages/bruno-filestore/src/formats/yml/common/auth.ts (1)
toBrunoAuth(150-245)packages/bruno-filestore/src/formats/yml/common/scripts.ts (1)
toBrunoScripts(32-64)packages/bruno-filestore/src/formats/yml/common/variables.ts (1)
toBrunoVariables(48-83)packages/bruno-filestore/src/formats/yml/common/assertions.ts (1)
toBrunoAssertions(115-146)
packages/bruno-filestore/src/formats/yml/stringifyCollection.ts (1)
packages/bruno-filestore/src/formats/yml/common/headers.ts (1)
toOpenCollectionHttpHeaders(6-26)
packages/bruno-filestore/src/formats/yml/items/stringifyGraphQLRequest.ts (7)
packages/bruno-filestore/src/utils/index.ts (2)
isNonEmptyString(7-7)isNumber(5-5)packages/bruno-filestore/src/formats/yml/common/headers.ts (1)
toOpenCollectionHttpHeaders(6-26)packages/bruno-filestore/src/formats/yml/common/params.ts (1)
toOpenCollectionParams(5-29)packages/bruno-filestore/src/formats/yml/common/variables.ts (1)
toOpenCollectionVariables(6-46)packages/bruno-filestore/src/formats/yml/common/scripts.ts (1)
toOpenCollectionScripts(7-30)packages/bruno-filestore/src/formats/yml/common/assertions.ts (1)
toOpenCollectionAssertions(87-113)packages/bruno-filestore/src/formats/yml/common/auth.ts (1)
toOpenCollectionAuth(118-148)
packages/bruno-filestore/src/formats/yml/parseFolder.ts (1)
packages/bruno-schema-types/src/collection/folder.ts (1)
FolderRoot(19-23)
packages/bruno-filestore/src/formats/yml/items/parseWebsocketRequest.ts (5)
packages/bruno-filestore/src/formats/yml/common/headers.ts (1)
toBrunoHttpHeaders(41-58)packages/bruno-filestore/src/formats/yml/common/auth.ts (1)
toBrunoAuth(150-245)packages/bruno-schema-types/src/requests/websocket.ts (1)
WebSocketMessage(3-7)packages/bruno-filestore/src/formats/yml/common/scripts.ts (1)
toBrunoScripts(32-64)packages/bruno-filestore/src/formats/yml/common/variables.ts (1)
toBrunoVariables(48-83)
packages/bruno-filestore/src/formats/yml/common/headers.ts (1)
packages/bruno-filestore/src/utils/index.ts (1)
uuid(9-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Agent
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (44)
packages/bruno-filestore/src/formats/yml/parseEnvironment.ts (2)
17-26: LGTM! Intentional security design for secret variables.Setting
valueto an empty string for secret variables is the correct approach—secret values should not be persisted in the Bruno format, only metadata is preserved.
33-33: Good practice: explicitsecret: falsefor non-secret variables.Explicitly setting
secret: falseimproves clarity and ensures consistent structure across all variable types.packages/bruno-filestore/src/formats/yml/stringifyEnvironment.ts (2)
18-27: LGTM! Correct secret variable serialization.Secret variables are properly serialized with only
nameandsecretmetadata—novaluefield is included. This mirrors the parsing logic inparseEnvironment.tsand maintains the security posture of not persisting secret values to disk.
6-6: Type signatures correctly updated for secret variable support.The function signature and return types properly reflect the union
(Variable | SecretVariable)[], ensuring type safety for the new secret variable handling.Also applies to: 11-11, 17-17
packages/bruno-filestore/src/formats/yml/utils.ts (2)
3-4: LGTM! Well-defined block keys for enhanced YAML readability.The constant clearly defines which top-level keys should have preceding blank lines, improving the structure of generated YAML files for the OpenCollection schema.
6-34: LGTM! Solid implementation of blank-line formatting.The post-processing logic correctly identifies top-level keys and adds blank lines before major blocks while avoiding duplicates. The implementation handles edge cases appropriately (empty lines, comments, lines without colons) and maintains the original YAML structure.
packages/bruno-filestore/package.json (1)
25-25: LGTM!The dependency version update from 0.1.0 to 0.2.0 enables the new type system used throughout this PR. The changes are well-coordinated across all affected files.
packages/bruno-filestore/src/formats/yml/items/parseScript.ts (1)
2-2: LGTM!The type update from
ScripttoScriptFilealigns with the new schema in @opencollection/types 0.2.0. The change is consistent with the corresponding update in stringifyScript.ts.Also applies to: 5-5
packages/bruno-filestore/src/formats/yml/stringifyCollection.ts (1)
3-3: LGTM!The type refinement from
HttpHeadertoHttpRequestHeaderimproves type precision and aligns with the updated return type oftoOpenCollectionHttpHeaders. This change is consistent with the header handling updates across the codebase.Also applies to: 131-131
packages/bruno-filestore/src/formats/yml/items/stringifyScript.ts (1)
2-2: LGTM!The type update to
ScriptFilemaintains round-trip consistency with parseScript.ts. The implementation correctly setstype: 'script'and populates the script content fromitem.fileContent.Also applies to: 7-9
packages/bruno-filestore/src/formats/yml/common/auth-oauth2.ts (1)
166-166: LGTM!The OAuth2 flow rename from
resource_owner_passwordtoresource_owner_password_credentialsnormalizes the terminology to match RFC 6749. Both forward and reverse mappings are updated consistently, maintaining proper round-trip conversion with Bruno's internalpasswordgrant type.Also applies to: 397-398
packages/bruno-filestore/src/formats/yml/items/parseGrpcRequest.ts (4)
31-34: LGTM!The introduction of local aliases (
info,grpc,runtime) for the nested structure is clean and improves readability. This aligns with the broader schema refactoring to separate metadata, protocol-specific data, and runtime configuration.
36-41: LGTM!The field mappings from the nested structure are well-implemented with proper optional chaining and sensible defaults. This ensures backward compatibility while adopting the new schema.
60-64: LGTM!The nested data access patterns for message, scripts, variables, and assertions are consistent and properly guarded with optional chaining. The fallback logic preserves existing behavior.
Also applies to: 68-89
100-102: LGTM!The item metadata extraction from
infowith appropriate defaults (seq: 1, name: 'Untitled Request', tags: []) ensures robustness when optional fields are missing.packages/bruno-filestore/src/formats/yml/items/parseGraphQLRequest.ts (3)
13-16: LGTM!The nested structure aliases are consistent with the pattern established in parseGrpcRequest.ts, maintaining uniformity across the codebase.
18-22: LGTM!The field mappings correctly source data from the nested
graphqlblock. The type assertion forGraphQLBodyon lines 32-33 is appropriate given the body mode check and aligns with the GraphQL request schema.Also applies to: 32-34
51-72: LGTM!The runtime data extraction (scripts, variables, assertions) and metadata handling maintain the same robust pattern with optional chaining and sensible defaults seen in other parse functions.
Also applies to: 83-85
packages/bruno-filestore/src/formats/yml/items/parseWebsocketRequest.ts (3)
18-21: LGTM!The nested structure pattern is consistently applied, using
websocketfor protocol-specific data alongsideinfoandruntimeblocks.
23-25: LGTM!The field mappings and message handling correctly source data from the nested
websocketblock. The type assertion forWebSocketMessageon line 45 is appropriate and the message data extraction with default type 'text' is well-implemented.Also applies to: 44-53
56-71: LGTM!The runtime data extraction and metadata handling maintain consistency with the patterns established in the GraphQL and gRPC parsers. The fallback values ensure robustness.
Also applies to: 97-99
packages/bruno-filestore/src/formats/yml/parseItem.ts (2)
14-24: LGTM on thegetItemTypehelper.Clean extraction logic that handles both the new nested
info.typestructure and the legacy root-leveltypeforScriptFile. The type guard pattern is appropriate.
35-49: Type assertions are acceptable given the validateditemType.The switch-case logic validates the type string before casting, so each
ascast is safe. Structure is consistent across request types.packages/bruno-filestore/src/formats/yml/items/stringifyGrpcRequest.ts (2)
18-32: Good restructuring to nested blocks.The
infoblock pattern with conditionalseqandtagsis clean and consistent with the new schema. Default handling fornameis appropriate.
90-124: Runtime block conditional attachment is well implemented.The
hasRuntimeflag pattern prevents emitting an emptyruntimeobject. Each field properly sets the flag when populated.packages/bruno-filestore/src/formats/yml/items/parseHttpRequest.ts (2)
14-46: Clean migration to nested structure extraction.Proper use of optional chaining with sensible defaults. The expanded body default structure aligns with the Bruno schema requirements.
77-92: ExtendedbrunoIteminitialization looks correct.New fields (
root,items,examples,filename,pathname) are initialized appropriately. Info block extraction forseq,name, andtagswith fallbacks is solid.packages/bruno-filestore/src/formats/yml/stringifyFolder.ts (2)
35-43: Folder info block aligns with the new schema structure.Consistent pattern with request info blocks. Default fallbacks for
nameandseqare appropriate.
82-88: Good docs handling with content type metadata.Wrapping docs content with
type: 'text/markdown'provides proper content type information for consumers.packages/bruno-filestore/src/formats/yml/common/headers.ts (2)
28-39: NewtoOpenCollectionResponseHeadershelper is well-structured.Response headers correctly omit the
disabledfield since response headers don't have enable/disable state. Good separation of concerns.
41-55: Robust enabled flag derivation.The
'disabled' in headercheck correctly handles the union type, defaulting toenabled: truewhen the property doesn't exist (response headers case).packages/bruno-filestore/src/formats/yml/items/stringifyHttpRequest.ts (4)
24-35: Info block construction is consistent with other request types.Proper handling of optional
seqandtagsfields. Name defaults to 'Untitled Request' when absent.
63-97: Runtime block pattern matches gRPC implementation.The
hasRuntimeflag approach is consistent across request types. Auth inclusion in runtime aligns with the schema changes.
176-179: Correct use oftoOpenCollectionResponseHeadersfor example responses.Response headers properly use the new helper that omits the
disabledfield.
99-132: Settings are always emitted, unlike runtime.The runtime block is conditionally assigned only when
hasRuntimeis true, but settings are always constructed with defaults and unconditionally assigned toocRequest.settings. This pattern is consistent across stringify functions (bothstringifyHttpRequest.tsandstringifyGraphQLRequest.tsfollow the same approach). This appears to be intentional—settings require explicit defaults even when user-provided values are absent, whereas the runtime block is optional.packages/bruno-filestore/src/formats/yml/items/stringifyWebsocketRequest.ts (4)
22-38: LGTM - Info block construction is clean.Proper fallback to 'Untitled Request' for empty names, and conditional inclusion of
seqandtagsfollows best practices for avoiding empty fields in the output.
40-69: LGTM - WebSocket block handles URL, headers, and message appropriately.Good conditional checks before assigning headers and message to avoid empty arrays/objects in output.
71-98: LGTM - Runtime block with conditional attachment.Using
hasRuntimeflag to only attach the runtime block when it has content is a good pattern for keeping output clean.
110-118: LGTM - Docs and error handling are appropriate.Docs are conditionally added, and the catch block properly logs and rethrows for upstream handling.
packages/bruno-filestore/src/formats/yml/items/stringifyGraphQLRequest.ts (5)
18-34: LGTM - Info block follows the same pattern as WebSocket.Consistent handling of name, type, seq, and tags with proper fallbacks.
36-74: LGTM - GraphQL block construction is well-structured.Proper conditional inclusion of headers, params, and body. The
hasBodyflag ensures we don't emit an empty body object.
76-110: LGTM - Runtime block pattern consistent with WebSocket.Variables, scripts, assertions, and auth all conditionally added with the
hasRuntimeguard.
147-156: LGTM - Docs and error handling.Consistent with WebSocket stringify.
36-40: Type compatibility is correct; no changes needed.
brunoRequest.methodis properly narrowed to a string by theisNonEmptyStringtype guard, and the ternary operator always returns a string (either the narrowed method or the'POST'literal). With strict TypeScript checking enabled in the project, any actual type incompatibility would be caught at build time. The code compiles and runs without errors.Likely an incorrect or invalid review comment.
* feat: changes to incorporate oc schema updates * chore: fixed oc types resolution issue
Changes to incorporate opencollection schema updates
Summary by CodeRabbit
New Features
Enhancements / Refactor
✏️ Tip: You can customize this high-level summary in your review settings.