Repository navigation
feat: improve bot presence and harden deploy smoke checks - #122
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for regal-bunny-0c8efe ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
dependencies
📝 WalkthroughWalkthroughAdds an interval-based Discord presence rotation, enhances CI/CD deploy workflow with webhook retry logic and a post-rollout OAuth health smoke check, tightens Node engine constraint, runs Prisma generate before builds, and updates docs/changelog and tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Action (deploy)
participant Retry as call_webhook_with_retry
participant App as Deployed App (webhook endpoint)
participant Smoke as Auth Config Endpoint (/api/health/auth-config)
GH->>Retry: invoke webhook(s)
Retry->>App: POST webhook (may retry on 5xx/0, up to 5 attempts)
App-->>Retry: HTTP 2xx / 4xx / 5xx
Retry-->>GH: final webhook result (success/failure)
alt webhook success
GH->>Smoke: poll /api/health/auth-config (Node check, up to 18 attempts)
Smoke-->>GH: JSON { status, warnings, legacyDomain? }
alt status ok & no warnings & no legacyDomain
GH-->>GH: finish deploy success
else
GH-->>GH: fail deploy with detailed error
end
else
GH-->>GH: fail deploy (webhook errors)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Size Change: 0 B Total Size: 291 kB ℹ️ View Unchanged
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/bot/tests/handlers/clientHandler/presence.test.ts (1)
50-74: Consider adding a test forstartPresenceRotationtimer behavior.While the underlying functions are well-tested, there's no direct test for
startPresenceRotationwhich manages the interval timer. A test verifying that it calls the rotation immediately and returns a working cleanup function would improve coverage.Would you like me to draft an additional test case for
startPresenceRotation?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/tests/handlers/clientHandler/presence.test.ts` around lines 50 - 74, Add a unit test for startPresenceRotation that verifies it invokes the rotation immediately and returns a cleanup function that stops the interval; specifically, call startPresenceRotation (using a mock client with user.setPresence and a small interval), assert setPresence was called once synchronously, then call the returned cleanup function and advance timers (or wait) to confirm no further setPresence calls occur; reference the startPresenceRotation function and the returned cleanup function to locate where to add the test and reuse mocks similar to setPresenceActivity and nextPresenceIndex to validate behavior.README.md (1)
167-167: Minor: Use hyphen for compound adjective.Per static analysis, "Prisma generated" should be "Prisma-generated" when used as a compound adjective before a noun.
✏️ Suggested fix
-Vercel note: `vercel.json` runs `npm run db:generate` before `build:shared` and `build:frontend` to ensure Prisma generated client files are present during cloud builds. +Vercel note: `vercel.json` runs `npm run db:generate` before `build:shared` and `build:frontend` to ensure Prisma-generated client files are present during cloud builds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 167, The phrase "Prisma generated client files" should use a hyphen as a compound adjective: update the README text that currently reads "Prisma generated client files" (in the Vercel note sentence about vercel.json running `npm run db:generate` before builds) to "Prisma-generated client files" so the compound adjective is correctly hyphenated.packages/bot/src/handlers/clientHandler/presence.ts (2)
52-54: Add debug logging whenclient.useris unavailable.When
client.useris null (e.g., during initialization or after disconnect), the function silently returns. Adding a debug log would help with troubleshooting presence issues.Suggested fix
+import { debugLog } from '@lukbot/shared/utils' + // ... export const setPresenceActivity = ( client: CustomClient, index: number, ): number => { if (!client.user) { + debugLog('presence', 'Skipping presence update: client.user not available') return index }As per coding guidelines: "Use logging utilities
errorLoganddebugLogfrom@lukbot/shared/utilsfor logging throughout the bot".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/clientHandler/presence.ts` around lines 52 - 54, The function early-returns when client.user is falsy but provides no visibility; import and use the debugLog utility from `@lukbot/shared/utils` and add a debugLog call just before the return in the client.user check (referencing client.user and the local variable index) that logs a short contextual message and the current client state so presence initialization/disconnect conditions can be traced; ensure you follow existing logging conventions and keep the message concise.
4-7: Type alias should useT{Name}naming convention.Per coding guidelines, type aliases in TypeScript should use the
T{Name}naming convention.Suggested fix
-type PresenceActivity = { +type TPresenceActivity = { type: ActivityType name: string }Then update references on lines 35 and elsewhere to use
TPresenceActivity.As per coding guidelines: "Use
T{Name}naming convention for type aliases and utility types in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/clientHandler/presence.ts` around lines 4 - 7, Rename the type alias PresenceActivity to follow the T{Name} convention (TPresenceActivity) and update all usages to reference TPresenceActivity (for example any function parameters, variable annotations, or other type references that currently use PresenceActivity such as the occurrences around the presence handling logic and the reference noted on line 35). Ensure the renamed type keeps the same shape (properties type: ActivityType, name: string) and update imports/exports if this alias is exported or imported elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deploy.yml:
- Around line 48-72: The call_webhook_with_retry function currently treats
persistent 5xx or network errors as success because after exhausting retries it
echoes the response and returns 0; change the control flow so that when
call_webhook_with_retry (the function) finishes all 5 attempts and the last HTTP
code is a 5xx or "000", it returns a non-zero exit code (e.g., return 1) to
indicate failure instead of returning 0—either by returning failure inside the
branch when attempt == 5 or by adding a final failure return after the retry
loop while preserving the existing successful-path behavior for 2xx responses
and the existing echo of the response body.
---
Nitpick comments:
In `@packages/bot/src/handlers/clientHandler/presence.ts`:
- Around line 52-54: The function early-returns when client.user is falsy but
provides no visibility; import and use the debugLog utility from
`@lukbot/shared/utils` and add a debugLog call just before the return in the
client.user check (referencing client.user and the local variable index) that
logs a short contextual message and the current client state so presence
initialization/disconnect conditions can be traced; ensure you follow existing
logging conventions and keep the message concise.
- Around line 4-7: Rename the type alias PresenceActivity to follow the T{Name}
convention (TPresenceActivity) and update all usages to reference
TPresenceActivity (for example any function parameters, variable annotations, or
other type references that currently use PresenceActivity such as the
occurrences around the presence handling logic and the reference noted on line
35). Ensure the renamed type keeps the same shape (properties type:
ActivityType, name: string) and update imports/exports if this alias is exported
or imported elsewhere.
In `@packages/bot/tests/handlers/clientHandler/presence.test.ts`:
- Around line 50-74: Add a unit test for startPresenceRotation that verifies it
invokes the rotation immediately and returns a cleanup function that stops the
interval; specifically, call startPresenceRotation (using a mock client with
user.setPresence and a small interval), assert setPresence was called once
synchronously, then call the returned cleanup function and advance timers (or
wait) to confirm no further setPresence calls occur; reference the
startPresenceRotation function and the returned cleanup function to locate where
to add the test and reuse mocks similar to setPresenceActivity and
nextPresenceIndex to validate behavior.
In `@README.md`:
- Line 167: The phrase "Prisma generated client files" should use a hyphen as a
compound adjective: update the README text that currently reads "Prisma
generated client files" (in the Vercel note sentence about vercel.json running
`npm run db:generate` before builds) to "Prisma-generated client files" so the
compound adjective is correctly hyphenated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc242a16-1c5b-41ce-9049-8266c9e775a7
📒 Files selected for processing (9)
.github/workflows/deploy.ymlCHANGELOG.mdREADME.mddocs/CLOUDFLARE_TUNNEL_SETUP.mdpackage.jsonpackages/bot/src/handlers/clientHandler/presence.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/tests/handlers/clientHandler/presence.test.tsvercel.json
📜 Review details
⏰ 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: Quality Gates
- GitHub Check: Cursor Bugbot
🧰 Additional context used
📓 Path-based instructions (25)
{CHANGELOG.md,docs/**/*}
📄 CodeRabbit inference engine (.cursor/rules/lukbot-project.mdc)
Update CHANGELOG.md and relevant docs/ files when behavior or setup changes
Files:
docs/CLOUDFLARE_TUNNEL_SETUP.mdCHANGELOG.md
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validatorsDo not hardcode secrets, IP addresses, or ports; use
.envconfiguration and document required variables indocs/
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.
Files:
packages/bot/tests/handlers/clientHandler/presence.test.ts
packages/bot/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lukbot-discord-bot.mdc)
packages/bot/**/*.{ts,tsx}: Use logging utilitieserrorLoganddebugLogfrom@lukbot/shared/utilsfor logging throughout the bot
Use embed, reply, and error utilities from@lukbot/shared(embeds, interactionReply, errorSanitizer) instead of implementing custom implementations
Use DatabaseService and Redis client services from@lukbot/sharedfor database and cache access; do not instantiate Prisma or Redis directly in the bot package
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
packages/bot/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.cursor/rules/lukbot-project.mdc)
Use Discord.js and Discord Player for bot implementation
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.cursor/rules/lukbot-project.mdc)
**/*.{ts,tsx,js,jsx,mjs}: Require Node.js ≥22 and use ESM-only modules
Use env files (.env, .env.example) for secrets, ports, and hosts; never hardcode these values
Avoid redundant or decorative AI comments; code should be self-explanatory. Comment only when logic is non-obvious; prefer refactoring and short documentation over long comments
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
{packages/*/tests/**/*.{ts,tsx,js,jsx},tests/**/*.{ts,tsx,js,jsx}}
📄 CodeRabbit inference engine (.cursor/rules/lukbot-project.mdc)
Add or adjust unit/integration tests when changing behavior; follow existing patterns in packages/*/tests and root tests/
Files:
packages/bot/tests/handlers/clientHandler/presence.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries
Files:
packages/bot/tests/handlers/clientHandler/presence.test.ts
packages/bot/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Apply
.cursor/rules/lukbot-discord-bot.mdcfor Discord bot commands and player functionality
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tspackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details
Files:
packages/bot/tests/handlers/clientHandler/presence.test.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid redundant or decorative AI comments; code should be clear from names and structure; comment only when logic is non-obvious
Files:
packages/bot/tests/handlers/clientHandler/presence.test.tsvercel.jsonpackages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.tspackage.json
{CHANGELOG.md,README.md}
📄 CodeRabbit inference engine (.cursor/rules/agent-rules.mdc)
ALWAYS update CHANGELOG.md and README.md as changes are made.
Files:
CHANGELOG.mdREADME.md
CHANGELOG.md
📄 CodeRabbit inference engine (.cursor/rules/templates-examples.mdc)
CHANGELOG.md must be updated with all changes in pull requests
Files:
CHANGELOG.md
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
@lukbot/sharedfor database, Redis, logging, and embed utilities
Files:
packages/bot/src/handlers/clientHandler/service.tspackages/bot/src/handlers/clientHandler/presence.ts
**/{.github/workflows,}/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/ci-cd.mdc)
**/{.github/workflows,}/*.{yml,yaml}: CI pipeline must include setup step (node install, environment)
CI pipeline must include lint step (TypeScript typecheck + linter)
CI pipeline must include build step (production build)
CI pipeline must include test step (unit + integration) with coverage report
CI pipeline must include quality step (static analysis, vulnerability scan)
Files:
.github/workflows/deploy.yml
**/{.github/workflows,dependabot.yml}/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/ci-cd.mdc)
Configure dependency update bot with PR templates and tests (recommended)
Files:
.github/workflows/deploy.yml
**/.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/ci-cd.mdc)
**/.github/workflows/*.{yml,yaml}: Configure SAST / secrets scan on PRs (recommended)
Publish artifacts only from protected pipeline steps
Files:
.github/workflows/deploy.yml
{jest.config.*,*.coverage.*,.nycrc*,nyc.config.*,coveragerc,.github/workflows/*.yml,.github/workflows/*.yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
Minimum recommended coverage threshold: 85% (raise per project risk)
Files:
.github/workflows/deploy.yml
{.github/workflows/*.{yml,yaml},*.github/workflows/*.{yml,yaml},.gitlab-ci.yml,.circleci/config.yml,bitbucket-pipelines.yml}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
CI must run in order: lint → build → test → quality checks
Files:
.github/workflows/deploy.yml
{LICENSE,COPYING,COPYING.md,LICENSE.md,license.txt,**/package.json,**/requirements.txt,**/Cargo.toml,**/pom.xml,**/composer.json}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Respect OSS licenses for third-party code and declare license in repo
Files:
package.json
{webpack.config.{js,ts},vite.config.{js,ts},esbuild.config.{js,ts},package.json,.bundlebudgetrc,.bundlebudgetrc.json}
📄 CodeRabbit inference engine (.cursor/rules/performance-observability.mdc)
Add performance budgets for frontend bundles
Files:
package.json
README.md
📄 CodeRabbit inference engine (.cursor/rules/templates-examples.mdc)
README.md must be updated if behavior changed
Files:
README.md
🪛 LanguageTool
README.md
[grammar] ~167-~167: Use a hyphen to join words.
Context: ...dandbuild:frontend` to ensure Prisma generated client files are present durin...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (12)
package.json (1)
58-60: LGTM - Reasonable version pinning for stability.Narrowing the Node engine from
>=22.xto22.xis a sensible approach to prevent unexpected breaking changes from future major versions in CI and Vercel builds, as noted in the changelog.vercel.json (1)
2-2: LGTM - Correct build sequence for Prisma client generation.Prefixing with
db:generateensures the Prisma client is available before the dependent builds, which is necessary for cloud builds where the generated client may not exist.docs/CLOUDFLARE_TUNNEL_SETUP.md (1)
198-198: LGTM - Documentation correction for domain migration.The section title now correctly reflects the migration from the legacy
nexusdomain tolucky, which aligns with the OAuth domain validation changes elsewhere in this PR..github/workflows/deploy.yml (1)
94-146: LGTM - Well-structured OAuth smoke check with proper error handling.The smoke check implementation is solid:
- Uses safe environment variable passing to Node.js for JSON parsing
- Validates critical OAuth configuration states (status, legacy domain, warnings)
- Includes reasonable retry logic (18 attempts × 10s ≈ 3 min timeout for service readiness)
- Fails explicitly with descriptive error messages
README.md (1)
36-105: LGTM - Comprehensive documentation updates.The README accurately documents the new features including:
- Updated test counts and coverage information
- Brand colors and design system tokens
- Domain-based dashboard navigation structure
- Presence rotation and OAuth health check features
CHANGELOG.md (1)
8-106: LGTM - Comprehensive changelog entries.The changelog thoroughly documents all PR changes including:
- Bot presence rotation module
- Deploy workflow retry logic and OAuth smoke gate
- Frontend route metadata and UI improvements
- OAuth flow fixes and health endpoint additions
The entries follow the Keep a Changelog format appropriately.
packages/bot/src/handlers/clientHandler/service.ts (2)
13-15: LGTM - Clean presence rotation lifecycle management.The module-level
stopPresenceRotationvariable appropriately manages the rotation cleanup function. This pattern is acceptable for singleton lifecycle management within a module.
63-64: LGTM - Correct rotation restart on reconnect.The implementation properly handles the
readyevent by stopping any existing rotation before starting a new one. This prevents multiple concurrent intervals if the bot reconnects.packages/bot/tests/handlers/clientHandler/presence.test.ts (1)
1-74: LGTM - Good test coverage for presence rotation utilities.The tests effectively verify:
- Branded activity generation with server/member counts
- Safe member count aggregation handling edge cases (0, undefined)
- Presence application with correct activity shape and index rotation
The behavior-focused approach aligns with testing best practices.
packages/bot/src/handlers/clientHandler/presence.ts (3)
19-27: LGTM!The defensive null coalescing (
?? 0) handles potential edge cases wherememberCountmight be unavailable. The iteration approach is clear and efficient.
29-46: LGTM!Well-structured pure function with branded activities. The dynamic guild and member counts provide useful live statistics in the bot's presence.
72-83: LGTM!Clean implementation with proper resource cleanup. The returned stop function allows the caller to manage the rotation lifecycle correctly. The immediate
rotate()call ensures presence is set right away without waiting for the first interval.
| call_webhook_with_retry() { | ||
| local url="$1" | ||
| local attempt http body response | ||
| for attempt in $(seq 1 5); do | ||
| response=$(call_webhook "$url") | ||
| http=$(echo "$response" | tail -1) | ||
| body=$(echo "$response" | sed '$d') | ||
| echo "Response from $url (attempt $attempt/5): $body (HTTP $http)" | ||
|
|
||
| if [ "$http" -ge 200 ] && [ "$http" -lt 300 ]; then | ||
| echo "$response" | ||
| return 0 | ||
| fi | ||
|
|
||
| if [ "$http" -ge 500 ] || [ "$http" = "000" ]; then | ||
| if [ "$attempt" -lt 5 ]; then | ||
| sleep $((attempt * 2)) | ||
| continue | ||
| fi | ||
| fi | ||
|
|
||
| echo "$response" | ||
| return 0 | ||
| done | ||
| } |
There was a problem hiding this comment.
Persistent 5xx errors after retry exhaustion will incorrectly return success.
After all 5 retry attempts are exhausted for 5xx/network errors, the function falls through to lines 69-71 which echo the response and return 0. This means persistent server errors are treated as successful webhook calls, potentially masking deployment failures.
🐛 Proposed fix to fail after retry exhaustion
call_webhook_with_retry() {
local url="$1"
local attempt http body response
for attempt in $(seq 1 5); do
response=$(call_webhook "$url")
http=$(echo "$response" | tail -1)
body=$(echo "$response" | sed '$d')
echo "Response from $url (attempt $attempt/5): $body (HTTP $http)"
if [ "$http" -ge 200 ] && [ "$http" -lt 300 ]; then
echo "$response"
return 0
fi
if [ "$http" -ge 500 ] || [ "$http" = "000" ]; then
if [ "$attempt" -lt 5 ]; then
sleep $((attempt * 2))
continue
fi
+ # All retries exhausted for transient error
+ echo "$response"
+ return 1
fi
- echo "$response"
- return 0
+ # Non-retryable error (4xx except 404/405 handled by caller)
+ echo "$response"
+ return 0
done
+ # Should not reach here, but fail-safe
+ echo "$response"
+ return 1
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| call_webhook_with_retry() { | |
| local url="$1" | |
| local attempt http body response | |
| for attempt in $(seq 1 5); do | |
| response=$(call_webhook "$url") | |
| http=$(echo "$response" | tail -1) | |
| body=$(echo "$response" | sed '$d') | |
| echo "Response from $url (attempt $attempt/5): $body (HTTP $http)" | |
| if [ "$http" -ge 200 ] && [ "$http" -lt 300 ]; then | |
| echo "$response" | |
| return 0 | |
| fi | |
| if [ "$http" -ge 500 ] || [ "$http" = "000" ]; then | |
| if [ "$attempt" -lt 5 ]; then | |
| sleep $((attempt * 2)) | |
| continue | |
| fi | |
| fi | |
| echo "$response" | |
| return 0 | |
| done | |
| } | |
| call_webhook_with_retry() { | |
| local url="$1" | |
| local attempt http body response | |
| for attempt in $(seq 1 5); do | |
| response=$(call_webhook "$url") | |
| http=$(echo "$response" | tail -1) | |
| body=$(echo "$response" | sed '$d') | |
| echo "Response from $url (attempt $attempt/5): $body (HTTP $http)" | |
| if [ "$http" -ge 200 ] && [ "$http" -lt 300 ]; then | |
| echo "$response" | |
| return 0 | |
| fi | |
| if [ "$http" -ge 500 ] || [ "$http" = "000" ]; then | |
| if [ "$attempt" -lt 5 ]; then | |
| sleep $((attempt * 2)) | |
| continue | |
| fi | |
| # All retries exhausted for transient error | |
| echo "$response" | |
| return 1 | |
| fi | |
| # Non-retryable error (4xx except 404/405 handled by caller) | |
| echo "$response" | |
| return 0 | |
| done | |
| # Should not reach here, but fail-safe | |
| echo "$response" | |
| return 1 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy.yml around lines 48 - 72, The
call_webhook_with_retry function currently treats persistent 5xx or network
errors as success because after exhausting retries it echoes the response and
returns 0; change the control flow so that when call_webhook_with_retry (the
function) finishes all 5 attempts and the last HTTP code is a 5xx or "000", it
returns a non-zero exit code (e.g., return 1) to indicate failure instead of
returning 0—either by returning failure inside the branch when attempt == 5 or
by adding a final failure return after the retry loop while preserving the
existing successful-path behavior for 2xx responses and the existing echo of the
response body.
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Free Tier Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| response=$(call_webhook "$url") | ||
| http=$(echo "$response" | tail -1) | ||
| body=$(echo "$response" | sed '$d') | ||
| echo "Response from $url (attempt $attempt/5): $body (HTTP $http)" |
There was a problem hiding this comment.
Retry diagnostic messages swallowed by command substitution
Medium Severity
The echo diagnostic messages inside call_webhook_with_retry write to stdout, but the function is invoked via command substitution (response=$(call_webhook_with_retry ...)), which captures all stdout. This means retry progress lines like "Response from ... (attempt 2/5)..." are silently swallowed into the response variable instead of appearing in the GitHub Actions log. Debugging transient deploy webhook failures becomes impossible since no retry progress is visible. The diagnostic echo calls need to be redirected to stderr (>&2) so they appear in the CI log while keeping the actual response on stdout for capture.
Additional Locations (1)
| @@ -0,0 +1,7 @@ | |||
| [build] | |||
| base = "." | |||
| command = "npm ci --legacy-peer-deps --ignore-scripts && npm run db:generate && npm run build --workspace=packages/frontend" | |||
There was a problem hiding this comment.
Netlify build missing shared package build step
Medium Severity
The netlify.toml build command runs npm run build --workspace=packages/frontend without first building the shared package. The equivalent vercel.json correctly includes npm run build:shared before npm run build:frontend. If the frontend depends on build artifacts from packages/shared, Netlify deploys will fail with missing module errors.
* fix: harden vercel build for prisma client generation * ci: add oauth auth-config deploy smoke gate * ci: retry deploy webhook on transient failures * feat: improve bot activity presence rotation * ci: fix sonar key and ensure prisma generation in root build * test: increase bot presence coverage for sonar gate * ci: avoid postinstall rate-limit failures in actions * ci: configure netlify frontend preview build
This branch was successfully deployed
1 participant
Summary
/api/health/auth-configdeploy smoke check gate after webhook rolloutValidation
Notes
Note
Medium Risk
Touches CI/deploy automation and adds a post-deploy smoke gate, which could cause pipeline failures or block rollouts if the target service/endpoint behavior differs from expectations. Bot presence rotation introduces a new interval-driven runtime path, albeit low impact.
Overview
Improves deployment reliability and verification. The deploy workflow now retries webhook triggers (including transient 5xx/network failures) and treats
404/405as a signal to fall back to the canonical/webhook/deploypath, then runs a post-rollout OAuth config smoke check against/api/health/auth-configand fails on degraded/legacy-domain states.Adds rotating bot presence. The bot replaces the fixed activity with a timed rotation of branded presence messages, including live guild and member counters, with new unit tests covering rotation/indexing behavior.
Hardens builds across CI and hosting. CI installs now use
npm ci --ignore-scripts, root/Vercel/Netlify builds generate the Prisma client before building packages, and the Node engine is pinned to22.x(plus Sonar project key update and docs/changelog updates reflecting these changes).Written by Cursor Bugbot for commit 3f0a0dc. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Improvements
Chores
Tests