feat: Github PR creation #30
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| if (!config) return 'deploy.yaml'; | ||
| const chains = objKeys(config).sort(); | ||
| export function getConfigsFilename(warpRouteId: string) { | ||
| const [_, label] = warpRouteId.split('/'); |
There was a problem hiding this comment.
i don't remember if it is exported from the registry package but there is a function that allows you to split the symbol and label from a warp route id
There was a problem hiding this comment.
there is indeed a fn in the registry but it is currently not exported
fixes [ENG-1824](https://linear.app/hyperlane-xyz/issue/ENG-1824/rate-limiting-avoid-spamming-of-endpoints) This PR features different ways to prevent the `create-pr` endpoint from being spammed, the process goes like: 1. In the UI, the temporary deployer wallet will sign a message that contains a `timestamp` and `warpRouteId`. 2. The returned `signature` will be sent to the endpoint along with the `address` and `message` 3. The backend service will verify the signature using viem's `verifySignature` and also check if the `timestamp` has expired (its currently set to 2 minutes) 4. A `keccak256` hash will be created using the `warpRouteId`, `deployConfig` and `warpConfig`, this will be used for the branch name, which will be in `warpRouteId-keccak256hash` format 5. This branch name will be verified using `octokit` so no other branches exists with the same name 6. If every check pass then the PR is created Rate limiting will be implemented at a later PR <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a clear message in the PR creation modal informing users that wallet signature is required for verification, with no on-chain transaction involved. * Introduced wallet-based signature verification for PR creation, enhancing security and authenticity. * Implemented deterministic branch naming to prevent duplicate PRs for the same configuration. * **Bug Fixes** * Prevented duplicate pull requests by checking for existing branches with the same configuration. * **Refactor** * Improved internal structure for PR creation and signature verification processes to enhance maintainability. * **Chores** * Added utility for sorting object keys to ensure consistent configuration handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
Warning Rate limit exceeded@Xaroz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new feature was added allowing users to create GitHub pull requests for Warp route deployments directly from the app. This includes new UI components, server-side API logic, config schemas, utility functions, and integration with the GitHub API. Several utility modules and validation helpers were also introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (WarpDeploymentSuccess)
participant Modal (CreateRegistryPrModal)
participant Hook (useCreateWarpRoutePR)
participant API (/api/create-pr)
participant GitHub
User->>UI (WarpDeploymentSuccess): Click "Create Registry PR"
UI->>Modal (CreateRegistryPrModal): Open modal
User->>Modal (CreateRegistryPrModal): Fill form & confirm
Modal->>Hook (useCreateWarpRoutePR): Trigger PR mutation
Hook->>API (/api/create-pr): POST PR data & signature
API->>GitHub: Create branch, commit files, open PR
GitHub-->>API: PR URL
API-->>Hook (useCreateWarpRoutePR): Respond with PR URL
Hook-->>Modal (CreateRegistryPrModal): Show PR link
Modal-->>User: Display PR link
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/features/deployment/utils.ts (1)
1-56: Consider reusing utils from the registry packageJust like I wouldn't reinvent the wheel when there's a perfectly good boulder to roll, you might want to use the existing utilities from the registry package for generating warp IDs and filenames.
src/features/deployment/github.ts (1)
72-98: Consider using WarpDeploySchema for validationLike checking if the bridge is sturdy before crossing, you might want to use the
WarpDeploySchemafrom the SDK to validate your deployment config. Keeps things consistent across the codebase.src/pages/api/create-pr.ts (1)
80-132: PR creation logic looks solid!Good to see the warpRouteId being used in the changeset message as suggested. The sequential file uploads and PR creation flow is clean and follows GitHub's API patterns well.
🧹 Nitpick comments (6)
src/utils/object.ts (1)
6-11: Shallow sort is grand, but type cast hides a tad of truthCasting the reduced
{}toTswallows type-safety if keys differ. If you fancy stricter smells, start with{} as Partial<T>andreturn acc as Tafter the loop. Not critical, just cleaner.src/utils/zod.ts (1)
21-29: Neat helper – consider explicit return typeDeclaring
validateStringToZodSchema<T>(...): T | nullmakes intent clearer to TS and readers alike.src/types/api.ts (1)
1-4: Consider making thesuccessflag required inApiErrorWell now, having an optional
successflag in your error type is like having a swamp that might or might not be murky - you want to be certain about these things. Making it required (success: false) would ensure clearer type discrimination between success and error responses.export interface ApiError { - success?: false; + success: false; error: string; }src/features/deployment/CreateRegistryPrModal.tsx (1)
56-56: Simplify with optional chainingSometimes the simple path through the swamp is the best one. You can make this check cleaner with optional chaining.
- {response && response.success ? ( + {response?.success ? (src/features/deployment/warp/WarpDeploymentSuccess.tsx (1)
24-211: Consider breaking down this component for better maintainabilityThis component's gotten bigger than my swamp hut! With all the modal handling, config operations, and PR creation logic, it might be time to extract some of these concerns into custom hooks or smaller components. Makes it easier to test and understand, you know?
src/features/deployment/github.ts (1)
115-129: Multi-protocol support needs implementationThe TODO comment's been sitting there like a log in the swamp. Since you're only supporting EthersV5 right now, users with other wallet types will hit that error faster than you can say "get out of my swamp!"
Would you like me to help implement support for other provider types or create an issue to track this work?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/images/icons/folder-code-icon.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
eslint.config.mjs(1 hunks)package.json(2 hunks)src/consts/config.server.ts(1 hunks)src/features/deployment/CoinGeckoConfirmationModal.tsx(1 hunks)src/features/deployment/CreateRegistryPrModal.tsx(1 hunks)src/features/deployment/DeploymentDetailsModal.tsx(3 hunks)src/features/deployment/github.ts(1 hunks)src/features/deployment/utils.ts(2 hunks)src/features/deployment/warp/WarpDeploymentSuccess.tsx(3 hunks)src/flows/LandingCard.tsx(1 hunks)src/libs/github.ts(1 hunks)src/pages/api/create-pr.ts(1 hunks)src/types/api.ts(1 hunks)src/types/createPr.ts(1 hunks)src/utils/api.ts(1 hunks)src/utils/object.ts(1 hunks)src/utils/string.ts(1 hunks)src/utils/zod.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
src/utils/object.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/features/deployment/DeploymentDetailsModal.tsx (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/features/deployment/utils.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
src/features/deployment/warp/WarpDeploymentSuccess.tsx (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/features/deployment/github.ts (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/pages/api/create-pr.ts (1)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
src/types/createPr.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
🧬 Code Graph Analysis (6)
src/utils/api.ts (1)
src/types/api.ts (1)
ApiResponseBody(11-11)
src/libs/github.ts (1)
src/consts/config.server.ts (2)
ServerConfigSchema(3-28)serverConfig(39-46)
src/features/deployment/DeploymentDetailsModal.tsx (1)
src/features/deployment/utils.ts (1)
sortWarpCoreConfig(37-56)
src/features/deployment/CreateRegistryPrModal.tsx (3)
src/components/text/A.tsx (1)
A(5-7)src/utils/string.ts (1)
normalizeEmptyStrings(1-8)src/utils/zod.ts (1)
zodErrorToFormikErrors(10-19)
src/features/deployment/warp/WarpDeploymentSuccess.tsx (6)
src/features/deployment/hooks.ts (3)
useWarpDeploymentConfig(14-18)useDeploymentHistory(26-39)useLatestDeployment(41-44)src/features/deployment/github.ts (1)
useCreateWarpRoutePR(29-70)src/features/deployment/warp/utils.ts (1)
isSyntheticTokenType(23-25)src/features/deployment/utils.ts (3)
tryCopyConfig(6-12)sortWarpCoreConfig(37-56)getConfigsFilename(28-35)src/components/text/A.tsx (1)
A(5-7)src/features/deployment/CreateRegistryPrModal.tsx (1)
CreateRegistryPrModal(13-79)
src/features/deployment/github.ts (4)
src/features/deployment/hooks.ts (1)
useLatestDeployment(41-44)src/features/deployment/warp/utils.ts (1)
isSyntheticTokenType(23-25)src/features/deployment/utils.ts (2)
getConfigsFilename(28-35)sortWarpCoreConfig(37-56)src/utils/string.ts (1)
normalizeEmptyStrings(1-8)
🪛 Biome (1.9.4)
src/features/deployment/CreateRegistryPrModal.tsx
[error] 56-56: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ 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). (1)
- GitHub Check: install
🔇 Additional comments (18)
eslint.config.mjs (1)
58-60: Key looks good without the fancy dressDropping the quotes around
camelcasestill gives ESLint the same tasty key, so no worries in the swamp. Carry on.package.json (2)
27-27: New Octokit dependency – double-check runtime & bundle impact 🪓
@octokit/rest@^22shifted to pure ESM; if any serverless edge or older Node runtimes are involved, imports may break. Give it a quick spin locally and peek at the bundle to be safe, lad.
48-48:human-idaddition looks fineTiny util, no complaints from the swamp.
src/flows/LandingCard.tsx (1)
42-44: Jumping straight to success page – intentional?Switching from
WarpFormtoWarpSuccessskips configuration. If that’s by design, all good; if accidental, users will hit an empty swamp. Give it a quick whirl.src/features/deployment/CoinGeckoConfirmationModal.tsx (1)
18-24: Well, this looks good to me!The addition of the
closeprop is clean and follows the expected pattern. Proper typing and consistent with how other modal components handle closing.src/utils/api.ts (1)
4-12: This utility function is solid!Nice clean implementation for standardizing API responses. The conditional logic properly handles both cases - sending JSON when there's a body, or just ending with status code when there isn't. The generic typing ensures type safety throughout.
src/utils/string.ts (1)
1-8: This utility does what it says on the tin!The logic for normalizing empty strings to undefined is sound. The
Object.entries/fromEntriespattern works well here, and thetrim()check catches whitespace-only strings nicely. The type assertion is reasonable since you're preserving the object structure.src/features/deployment/DeploymentDetailsModal.tsx (2)
10-10: Good move extracting this into a utility!Importing the
sortWarpCoreConfigutility promotes code reuse and consistency across deployment components.
27-27: Clean application of the sorting utility.The sorted result ensures consistent display of deployment data, which is especially important for the CollapsibleData component below.
src/libs/github.ts (1)
7-16: This caching approach makes sense for server-side usage.The logic is sound - validate the config once, cache the authenticated client, and reuse it. The
safeParsevalidation ensures the client is only created with valid configuration.One thing to keep in mind: the cached client won't refresh if server config changes during runtime, but that's probably fine for most deployment scenarios where config is static.
src/types/createPr.ts (4)
4-20: Well-crafted GitHub identity validation!This regex pattern properly enforces GitHub's username rules - no leading/trailing hyphens, no double hyphens, and the 39-character limit. Like layers in an onion, every detail matters here.
22-31: Solid file schema definition!The validation ensures both path and content are non-empty, and the descriptive messages help with debugging. Clean and simple, just the way it should be.
32-47: Clean schema composition!Nice use of zod's merge() to combine schemas. The warpRouteId validation and type inference are spot on.
48-60: Proper signature verification structure!All the essential fields for signature verification are here with appropriate validation. The separation of concerns between PR data and verification is well done.
src/pages/api/create-pr.ts (4)
29-51: Good security practices in the handler setup!Smart move with the environment-specific error messages - keeps internal details hidden in production while helping developers debug locally.
52-79: Well-structured validation flow!The sequential validation of body, signature, and branch name keeps things organized. The duplicate PR prevention is a nice touch.
134-169: Thorough request validation!The nested validation of deploy and warp configs ensures data integrity at multiple levels. Like peeling back the layers, each validation step adds confidence.
216-276: Well-implemented utility functions!The changeset generation is clean, and I see the double sorting in getBranchName is intentional for deterministic hashing - makes sense for consistent branch naming. The branch existence check handles errors gracefully.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/api/create-pr.ts (1)
182-185: Heads up - there's a copy-paste situation here.Line 184 is checking the address again instead of validating the signature format. Based on the past review comments, I see this was mentioned before and you indicated it was intentional, but it still looks odd.
If this extra address check is really needed, maybe add a comment explaining why? Otherwise it just looks like a mistake waiting to happen.
🧹 Nitpick comments (2)
src/components/toast/useToastError.tsx (1)
10-10: Good improvement, but let's make it even more robust.Nice touch extracting the nested error message first - makes sense for the new PR creation flow where errors might come wrapped. The fallback logic is solid.
Consider making it a bit more defensive though:
-const errorMsg = error?.error?.message || errorToString(error, errorLength); +const errorMsg = error?.error?.message || error?.message || errorToString(error, errorLength);This way we catch both deeply nested errors and simple message properties before falling back to the full error string conversion.
src/pages/api/create-pr.ts (1)
74-78: Good branch collision prevention.Smart check to avoid creating duplicate PRs with the same config. The error message could be a bit more helpful though - maybe suggest what the user should do next?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/toast/useToastError.tsx(1 hunks)src/features/deployment/github.ts(1 hunks)src/pages/api/create-pr.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/deployment/github.ts
🧰 Additional context used
🧠 Learnings (1)
src/pages/api/create-pr.ts (2)
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/pages/api/create-pr.ts:228-232
Timestamp: 2025-07-03T15:59:33.410Z
Learning: In src/pages/api/create-pr.ts, the getBranchName function uses double sorting intentionally: sortWarpCoreConfig sorts by chainName and internal connections by chainName, while the outer sortObjByKeys sorts by keys in case options are provided. Both are needed for deterministic hashing to ensure consistent branch naming.
Learnt from: Xaroz
PR: hyperlane-xyz/hyperlane-deploy-app#31
File: src/utils/object.ts:3-12
Timestamp: 2025-07-03T15:57:43.709Z
Learning: In the sortObjByKeys function in src/utils/object.ts, the user confirmed that for their PR's purpose (creating deterministic branch names via hashing), they only need to sort top-level keys and do not require recursive sorting of nested objects.
⏰ 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). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (9)
src/pages/api/create-pr.ts (9)
32-32: Solid HTTP method guard.Clean and straightforward - catches the wrong method early and returns the right status code. No complaints here.
41-49: Smart environment-aware error handling.Good thinking here - showing detailed error messages in dev but keeping things vague in production. Security-conscious approach that'll help with debugging without leaking internals.
53-58: Well-structured validation flow.Nice pattern here - validate the request body first, then the signature. Early returns keep the happy path clean and readable.
140-167: Solid request validation structure.Clean separation of concerns - parse the main body, then validate each config file separately. The type safety with the return union is a nice touch.
198-212: Thoughtful timestamp validation logic.Good security practice here - preventing replay attacks with the timestamp check. The 2-minute window seems reasonable for user interactions.
The string parsing approach is straightforward, though you might want to be more specific about the expected message format in case it changes later.
217-229: Neat changeset generation.Clean implementation following the changesets pattern. The human-readable ID generation is a nice touch for branch names.
236-237: Smart deterministic hashing approach.From the retrieved learnings, I understand this double sorting is intentional for consistent branch naming. Good thinking - ensures the same config always generates the same branch name regardless of key order.
242-252: Robust hash generation with error handling.The keccak256 approach for branch naming is solid - gives you consistent, collision-resistant identifiers. Good defensive programming with the try-catch too.
255-277: Clean branch existence checking.Smart use of the GitHub API's 404 behavior to check if a branch exists. The error handling distinguishes between "branch doesn't exist" (good) and "something went wrong" (bad).
| try { | ||
| // Get latest SHA of base branch in fork | ||
| const { data: refData } = await octokit.git.getRef({ | ||
| owner: githubForkOwner, | ||
| repo: githubRepoName, | ||
| ref: `heads/${githubBaseBranch}`, | ||
| }); | ||
|
|
||
| const latestCommitSha = refData.object.sha; | ||
|
|
||
| // Create new branch | ||
| await octokit.git.createRef({ | ||
| owner: githubForkOwner, | ||
| repo: githubRepoName, | ||
| ref: `refs/heads/${branchName}`, | ||
| sha: latestCommitSha, | ||
| }); | ||
|
|
||
| const changesetFile = writeChangeset(`Add ${warpRouteId} warp route deploy artifacts`); | ||
|
|
||
| // Upload files to the new branch | ||
| for (const file of [deployConfig, warpConfig, changesetFile]) { | ||
| await octokit.repos.createOrUpdateFileContents({ | ||
| owner: githubForkOwner, | ||
| repo: githubRepoName, | ||
| path: file.path, | ||
| message: `feat: add ${file.path}`, | ||
| content: Buffer.from(file.content).toString('base64'), | ||
| branch: branchName, | ||
| }); | ||
| } | ||
|
|
||
| const githubInfo = [username && `by ${username}`, organization && `from ${organization}`] | ||
| .filter(Boolean) | ||
| .join(' '); | ||
|
|
||
| // Create a PR from the fork branch to upstream main | ||
| const { data: pr } = await octokit.pulls.create({ | ||
| owner: githubUpstreamOwner, | ||
| repo: githubRepoName, | ||
| title: `feat: add ${warpRouteId} warp route deploy artifacts`, | ||
| head: `${githubForkOwner}:${branchName}`, | ||
| base: githubBaseBranch, | ||
| body: `This PR was created from the deploy app to add ${warpRouteId} warp route deploy artifacts.${ | ||
| githubInfo ? `\n\nThis config was provided ${githubInfo}.` : '' | ||
| }`, | ||
| }); | ||
|
|
||
| return sendJsonResponse(res, 200, { data: { prUrl: pr.html_url }, success: true }); | ||
| } catch (err: any) { | ||
| return sendJsonResponse(res, 500, { error: err.message }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Comprehensive PR creation flow, but watch those errors.
The GitHub API workflow looks solid - create branch, upload files, create PR. Good use of the changeset generation too.
However, that catch-all error handler on line 129 might be swallowing important details. Consider logging the full error before returning just the message, especially since some GitHub API errors have useful context in other properties.
} catch (err: any) {
+ console.error('PR creation failed:', err);
return sendJsonResponse(res, 500, { error: err.message });
}🤖 Prompt for AI Agents
In src/pages/api/create-pr.ts around lines 79 to 131, the catch block at line
129 only returns the error message, which may omit important error details from
the GitHub API response. Modify the catch block to log the full error object
before returning the error message in the response, ensuring that additional
context like error status or documentation URLs is captured for debugging.
This PR allows the deploy app to offer auto creation of pull request to the registry using
octokitpackage as a base.Fixes ENG-1671
TODO:
For the Eslint rules instead of using ESLint which is not allowed in the browser, I sorted the tokens by chain name and the connections by their chain name too, which would match the rules we have in the registry plus yaml takes care of sorting the config by their keys
Summary by CodeRabbit
New Features
Bug Fixes
Chores