-
Notifications
You must be signed in to change notification settings - Fork 0
build: move scripts to the workflows folder #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub Actions
participant Stage as Staging Deploy API
participant Prod as Production Deploy API
rect rgba(200,220,255,0.2)
note over Dev,GH: Staging deployment
Dev->>GH: Push to main or workflow_dispatch
GH->>Stage: GET /api/deploy/${{secret EASYPANEL_DEPLOY_KEY_STAGING }}
Stage-->>GH: 200 / Error
GH-->>Dev: Job result
end
rect rgba(200,255,200,0.2)
note over Dev,GH: Production deployment
Dev->>GH: Release published or workflow_dispatch
GH->>Prod: GET /api/deploy/${{secret EASYPANEL_DEPLOY_KEY_PRODUCTION }}
Prod-->>GH: 200 / Error
GH-->>Dev: Job result
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.env.example (1)
1-4
: Alphabetize keys for consistency with dotenv linters.Keeps diffs clean and satisfies tooling.
Apply this diff:
-NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID= -RECIPIENT_WALLET= -NEXT_PUBLIC_REQUEST_API_URL= +NEXT_PUBLIC_REQUEST_API_URL= +NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID= +RECIPIENT_WALLET= NEXT_PUBLIC_REQUEST_API_CLIENT_ID=.github/workflows/deploy-to-production.yml (1)
1-15
: Harden the deploy step: fail-fast, timeouts, retries, and concurrency guard.Prevents silent greens, flakes, and overlapping prod deploys.
Apply this diff:
name: Deploy to production @@ on: workflow_dispatch: release: types: [published] jobs: deploy: runs-on: ubuntu-latest + timeout-minutes: 15 + concurrency: + group: production-deploy + cancel-in-progress: true steps: - name: Webhook request - run: | - curl -X GET https://prod.request.network/api/deploy/${{ secrets.EASYPANEL_DEPLOY_KEY_PRODUCTION }} + run: | + set -euo pipefail + if [ -z "${{ secrets.EASYPANEL_DEPLOY_KEY_PRODUCTION }}" ]; then + echo "Secret EASYPANEL_DEPLOY_KEY_PRODUCTION is not set"; exit 1 + fi + curl -fsS --max-time 30 --retry 3 --retry-delay 2 --retry-connrefused \ + "https://prod.request.network/api/deploy/${{ secrets.EASYPANEL_DEPLOY_KEY_PRODUCTION }}".github/workflows/deploy-to-staging.yml (1)
1-15
: Add resiliency and safety for staging deploys.Use retries/timeout, fail on errors, and avoid overlapping runs on busy branches.
Apply this diff:
name: Deploy to staging @@ on: workflow_dispatch: push: branches: [main] jobs: deploy: runs-on: ubuntu-latest + timeout-minutes: 10 + concurrency: + group: staging-deploy + cancel-in-progress: true steps: - name: Webhook request - run: | - curl -X GET https://stage.request.network/api/deploy/${{ secrets.EASYPANEL_DEPLOY_KEY_STAGING }} + run: | + set -euo pipefail + if [ -z "${{ secrets.EASYPANEL_DEPLOY_KEY_STAGING }}" ]; then + echo "Secret EASYPANEL_DEPLOY_KEY_STAGING is not set"; exit 1 + fi + curl -fsS --max-time 20 --retry 3 --retry-delay 2 --retry-connrefused \ + "https://stage.request.network/api/deploy/${{ secrets.EASYPANEL_DEPLOY_KEY_STAGING }}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example
(1 hunks).github/workflows/deploy-to-production.yml
(1 hunks).github/workflows/deploy-to-staging.yml
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T15:54:47.528Z
Learnt from: bassgeta
PR: RequestNetwork/ui-registry#1
File: .github/deploy-to-production.yml:14-14
Timestamp: 2025-09-17T15:54:47.528Z
Learning: The RequestNetwork organization uses deployment workflows with secrets embedded in URLs (e.g., curl -X GET https://prod.request.network/api/deploy/${{ secrets.EASYPANEL_DEPLOY_KEY_PRODUCTION }}) consistently across all repositories. This is an accepted pattern for their EasyPanel deployment setup and should not be flagged as a security concern in future reviews.
Applied to files:
.github/workflows/deploy-to-production.yml
.github/workflows/deploy-to-staging.yml
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 3-3: [UnorderedKey] The NEXT_PUBLIC_NEXT_PUBLIC_REQUEST_API_CLIENT_ID key should go before the NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_REQUEST_API_URL key should go before the NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID key
(UnorderedKey)
🔇 Additional comments (3)
.github/workflows/deploy-to-production.yml (1)
1-15
: LGTM overall; pattern matches org standard.Per prior learnings for this org, embedding the EasyPanel deploy key in the webhook path is an accepted pattern here.
.github/workflows/deploy-to-staging.yml (1)
1-15
: LGTM; simple and fits the repo’s deployment approach.Triggers on main and manual dispatch as expected.
.env.example (1)
2-2
: Confirm intended exposure of RECIPIENT_WALLET (client vs server)..env.example:
RECIPIENT_WALLET=
- If read in browser/client code, rename to NEXT_PUBLIC_RECIPIENT_WALLET and update usages; if server-only, keep as-is.
- Verify by searching the repo for usages: rg -n -S "RECIPIENT_WALLET|process.env.RECIPIENT_WALLET" -g '!/node_modules/' -g '!/dist/' and confirm whether matches are in client-side files (pages/components) or server-only code.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.env.example (1)
3-3
: Typo resolved for REQUEST API client ID.The duplicate NEXT_PUBLIC prefix from the earlier commit is fixed. Thanks for addressing it.
🧹 Nitpick comments (2)
.env.example (2)
1-4
: Reorder keys to satisfy dotenv-linter (alphabetical).Purely cosmetic, but keeps CI/noise down.
-NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID= -RECIPIENT_WALLET= -NEXT_PUBLIC_REQUEST_API_CLIENT_ID= -NEXT_PUBLIC_REQUEST_API_URL= +NEXT_PUBLIC_REQUEST_API_CLIENT_ID= +NEXT_PUBLIC_REQUEST_API_URL= +NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID= +RECIPIENT_WALLET=
2-2
: Add usage hints/sample formats for faster onboarding.Suggest brief comments or sample values (non‑secret) to reduce setup friction.
-RECIPIENT_WALLET= +RECIPIENT_WALLET= +# e.g. 0x0000000000000000000000000000000000000000Optionally add short comments for the others:
- NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID: “WalletConnect Cloud project ID”
- NEXT_PUBLIC_REQUEST_API_URL: “Base URL, e.g. https://api.request.network”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env.example
(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 3-3: [UnorderedKey] The NEXT_PUBLIC_REQUEST_API_CLIENT_ID key should go before the NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_REQUEST_API_URL key should go before the NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID key
(UnorderedKey)
🔇 Additional comments (2)
.env.example (2)
1-4
: Confirm public exposure of these values is intended.Vars prefixed with NEXT_PUBLIC are shipped to the browser in Next.js. Verify these are non‑sensitive.
1-5
: Avoid putting secrets in webhook URLs in the new workflows.AI summary shows deployment workflows using secrets in the URL path. Secrets in URLs can leak via server logs/caches and proxies; prefer headers or POST body.
Suggested change in the workflow curl step (illustrative):
- run: curl -sSf -X GET "https://prod.request.network/api/deploy/${{ secrets.EASYPANEL_DEPLOY_KEY_PRODUCTION }}" + env: + DEPLOY_KEY: ${{ secrets.EASYPANEL_DEPLOY_KEY_PRODUCTION }} + run: | + curl -sSf -X POST \ + -H "Authorization: Bearer ${DEPLOY_KEY}" \ + -H "Content-Type: application/json" \ + --data '{}' \ + https://prod.request.network/api/deployAlso consider:
- Use POST (idempotency via an id if needed), not GET, for actions.
- Add --fail-with-body and timeouts.
- Ensure the endpoint accepts header‑based auth before merging.
⛔ Skipped due to learnings
Learnt from: bassgeta PR: RequestNetwork/ui-registry#1 File: .github/deploy-to-production.yml:14-14 Timestamp: 2025-09-17T15:54:47.528Z Learning: The RequestNetwork organization uses deployment workflows with secrets embedded in URLs (e.g., curl -X GET https://prod.request.network/api/deploy/${{ secrets.EASYPANEL_DEPLOY_KEY_PRODUCTION }}) consistently across all repositories. This is an accepted pattern for their EasyPanel deployment setup and should not be flagged as a security concern in future reviews.
The workflow files were copied instead of moved, creating duplicates. GitHub Actions only recognizes workflows in .github/workflows/ directory. This commit completes the migration by removing the old duplicate files.
Summary by CodeRabbit
Documentation
Chores
Notes