Merged
Conversation
- Add Inngest to Docker Compose (self-hosted + dev mode) - Create Inngest client with 4 functions: deliverWebhook, evaluateNotificationRule, exportProjectUsers, pollNotificationRules - Add webhook-dispatcher helper for CDC layer - Register /api/inngest serve endpoint in server - Update env validation for INNGEST_BASE_URL, INNGEST_SIGNING_KEY, INNGEST_EVENT_KEY - Migrate webhook retry/test endpoints to use Inngest - Add POST /admin/notifications/:id/test endpoint - Add test suite for Inngest functions - Update README and CODEBASE_MAP documentation
- Add backend proxy routes for Inngest API (/admin/inngest/*) - Create Inngest instance settings migration - Add inngest-client.ts API client with types - Create InngestDashboardPage.tsx with functions and runs view - Add /settings/inngest route and navigation link - Handle both self-hosted and cloud Inngest modes
Improve context generation in dev command and context generator by catching errors and logging detailed messages. Adds warning for missing @betterbase/core when IaC functions discovery fails. Also adds Inngest specification documentation files.
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. Warning
|
| Cohort / File(s) | Summary |
|---|---|
Docs & Examples \.env.example, README.md, CODEBASE_MAP.md, BetterBase_Inngest_Spec.md, BetterBase_Inngest_Dashboard_Spec.md |
Added Inngest docs, env variables, architecture/diagram updates and detailed integration/dashboard specifications. |
Docker & Nginx docker-compose.dev.yml, docker-compose.self-hosted.yml, docker/nginx/nginx.conf |
New Inngest service (dev & self-hosted), volumes, healthchecks; nginx proxy for /inngest/ and /api/inngest; server waits for Inngest readiness. |
Server Inngest Core packages/server/src/lib/inngest.ts, packages/server/src/lib/webhook-dispatcher.ts |
New Inngest client, typed events, four functions (deliverWebhook, evaluateNotificationRule, exportProjectUsers, pollNotificationRules), and dispatcher to enqueue per-webhook events. |
Server Routes & Admin API packages/server/src/routes/admin/inngest.ts, packages/server/src/routes/admin/index.ts, packages/server/src/routes/admin/notifications.ts, packages/server/src/routes/admin/project-scoped/webhooks.ts |
Added admin routes for status/functions/runs/jobs/test/cancel; routed webhook retry/test to enqueue Inngest events; added notifications test endpoint. |
Server Config & Migrations packages/server/src/index.ts, packages/server/src/lib/env.ts, packages/server/package.json, packages/server/migrations/014_inngest_support.sql, packages/server/migrations/015_inngest_settings.sql, packages/server/tsconfig.json |
Registered Inngest HTTP handler, extended env validation (INNGEST_*), added deps and DB migrations for export jobs and settings, adjusted tsconfig includes. |
Frontend Dashboard & Client apps/dashboard/src/lib/inngest-client.ts, apps/dashboard/src/pages/settings/InngestDashboardPage.tsx, apps/dashboard/src/routes.tsx, apps/dashboard/src/layouts/AppLayout.tsx |
Added typed admin API client, dashboard page with polling, function/runs views, cancel/test actions, route registration and sidebar link. |
CLI & IaC packages/cli/src/commands/dev.ts, packages/cli/src/utils/context-generator.ts, packages/core/src/iac/db-context.ts |
Improved dev command/context-generator error logging; added DatabaseWriter.delete to IaC DB context. |
Tests packages/server/test/inngest.test.ts, packages/server/test/routes.test.ts, packages/cli/test/generate-crud.test.ts, other server tests |
Added Inngest-focused tests; adjusted many tests' mock typings and added new assertions; one suite skipped to avoid flakiness. |
Client types & exports packages/client/src/iac/index.ts, packages/client/src/iac/provider.tsx, packages/client/src/index.ts |
Refactored exported config types to BetterBaseReactConfig, updated provider signatures and public type re-exports. |
Misc config .coderabbit.yaml, various small edits |
Minor config and linting/test typing adjustments across repo (tsconfig, package.json ordering, README dates, etc.). |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant Server as BetterBase Server
participant Inngest
participant DB as PostgreSQL
participant Webhook as Webhook Endpoint
Client->>Server: POST /api/project/:id/webhooks/:id/retry
Server->>DB: SELECT last failed delivery
Server->>DB: INSERT webhook_deliveries (status: pending) -> deliveryId
Server->>Inngest: inngest.send([{ name: "betterbase/webhook.deliver", data: { deliveryId, ... } }])
Server-->>Client: { success: true, queued: true }
Note over Inngest,Webhook: Inngest executes webhook.deliver
Inngest->>DB: Lookup webhook secret/metadata
Inngest->>Webhook: POST payload with HMAC header
Webhook-->>Inngest: 200 OK
Inngest->>DB: INSERT webhook_deliveries (status: success, response, duration)
sequenceDiagram
participant InngestCron as Inngest (cron)
participant Server as BetterBase Server
participant DB as PostgreSQL
participant Notifier as Email/Webhook Service
InngestCron->>Server: trigger pollNotificationRules()
Server->>DB: SELECT enabled notification rules
Server->>DB: Query request_logs for metrics (last 5min)
loop per rule
Server->>InngestCron: send({ name: "betterbase/notification.evaluate", data: { rule, currentValue } })
end
Note over InngestCron,Notifier: evaluate events perform deliveries when breached
InngestCron->>Notifier: SMTP or HTTP webhook delivery
Notifier-->>InngestCron: delivery result
InngestCron->>DB: record evaluation/delivery outcome
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- Feat/cli storage setup #31: Overlaps changes to
packages/cli/src/commands/dev.ts(dev command error handling). - feat: comprehensive core tasks for March 2026 release #36: Related webhook dispatch/delivery workflow and dispatcher logic changes.
- Pending changes exported from your codespace #41: Related IaC DB context changes (adds/edits to
packages/core/src/iac/db-context.ts).
Suggested labels
codex
Poem
🐰 I hopped into envs and docker with cheer,
I queued webhooks and polled rules far and near,
Functions hum, cron keeps the beat,
Dashboard shows runs — retries on repeat,
A carrot for jobs — background work is here! 🥕
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Title check | ❓ Inconclusive | The title "Adding Ingest" is vague and incomplete—it lacks specificity about what is being added and does not clearly communicate the scope or nature of the changes. | Revise the title to be more descriptive and specific, such as "Add Inngest integration for durable workflows and background jobs" or "Integrate Inngest for webhook delivery and notification orchestration." |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with 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.
Comment @coderabbitai help to get the list of available commands and usage tips.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9382d293d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/server/src/lib/inngest.ts
Outdated
Comment on lines
+120
to
+123
| if (!res.ok) { | ||
| // Throwing causes Inngest to retry with exponential backoff | ||
| throw new Error( | ||
| `Webhook delivery failed: HTTP ${res.status} from ${url} — ${responseBody.slice(0, 200)}`, |
There was a problem hiding this comment.
Non-2xx webhook responses throw at this point, but the only persistence step in deliverWebhook happens later in log-successful-delivery, so failures never get written to betterbase_meta.webhook_deliveries. This means failed deliveries (including exhausted retries) are missing from history, which breaks observability and retry diagnostics.
Useful? React with 👍 / 👎.
| `SELECT id, webhook_id as function_id, status, created_at as started_at, | ||
| delivered_at as ended_at, response_code, duration_ms | ||
| FROM betterbase_meta.webhook_deliveries | ||
| WHERE webhook_id = $1 |
There was a problem hiding this comment.
In self-hosted mode /admin/inngest/functions returns IDs like deliver-webhook, but this query filters by webhook_id = $1, where webhook_id is a webhook record ID. As a result, selecting a function from the dashboard will typically return no runs even when executions exist, so run history appears empty.
Useful? React with 👍 / 👎.
Comment on lines
+225
to
+229
| data: { | ||
| _test: true, | ||
| triggeredAt: new Date().toISOString(), | ||
| source: "admin-dashboard", | ||
| }, |
There was a problem hiding this comment.
The test endpoint emits a generic payload that does not include required fields for the mapped events (for example, betterbase/webhook.deliver expects webhookId, url, eventType, etc.). Those test-triggered runs will fail inside the function handlers while this route still returns success, so the “Test” action gives a false signal and cannot reliably validate function health.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
CODEBASE_MAP.md-570-575 (1)
570-575:⚠️ Potential issue | 🟡 MinorCode-format the cron expression in this table row.
The raw
*/5 * * * *is tripping markdownlint's emphasis parser here. Wrapping it in backticks keeps the table rendering stable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CODEBASE_MAP.md` around lines 570 - 575, The cron expression in the CODEBASE_MAP.md table row for the pollNotificationRules entry is being parsed as emphasis by markdownlint; update the table so the Trigger cell wraps the expression in backticks (i.e., change */5 * * * * to `*/5 * * * *`) for the pollNotificationRules row to ensure correct markdown rendering.CODEBASE_MAP.md-218-221 (1)
218-221:⚠️ Potential issue | 🟡 MinorThe self-hosted diagram is pointing at the wrong ports.
docker-compose.self-hosted.ymlrunsbetterbase-serveron3001, and the dashboard is fronted by nginx rather than exposed directly. These labels currently send operators to the wrong service.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CODEBASE_MAP.md` around lines 218 - 221, The diagram labels in CODEBASE_MAP.md are incorrect: update the port and exposure info so that docker-compose.self-hosted.yml's betterbase-server is shown as running on port 3001 (not the dashboard), indicate the Dashboard (React App) is fronted by nginx and not directly exposed on 3001, and ensure Inngest/Workflow Engine remains labeled with port 8288; search for the diagram block containing "Dashboard (React App)", "betterbase-server", and "Inngest (Workflow Engine)" and swap the port/exposure text accordingly.apps/dashboard/src/pages/settings/InngestDashboardPage.tsx-160-171 (1)
160-171:⚠️ Potential issue | 🟡 MinorGive the run-status filter an accessible name.
This
<select>has no associated<label>oraria-label, so assistive tech will expose it as an unnamed combobox in the header. Add an accessible label before shipping the settings page.♿ Proposed fix
<select + aria-label="Filter runs by status" className="border rounded px-2 py-1 text-sm" style={{ background: "var(--color-surface)", color: "var(--color-text-primary)" }} value={runStatusFilter}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/settings/InngestDashboardPage.tsx` around lines 160 - 171, The run-status <select> control (bound to runStatusFilter and updated via setRunStatusFilter) is missing an accessible name; add either a visible <label> tied to the select via htmlFor/id or an aria-label/aria-labelledby attribute on the select so screen readers can announce it (e.g., give the select an id like "run-status-filter" and add a preceding <label htmlFor="run-status-filter">Run status</label>, or add aria-label="Run status filter" directly to the select).BetterBase_Inngest_Spec.md-517-525 (1)
517-525:⚠️ Potential issue | 🟡 MinorSpec's
allInngestFunctionsis missingpollNotificationRules.The spec shows
allInngestFunctionswith only 3 functions (lines 520-524), but the task ING-05 (lines 906-914) addspollNotificationRulesto this array. The actual implementation file correctly includes all 4 functions. This inconsistency in the spec could cause confusion.The spec should show the final state after all tasks are complete, or clearly indicate that line 520-524 is before ING-05 changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterBase_Inngest_Spec.md` around lines 517 - 525, Update the spec's allInngestFunctions array to match the implementation by adding pollNotificationRules to the exported list (the array used for serve() registration), or annotate that the shown array is pre-ING-05; ensure the symbol names allInngestFunctions and pollNotificationRules are referenced so readers can find and verify the change against the ING-05 task description.BetterBase_Inngest_Spec.md-488-498 (1)
488-498:⚠️ Potential issue | 🟡 MinorCSV injection vulnerability in spec code snippet.
The CSV generation code (lines 493-494) doesn't escape quotes, commas, newlines, or formula injection characters. This mirrors the issue in the actual implementation.
Refer to the fix proposed in
packages/server/src/lib/inngest.tsreview for the escaping approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterBase_Inngest_Spec.md` around lines 488 - 498, The CSV building step in the "build-csv" step is vulnerable to CSV injection and unescaped special characters; update the step.run block that returns header + body (the "build-csv" handler) to escape cell values: replace direct interpolation of r.name, r.email, etc. with a sanitizer that 1) doubles internal double-quotes, 2) wraps fields containing commas/newlines/quotes in double-quotes, and 3) prefixes cells that start with =, +, -, or @ with a safe single-quote or other neutralizing character; follow the same escaping routine used in packages/server/src/lib/inngest.ts to sanitize each r.* field before joining into the CSV body so quotes, commas, newlines and formula injection are handled.BetterBase_Inngest_Spec.md-476-486 (1)
476-486:⚠️ Potential issue | 🟡 MinorSQL injection vulnerability in spec code snippet.
Same issue as in the actual implementation file—the
schemaNameis interpolated directly into the SQL query (line 480) without validation or parameterization. Since specs often serve as copy-paste templates, this should be corrected here as well.Refer to the fix proposed in
packages/server/src/lib/inngest.tsreview for the validation approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterBase_Inngest_Spec.md` around lines 476 - 486, The snippet interpolates schemaName directly into the SQL (used in the pool.query call and SELECT FROM ${schemaName}."user"), causing SQL injection risk; validate or sanitize schemaName before using it (e.g., check against a whitelist or enforce a strict identifier regex such as /^[a-z_][a-z0-9_]*$/) and only then interpolate a safe, validated identifier (or use a server-side mapping of allowed schema names) while keeping the query parameters passed via params; update the spec’s code around schemaName and the pool.query invocation to perform that validation like the fix shown in packages/server/src/lib/inngest.ts.apps/dashboard/src/lib/inngest-client.ts-18-25 (1)
18-25:⚠️ Potential issue | 🟡 MinorMissing
errorfield inInngestRuninterface.The spec (lines 313-314 in
BetterBase_Inngest_Dashboard_Spec.md) includes anerror?: stringfield inInngestRun, and the dashboard UI may need to display error messages for failed runs.🔧 Proposed fix
export interface InngestRun { id: string; functionId: string; status: "pending" | "running" | "complete" | "failed"; startedAt: string; endedAt?: string; output?: string; + error?: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/inngest-client.ts` around lines 18 - 25, Add the missing optional error field to the InngestRun interface so failed runs can surface error messages; update the InngestRun declaration (interface InngestRun) to include error?: string (optional) to match the spec and ensure dashboard components consuming InngestRun can access run.error.packages/server/src/lib/inngest.ts-140-152 (1)
140-152:⚠️ Potential issue | 🟡 MinorMissing
response_bodycolumn in webhook delivery insert.Per the migration schema (context snippet 1,
packages/server/migrations/010_delivery_invocation_logs.sql), thewebhook_deliveriestable has aresponse_bodycolumn, but it's not being populated. ThedeliveryResult.responseBodyvalue (line 130) is available but not inserted.🔧 Proposed fix
await step.run("log-successful-delivery", async () => { const { getPool } = await import("./db"); const pool = getPool(); await pool.query( `INSERT INTO betterbase_meta.webhook_deliveries - (webhook_id, event_type, payload, status, response_code, duration_ms, delivered_at, attempt_count) - VALUES ($1, $2, $3, 'success', $4, $5, NOW(), $6)`, + (webhook_id, event_type, payload, status, response_code, response_body, duration_ms, delivered_at, attempt_count) + VALUES ($1, $2, $3, 'success', $4, $5, $6, NOW(), $7)`, [ webhookId, eventType, JSON.stringify(payload), deliveryResult.httpStatus, + deliveryResult.responseBody, deliveryResult.durationMs, attempt, ], ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/inngest.ts` around lines 140 - 152, The INSERT into betterbase_meta.webhook_deliveries is missing the response_body column; update the pool.query call that inserts into webhook_deliveries (the INSERT and its parameter array) to include the response_body column and pass deliveryResult.responseBody as an additional parameter (e.g., add response_body to the column list and include deliveryResult.responseBody in the values array alongside webhookId, eventType, JSON.stringify(payload), deliveryResult.httpStatus, deliveryResult.durationMs, attempt) so the response body is persisted with each webhook delivery.BetterBase_Inngest_Dashboard_Spec.md-143-148 (1)
143-148:⚠️ Potential issue | 🟡 MinorIncorrect test event mapping for
poll-notification-rules.The
functionEventMapmapspoll-notification-rulestobetterbase/notification.evaluate, butpoll-notification-rulesis a cron-triggered function, not an event-triggered one. Sending anotification.evaluateevent won't trigger the cron function—it will triggerevaluate-notification-ruleinstead.For testing cron functions, consider either removing this mapping with a clear error message, or documenting that testing this function requires waiting for the next cron tick or using Inngest's dashboard directly.
🔧 Proposed fix
const functionEventMap: Record<string, string> = { "deliver-webhook": "betterbase/webhook.deliver", "evaluate-notification-rule": "betterbase/notification.evaluate", "export-project-users": "betterbase/export.users", - "poll-notification-rules": "betterbase/notification.evaluate", }; const eventName = functionEventMap[functionId]; if (!eventName) { - return c.json({ error: "Unknown function type" }, 400); + return c.json({ + error: functionId === "poll-notification-rules" + ? "Cron functions cannot be triggered via test events. Use the Inngest dashboard to invoke manually." + : "Unknown function type" + }, 400); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterBase_Inngest_Dashboard_Spec.md` around lines 143 - 148, The functionEventMap incorrectly maps the cron-triggered key "poll-notification-rules" to the event "betterbase/notification.evaluate"; remove that mapping from functionEventMap (or replace it with an explicit sentinel like null/"cron-only") and update any test harness code that looks up functionEventMap so it throws or logs a clear message when attempting to simulate a cron-only function; reference the functionEventMap object and the "poll-notification-rules" key and ensure tests targeting evaluate-notification-rule still use "betterbase/notification.evaluate" for event-driven simulation.apps/dashboard/src/lib/inngest-client.ts-27-52 (1)
27-52:⚠️ Potential issue | 🟡 MinorMissing HTTP error handling in API client methods.
All fetch calls blindly call
r.json()without checkingr.ok. If the server returns a 4xx/5xx status, the response body may contain an error structure that doesn't match the expected types, leading to silent failures or misleading data in the UI.🛡️ Proposed fix: Add error handling wrapper
+async function handleResponse<T>(response: Response): Promise<T> { + if (!response.ok) { + const error = await response.json().catch(() => ({ error: response.statusText })); + throw new Error(error.error || `HTTP ${response.status}`); + } + return response.json() as Promise<T>; +} + export const inngestApi = { - getStatus: () => fetch(`${API_BASE}/status`).then((r) => r.json() as Promise<InngestStatus>), + getStatus: () => fetch(`${API_BASE}/status`).then((r) => handleResponse<InngestStatus>(r)), - getFunctions: () => - fetch(`${API_BASE}/functions`).then((r) => r.json()) as Promise<{ - functions: InngestFunction[]; - }>, + getFunctions: () => + fetch(`${API_BASE}/functions`).then((r) => + handleResponse<{ functions: InngestFunction[] }>(r) + ),Apply similar pattern to remaining methods.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/inngest-client.ts` around lines 27 - 52, The API client currently calls r.json() directly in inngestApi methods (getStatus, getFunctions, getFunctionRuns, getRun, triggerTest, cancelRun, getJobs) without checking HTTP status; modify these calls to check r.ok and handle non-2xx responses by parsing the error body and throwing a rejected Error (or a typed error) so callers don't receive invalid data. Implement this by adding a small helper (e.g., apiFetch or checkResponse) that does fetch(...), awaits r, if r.ok returns r.json(), otherwise awaits r.json() or r.text() to extract error details and then throws a new Error containing the status and error payload; replace the direct fetch(...).then(r => r.json()) usage in each inngestApi method with calls to that helper.
🧹 Nitpick comments (8)
README.md (1)
32-34: Remove duplicate “(Convex-inspired)” label in the architecture block.The parenthetical appears twice in this section and reads as a copy artifact.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 32 - 34, Remove the duplicate parenthetical "(Convex-inspired)" in the architecture ASCII block so it appears only once; locate the repeated string next to the IaC Layer / betterbase label in the README architecture diagram and delete the extra "(Convex-inspired)" instance, leaving a single occurrence for clarity.docker-compose.dev.yml (1)
14-14: Pin the Inngest image tag to a specific version instead oflatest.Using
inngest/inngest:latestcauses non-deterministic behavior across different machines and over time as the tag points to different builds. Pin to a stable version tag (e.g.,v1.17.5) or digest for reproducible development environments. See Docker Hub for available versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.dev.yml` at line 14, Replace the non-deterministic image reference "image: inngest/inngest:latest" with a pinned tag or digest (for example "image: inngest/inngest:v1.17.5" or the image@sha256:<digest>) so the Compose service uses a specific, reproducible Inngest build; update the "image: inngest/inngest:latest" line accordingly and document the chosen version.packages/cli/src/commands/dev.ts (1)
45-47: Normalize caught rejection reasons before logging.These handlers assume the rejection reason is
Error. In practice it can be any value, so logs may degrade. Use a safe formatter with type narrowing (e instanceof Error ? e.message : String(e)). This pattern appears at lines 45–47 and 97–99, but also at lines 31–32, 34–35, 59–60, and 72.Suggested fix
- await ctxGen.generate(projectRoot).catch((e: Error) => { - error(`Context generation failed: ${e.message}`); + await ctxGen.generate(projectRoot).catch((e: unknown) => { + const message = e instanceof Error ? e.message : String(e); + error(`Context generation failed: ${message}`); });Apply the same pattern to all catch handlers that currently type the rejection as
Error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/dev.ts` around lines 45 - 47, The catch handlers assume rejection reasons are Error instances; update each catch that types its parameter as Error (e.g., the ctxGen.generate(...) catch) to normalize the rejection before logging by removing the explicit Error type and computing a safe message like: const msg = e instanceof Error ? e.message : String(e) and use that msg in the error(...) call; apply this exact pattern to every catch handler in this file that currently uses (e: Error) so logs never break when non-Error values are thrown.apps/dashboard/src/lib/inngest-client.ts (1)
30-33: Incorrect type cast placement.The type assertion
as Promise<{...}>is placed outside the.then()call, which casts the entire expression rather than the resolved value. This works but is misleading and inconsistent withgetStatuson line 28.♻️ Proposed fix
getFunctions: () => - fetch(`${API_BASE}/functions`).then((r) => r.json()) as Promise<{ - functions: InngestFunction[]; - }>, + fetch(`${API_BASE}/functions`).then((r) => r.json() as Promise<{ + functions: InngestFunction[]; + }>),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/inngest-client.ts` around lines 30 - 33, The type assertion is applied to the whole fetch promise instead of the resolved JSON; update getFunctions so the cast is applied to the value returned by r.json() (i.e., cast r.json() to Promise<{ functions: InngestFunction[] }> or cast the parsed JSON inside the .then callback) so the resolved value has the correct type (align with how getStatus handles typing); reference getFunctions, API_BASE, and InngestFunction when locating the change.BetterBase_Inngest_Spec.md (1)
973-975: Missing language specifier for fenced code block.The execution order summary code block lacks a language specifier.
📝 Proposed fix
-``` +```text Phase 1 — Infrastructure ING-01 Docker Compose services (self-hosted start + dev mode) + .env.exampleAs per coding guidelines from static analysis:
MD040, fenced-code-language.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterBase_Inngest_Spec.md` around lines 973 - 975, The fenced code block that shows the execution order summary (the block containing "Phase 1 — Infrastructure" and the "ING-01 Docker Compose services..." line) is missing a language specifier; update that opening fence to include a language (e.g., change ``` to ```text) so the block conforms to MD040 fenced-code-language rules and the spec renders correctly.packages/server/src/lib/inngest.ts (2)
215-232: Missing timeout for notification webhook fetch.The webhook notification fetch (lines 216-227) has no timeout configured. A slow or unresponsive target URL could cause the Inngest function to hang until the platform timeout (which may be several minutes).
⏱️ Proposed fix: Add AbortController timeout
if (channel === "webhook") { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout + const res = await fetch(target, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ rule_id: ruleId, rule_name: ruleName, metric, current_value: currentValue, threshold, triggered_at: new Date().toISOString(), }), + signal: controller.signal, }); + clearTimeout(timeoutId); + if (!res.ok) { throw new Error(`Notification webhook failed: HTTP ${res.status}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/inngest.ts` around lines 215 - 232, The webhook branch (the block where channel === "webhook") performs fetch(target, ...) with no timeout; update this code to use an AbortController to enforce a short request timeout (e.g., 5-10s). Create an AbortController, start a setTimeout that calls controller.abort() after the chosen ms, pass controller.signal into fetch, and clear the timer after fetch completes; ensure the thrown abort error is handled the same way the current non-ok response is reported so you still throw a descriptive Error on failure. Use the existing identifiers (channel, target, ruleId, ruleName, metric, currentValue, threshold) and replace the fetch call in that block with the AbortController-based request.
114-118: Missing timeout for webhook delivery fetch.Similar to the notification webhook, the main webhook delivery fetch (line 116) lacks a timeout. This is especially important since this function handles external URLs that may be slow or malicious.
⏱️ Proposed fix: Add AbortController timeout
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout + const start = Date.now(); - const res = await fetch(url, { method: "POST", headers, body }); + const res = await fetch(url, { method: "POST", headers, body, signal: controller.signal }); + clearTimeout(timeoutId); const duration = Date.now() - start;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/inngest.ts` around lines 114 - 118, The webhook delivery fetch lacks a timeout, so wrap the fetch call in an AbortController with a configurable timeout (e.g., timeoutMs defaulting to 10000), pass controller.signal into fetch (replacing fetch(url, { method: "POST", headers, body })), and clear the timeout after fetch completes; ensure the code around start, res and responseBody (the block that records start = Date.now(), awaits fetch and then reads res.text()) catches an AbortError and handles it gracefully (set responseBody to "" / mark as timed out) and still computes duration.BetterBase_Inngest_Dashboard_Spec.md (1)
670-678: Missing language specifier for fenced code block.The execution order summary code block (line 671) lacks a language specifier.
📝 Proposed fix
-``` +```text Phase 1 — Backend Routes IDG-01 Inngest API proxy routes + instance settingsAs per coding guidelines from static analysis:
MD040, fenced-code-language.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterBase_Inngest_Dashboard_Spec.md` around lines 670 - 678, The fenced code block containing "Phase 1 — Backend Routes" / "IDG-01" (and the subsequent "Phase 2 — Frontend Dashboard" / "IDG-02") is missing a language specifier; update the opening fence from ``` to ```text so the block uses a language tag (e.g., change the block that contains "Phase 1 — Backend Routes" and "IDG-02" to start with ```text). Ensure any other similar fenced blocks in the same document follow the same pattern to satisfy MD040 (fenced-code-language).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 90-103: The .env.example INNGEST_* variables (INNGEST_BASE_URL,
INNGEST_SIGNING_KEY, INNGEST_EVENT_KEY, INNGEST_LOG_LEVEL) are not present in
the server's environment validation schema, so add them to the server env
validation (the exported env validator/schema in env.ts) with appropriate types
and defaults, update the server to consume the validated values instead of raw
process.env where it reads Inngest config, or remove/annotate the entries from
.env.example; reference the INNGEST_* names when updating the env schema and the
code paths that construct the Inngest client/config.
In `@docker-compose.self-hosted.yml`:
- Around line 104-106: Remove the hardcoded fallback defaults for
INNGEST_SIGNING_KEY and INNGEST_EVENT_KEY so the self-hosted compose fails fast
when those secrets are not provided (i.e., stop using
${INNGEST_SIGNING_KEY:-betterbase-dev-signing-key} and
${INNGEST_EVENT_KEY:-betterbase-dev-event-key}); mirror the existing pattern
used for BETTERBASE_JWT_SECRET by requiring the env vars to be set (no `:-`
default) in the docker-compose.self-hosted.yml, and if you need local dev
defaults keep them only in a separate dev-only compose file.
In `@docker/nginx/nginx.conf`:
- Around line 27-32: The /inngest/ location block currently proxies to the
Inngest dashboard (location /inngest/ with proxy_pass http://inngest) and must
be protected before release; update the nginx/nginx.conf location /inngest/
block to enforce access control (for example enable HTTP basic auth via
auth_basic/auth_basic_user_file, restrict by IP with allow/deny, or mark the
upstream internal and expose only via authenticated gateway) and ensure
proxy_set_header directives remain; apply the chosen auth method consistently
and add a clear comment in the location block referencing the protection
mechanism.
In `@packages/server/migrations/014_inngest_support.sql`:
- Line 8: The migration currently persists raw CSV bodies in the
betterbase_meta.export_jobs table via the result_csv column; instead remove or
stop using result_csv and replace it with a reference to object storage plus
expiry metadata (e.g., add result_object_key/result_url TEXT and
result_expires_at TIMESTAMP WITH TIME ZONE) so artifacts live in MinIO and only
a pointer is kept in Postgres, and implement a migration path to move existing
CSVs to object storage and write their object keys (or add a cleanup TTL
job/policy to purge old CSVs and avoid storing new ones in result_csv). Ensure
you update code paths that write/read export jobs (the export job insert/update
and reader functions) to use the new object key/url and expiry fields instead of
result_csv.
In `@packages/server/src/index.ts`:
- Line 75: Replace the direct use of process.env.INNGEST_SIGNING_KEY with the
validated env value (e.g., use env.INNGEST_SIGNING_KEY) when assigning
signingKey in the Inngest configuration so startup validation is enforced; then
add INNGEST_SIGNING_KEY to the exported schema/defaults in
packages/server/src/lib/env.ts (the env schema) so the validator fails fast on
missing/invalid values. Ensure the symbol names referenced are signingKey (the
Inngest config field) and INNGEST_SIGNING_KEY in the env schema/export.
In `@packages/server/src/lib/env.ts`:
- Line 13: The default for STORAGE_BUCKET was changed (symbol STORAGE_BUCKET in
env.ts) which is a breaking change; revert the default value back to the
original "betterbase" or remove the hardcoded default so deployments must
explicitly provide STORAGE_BUCKET, and update any relevant tests/configs to
reflect the restored behavior to prevent silent uploads to a different bucket.
- Around line 17-19: The schema currently marks INNGEST_SIGNING_KEY and
INNGEST_EVENT_KEY as optional; update validateEnv() to perform a runtime check
for production cloud mode (e.g., when NODE_ENV === "production" or when
INNGEST_BASE_URL points to the cloud) and throw/exit with a clear error if
INNGEST_SIGNING_KEY or INNGEST_EVENT_KEY are missing; also ensure
INNGEST_SIGNING_KEY does not silently remain undefined by either requiring it in
that runtime check or providing a secure fallback, and preserve or reaffirm the
existing default for INNGEST_EVENT_KEY ("betterbase-dev-event-key") for
non-production environments.
In `@packages/server/src/lib/inngest.ts`:
- Around line 300-310: The CSV generation in the step.run("build-csv") block
naively interpolates row values and doesn't escape quotes, commas, newlines or
neutralize formula-injection leading characters; update the mapping over rows
(the function that builds body in step.run("build-csv")) to: convert each field
to a string, double any internal double-quotes and wrap every field in double
quotes to preserve commas/newlines, and for string fields that start with =, +,
-, or @ prefix with a single quote (') to neutralize CSV formula injection;
ensure non-string types (booleans, dates, numbers) are formatted consistently
before escaping.
- Around line 290-297: The SQL built in the export flow interpolates schemaName
(derived from projectSlug) directly into the query string in the block using
pool.query, creating an SQL injection risk; fix it by validating or mapping
projectSlug -> schemaName before use (e.g., enforce a strict regex whitelist
like /^[a-z0-9_]+$/ or map allowed slugs to internal schema names) and stop
direct string interpolation in the query that constructs `FROM
${schemaName}."user"`; alternatively use an identifier-escaping helper such as
pg-format's %I or pg.Client.prototype.escapeIdentifier when building the SQL,
and keep all user-provided values as parameterized params for pool.query (refer
to variables projectSlug, schemaName, and the pool.query call).
In `@packages/server/src/lib/webhook-dispatcher.ts`:
- Around line 17-23: The query in webhook-dispatcher.ts selects and copies the
secret into the enqueued event (the rows assigned to variable webhooks) which
causes the signing secret to be sent into the workflow system; instead, modify
the SELECT to only return id, name, url (not secret) and enqueue webhookId (e.g.
webhook.id) in the betterbase/webhook.deliver event payload, then change the
delivery worker to resolve the secret from Postgres by webhookId right before
signing the outbound request (apply the same fix where secret is currently used
in the enqueue logic around the other occurrence noted in lines 29-37).
In `@packages/server/src/routes/admin/inngest.ts`:
- Around line 91-96: The handler currently always returns c.json({ functions:
data.functions ?? [] }) even when the upstream fetch failed; modify the code
that creates url, calls fetch (res) and parses data so that you check res.ok
and, if false, propagate the upstream status and error payload back to the
client (e.g., return c.status(res.status).json({ error: data ?? { message:
res.statusText } })), otherwise continue returning the success shape; apply the
same change to the other similar blocks that use url/res/data and return c.json
(the other fetches at the same pattern for runs).
- Around line 222-230: The test endpoint currently sends the same generic
payload (the object built where inngest.send is called using eventName) for
every workflow, which causes validation failures; update the call site to build
function-specific sample payloads by switching on the workflow identifier (use
the existing eventName or the workflow's functionId) and construct tailored data
objects for each workflow (e.g., include webhook URL and delivery fields for
deliver-webhook, userId/message/notification context for notification.evaluate,
export parameters such as exportType or filters for export.users), then pass
that mapped sample payload to inngest.send; keep a sensible default fallback
payload for unknown functionIds so tests still run.
- Around line 116-127: The route is filtering webhook_deliveries by the concrete
webhook_id (betterbase_meta.webhook_deliveries.webhook_id) but the handler
receives an Inngest function id (e.g., "deliver-webhook"), so the query returns
no runs; update the SQL WHERE clause to filter by the persisted function id
column (use WHERE function_id = $1) or otherwise query the correct column that
stores the Inngest function id, and keep the SELECT alias (webhook_id as
function_id) consistent; update the parameter usage in the pool.query call
around getPool() / isSelfHosted() to pass the function id into that corrected
WHERE clause.
- Around line 204-220: The variable eventName is declared as a const but later
reassigned; change its declaration to let so it can be reassigned when deriving
a mapping from functionEventMap using functionId (leave the mapping logic that
uses Object.entries(functionEventMap).find(...) intact) and ensure the error
branch still returns c.json({ error: "Unknown function type" }, 400) if no
mapping is found.
In `@packages/server/src/routes/admin/project-scoped/webhooks.ts`:
- Around line 76-97: The pending delivery row is created after calling
inngest.send() so the enqueued betterbase/webhook.deliver event cannot reference
it; change the sequence in the handler to INSERT into
betterbase_meta.webhook_deliveries first (using RETURNING id to get the new
delivery id), then include that delivery id in the payload passed to
inngest.send() so the worker handling the event can deterministically update the
exact webhook_deliveries row; update any worker logic that uses the event to
expect and use delivery_id to mark status/attempts for the delivery.
- Around line 65-74: The lookup for "latest failed delivery" currently selects
any delivery and falls back to an empty payload; update the SQL in the
pool.query that reads betterbase_meta.webhook_deliveries (used to set payload
and attempt) to restrict results to failed deliveries (e.g., add a WHERE status
= 'failed' clause) and ORDER BY created_at DESC LIMIT 1, then handle the case
where no failed delivery exists by returning a 400/404 response instead of
defaulting to {}. Adjust the logic that derives payload and attempt (the payload
and attempt variables) so they only read from lastDelivery[0] when a failed row
exists and do not enqueue a retry or increment attempt if none is found.
In `@packages/server/test/inngest.test.ts`:
- Around line 10-34: The test currently only compares the local mock
objects/arrays to themselves after calling mock.module, so change the test to
import the real module under test after setting up mocks (use dynamic
import/require after mock.module) and assert on the module's behavior and
interactions instead of literal values; specifically, verify that the real
module registers functions and calls into the mocks by checking
mockInngestCreateFunction and mockInngestSend call history and that the module
exported items (allInngestFunctions, deliverWebhook, evaluateNotificationRule,
exportProjectUsers, pollNotificationRules) match what the module under test
actually registers/calls, and similarly ensure DB usage by asserting calls on
mockPool.query/getPool rather than comparing local arrays.
In `@packages/server/test/routes.test.ts`:
- Around line 81-144: Tests under the "should evaluate..." and the "Inngest
webhook delivery logic"/"Inngest cron polling logic" describes are tautological;
replace them with tests that call the actual production functions/handlers
instead of asserting local constants. For each spec (e.g., the threshold tests
in the "should evaluate threshold breach correctly" block, the
retry/concurrency/signature tests in "Inngest webhook delivery logic", and the
cron/error-rate tests in "Inngest cron polling logic") import and invoke the
real functions (e.g., the threshold evaluation function used by your route, the
function that builds retry events/constructs concurrency keys, the HMAC signing
function used for webhooks, the cron parser/poller and error rate calculator),
use mocks/stubs for external deps (Inngest client, request/response objects) and
assert meaningful outputs and side effects (HTTP responses, emitted events,
correct signature, retry attempt increment, concurrency key format, parsed cron
parts, and division-by-zero handling) instead of asserting hardcoded local
variables; remove or convert the trivial literal assertions into these
integration/unit calls and assertions.
In `@packages/server/tsconfig.json`:
- Line 9: The tsconfig currently sets "rootDir": "src" while the "include" array
contains "test/**/*", causing TS6059 because test files live outside the
rootDir; update the tsconfig's rootDir value from "src" to "." (or alternatively
remove/move "test/**/*" from the include) so all included files are inside the
designated rootDir and the tsc --noEmit errors are resolved.
---
Minor comments:
In `@apps/dashboard/src/lib/inngest-client.ts`:
- Around line 18-25: Add the missing optional error field to the InngestRun
interface so failed runs can surface error messages; update the InngestRun
declaration (interface InngestRun) to include error?: string (optional) to match
the spec and ensure dashboard components consuming InngestRun can access
run.error.
- Around line 27-52: The API client currently calls r.json() directly in
inngestApi methods (getStatus, getFunctions, getFunctionRuns, getRun,
triggerTest, cancelRun, getJobs) without checking HTTP status; modify these
calls to check r.ok and handle non-2xx responses by parsing the error body and
throwing a rejected Error (or a typed error) so callers don't receive invalid
data. Implement this by adding a small helper (e.g., apiFetch or checkResponse)
that does fetch(...), awaits r, if r.ok returns r.json(), otherwise awaits
r.json() or r.text() to extract error details and then throws a new Error
containing the status and error payload; replace the direct fetch(...).then(r =>
r.json()) usage in each inngestApi method with calls to that helper.
In `@apps/dashboard/src/pages/settings/InngestDashboardPage.tsx`:
- Around line 160-171: The run-status <select> control (bound to runStatusFilter
and updated via setRunStatusFilter) is missing an accessible name; add either a
visible <label> tied to the select via htmlFor/id or an
aria-label/aria-labelledby attribute on the select so screen readers can
announce it (e.g., give the select an id like "run-status-filter" and add a
preceding <label htmlFor="run-status-filter">Run status</label>, or add
aria-label="Run status filter" directly to the select).
In `@BetterBase_Inngest_Dashboard_Spec.md`:
- Around line 143-148: The functionEventMap incorrectly maps the cron-triggered
key "poll-notification-rules" to the event "betterbase/notification.evaluate";
remove that mapping from functionEventMap (or replace it with an explicit
sentinel like null/"cron-only") and update any test harness code that looks up
functionEventMap so it throws or logs a clear message when attempting to
simulate a cron-only function; reference the functionEventMap object and the
"poll-notification-rules" key and ensure tests targeting
evaluate-notification-rule still use "betterbase/notification.evaluate" for
event-driven simulation.
In `@BetterBase_Inngest_Spec.md`:
- Around line 517-525: Update the spec's allInngestFunctions array to match the
implementation by adding pollNotificationRules to the exported list (the array
used for serve() registration), or annotate that the shown array is pre-ING-05;
ensure the symbol names allInngestFunctions and pollNotificationRules are
referenced so readers can find and verify the change against the ING-05 task
description.
- Around line 488-498: The CSV building step in the "build-csv" step is
vulnerable to CSV injection and unescaped special characters; update the
step.run block that returns header + body (the "build-csv" handler) to escape
cell values: replace direct interpolation of r.name, r.email, etc. with a
sanitizer that 1) doubles internal double-quotes, 2) wraps fields containing
commas/newlines/quotes in double-quotes, and 3) prefixes cells that start with
=, +, -, or @ with a safe single-quote or other neutralizing character; follow
the same escaping routine used in packages/server/src/lib/inngest.ts to sanitize
each r.* field before joining into the CSV body so quotes, commas, newlines and
formula injection are handled.
- Around line 476-486: The snippet interpolates schemaName directly into the SQL
(used in the pool.query call and SELECT FROM ${schemaName}."user"), causing SQL
injection risk; validate or sanitize schemaName before using it (e.g., check
against a whitelist or enforce a strict identifier regex such as
/^[a-z_][a-z0-9_]*$/) and only then interpolate a safe, validated identifier (or
use a server-side mapping of allowed schema names) while keeping the query
parameters passed via params; update the spec’s code around schemaName and the
pool.query invocation to perform that validation like the fix shown in
packages/server/src/lib/inngest.ts.
In `@CODEBASE_MAP.md`:
- Around line 570-575: The cron expression in the CODEBASE_MAP.md table row for
the pollNotificationRules entry is being parsed as emphasis by markdownlint;
update the table so the Trigger cell wraps the expression in backticks (i.e.,
change */5 * * * * to `*/5 * * * *`) for the pollNotificationRules row to ensure
correct markdown rendering.
- Around line 218-221: The diagram labels in CODEBASE_MAP.md are incorrect:
update the port and exposure info so that docker-compose.self-hosted.yml's
betterbase-server is shown as running on port 3001 (not the dashboard), indicate
the Dashboard (React App) is fronted by nginx and not directly exposed on 3001,
and ensure Inngest/Workflow Engine remains labeled with port 8288; search for
the diagram block containing "Dashboard (React App)", "betterbase-server", and
"Inngest (Workflow Engine)" and swap the port/exposure text accordingly.
In `@packages/server/src/lib/inngest.ts`:
- Around line 140-152: The INSERT into betterbase_meta.webhook_deliveries is
missing the response_body column; update the pool.query call that inserts into
webhook_deliveries (the INSERT and its parameter array) to include the
response_body column and pass deliveryResult.responseBody as an additional
parameter (e.g., add response_body to the column list and include
deliveryResult.responseBody in the values array alongside webhookId, eventType,
JSON.stringify(payload), deliveryResult.httpStatus, deliveryResult.durationMs,
attempt) so the response body is persisted with each webhook delivery.
---
Nitpick comments:
In `@apps/dashboard/src/lib/inngest-client.ts`:
- Around line 30-33: The type assertion is applied to the whole fetch promise
instead of the resolved JSON; update getFunctions so the cast is applied to the
value returned by r.json() (i.e., cast r.json() to Promise<{ functions:
InngestFunction[] }> or cast the parsed JSON inside the .then callback) so the
resolved value has the correct type (align with how getStatus handles typing);
reference getFunctions, API_BASE, and InngestFunction when locating the change.
In `@BetterBase_Inngest_Dashboard_Spec.md`:
- Around line 670-678: The fenced code block containing "Phase 1 — Backend
Routes" / "IDG-01" (and the subsequent "Phase 2 — Frontend Dashboard" /
"IDG-02") is missing a language specifier; update the opening fence from ``` to
```text so the block uses a language tag (e.g., change the block that contains
"Phase 1 — Backend Routes" and "IDG-02" to start with ```text). Ensure any other
similar fenced blocks in the same document follow the same pattern to satisfy
MD040 (fenced-code-language).
In `@BetterBase_Inngest_Spec.md`:
- Around line 973-975: The fenced code block that shows the execution order
summary (the block containing "Phase 1 — Infrastructure" and the "ING-01 Docker
Compose services..." line) is missing a language specifier; update that opening
fence to include a language (e.g., change ``` to ```text) so the block conforms
to MD040 fenced-code-language rules and the spec renders correctly.
In `@docker-compose.dev.yml`:
- Line 14: Replace the non-deterministic image reference "image:
inngest/inngest:latest" with a pinned tag or digest (for example "image:
inngest/inngest:v1.17.5" or the image@sha256:<digest>) so the Compose service
uses a specific, reproducible Inngest build; update the "image:
inngest/inngest:latest" line accordingly and document the chosen version.
In `@packages/cli/src/commands/dev.ts`:
- Around line 45-47: The catch handlers assume rejection reasons are Error
instances; update each catch that types its parameter as Error (e.g., the
ctxGen.generate(...) catch) to normalize the rejection before logging by
removing the explicit Error type and computing a safe message like: const msg =
e instanceof Error ? e.message : String(e) and use that msg in the error(...)
call; apply this exact pattern to every catch handler in this file that
currently uses (e: Error) so logs never break when non-Error values are thrown.
In `@packages/server/src/lib/inngest.ts`:
- Around line 215-232: The webhook branch (the block where channel ===
"webhook") performs fetch(target, ...) with no timeout; update this code to use
an AbortController to enforce a short request timeout (e.g., 5-10s). Create an
AbortController, start a setTimeout that calls controller.abort() after the
chosen ms, pass controller.signal into fetch, and clear the timer after fetch
completes; ensure the thrown abort error is handled the same way the current
non-ok response is reported so you still throw a descriptive Error on failure.
Use the existing identifiers (channel, target, ruleId, ruleName, metric,
currentValue, threshold) and replace the fetch call in that block with the
AbortController-based request.
- Around line 114-118: The webhook delivery fetch lacks a timeout, so wrap the
fetch call in an AbortController with a configurable timeout (e.g., timeoutMs
defaulting to 10000), pass controller.signal into fetch (replacing fetch(url, {
method: "POST", headers, body })), and clear the timeout after fetch completes;
ensure the code around start, res and responseBody (the block that records start
= Date.now(), awaits fetch and then reads res.text()) catches an AbortError and
handles it gracefully (set responseBody to "" / mark as timed out) and still
computes duration.
In `@README.md`:
- Around line 32-34: Remove the duplicate parenthetical "(Convex-inspired)" in
the architecture ASCII block so it appears only once; locate the repeated string
next to the IaC Layer / betterbase label in the README architecture diagram and
delete the extra "(Convex-inspired)" instance, leaving a single occurrence for
clarity.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 074109f7-1147-43d3-be5c-752c587edace
📒 Files selected for processing (28)
.env.exampleBetterBase_Inngest_Dashboard_Spec.mdBetterBase_Inngest_Spec.mdCODEBASE_MAP.mdREADME.mdapps/dashboard/src/layouts/AppLayout.tsxapps/dashboard/src/lib/inngest-client.tsapps/dashboard/src/pages/settings/InngestDashboardPage.tsxapps/dashboard/src/routes.tsxdocker-compose.dev.ymldocker-compose.self-hosted.ymldocker/nginx/nginx.confpackages/cli/src/commands/dev.tspackages/cli/src/utils/context-generator.tspackages/server/migrations/014_inngest_support.sqlpackages/server/migrations/015_inngest_settings.sqlpackages/server/package.jsonpackages/server/src/index.tspackages/server/src/lib/env.tspackages/server/src/lib/inngest.tspackages/server/src/lib/webhook-dispatcher.tspackages/server/src/routes/admin/index.tspackages/server/src/routes/admin/inngest.tspackages/server/src/routes/admin/notifications.tspackages/server/src/routes/admin/project-scoped/webhooks.tspackages/server/test/inngest.test.tspackages/server/test/routes.test.tspackages/server/tsconfig.json
Comment on lines
+81
to
+144
| it("should evaluate threshold breach correctly", () => { | ||
| const threshold = 5; | ||
| const currentValue = 10; | ||
| const breached = currentValue >= threshold; | ||
| expect(breached).toBe(true); | ||
| }); | ||
|
|
||
| it("should not breach when value is below threshold", () => { | ||
| const threshold = 5; | ||
| const currentValue = 3; | ||
| const breached = currentValue >= threshold; | ||
| expect(breached).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Inngest webhook delivery logic", () => { | ||
| it("should construct retry event with incremented attempt", () => { | ||
| const lastAttempt = 2; | ||
| const newAttempt = lastAttempt + 1; | ||
| expect(newAttempt).toBe(3); | ||
| }); | ||
|
|
||
| it("should include webhook ID in concurrency key", () => { | ||
| const webhookId = "wh_abc123"; | ||
| const key = `event.data.${webhookId}`; | ||
| expect(key).toBe("event.data.wh_abc123"); | ||
| }); | ||
|
|
||
| it("should generate HMAC signature for webhook payload", () => { | ||
| // This tests the signature generation logic | ||
| const secret = "test-secret"; | ||
| const body = JSON.stringify({ test: "data" }); | ||
|
|
||
| // Simple HMAC-SHA256 simulation | ||
| expect(secret).toBe("test-secret"); | ||
| expect(typeof body).toBe("string"); | ||
| }); | ||
|
|
||
| it("should handle pending delivery status for dashboard", () => { | ||
| const status = "pending"; | ||
| expect(status).toBe("pending"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Inngest cron polling logic", () => { | ||
| it("should parse 5-minute cron expression correctly", () => { | ||
| const cron = "*/5 * * * *"; | ||
| const parts = cron.split(" "); | ||
| expect(parts[0]).toBe("*/5"); | ||
| }); | ||
|
|
||
| it("should calculate error rate from request logs", () => { | ||
| const totalRequests = 100; | ||
| const errorRequests = 5; | ||
| const errorRate = (errorRequests / totalRequests) * 100; | ||
| expect(errorRate).toBe(5); | ||
| }); | ||
|
|
||
| it("should handle zero requests without division by zero", () => { | ||
| const totalRequests = 0; | ||
| const errorRequests = 0; | ||
| const errorRate = totalRequests > 0 ? (errorRequests / totalRequests) * 100 : 0; | ||
| expect(errorRate).toBe(0); | ||
| }); |
There was a problem hiding this comment.
New tests are mostly tautological and don’t validate production behavior.
Most added cases assert local constants (e.g., threshold comparisons, string formatting, status literals) rather than exercising route logic or Inngest integration code paths. This inflates coverage without protecting against regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/test/routes.test.ts` around lines 81 - 144, Tests under the
"should evaluate..." and the "Inngest webhook delivery logic"/"Inngest cron
polling logic" describes are tautological; replace them with tests that call the
actual production functions/handlers instead of asserting local constants. For
each spec (e.g., the threshold tests in the "should evaluate threshold breach
correctly" block, the retry/concurrency/signature tests in "Inngest webhook
delivery logic", and the cron/error-rate tests in "Inngest cron polling logic")
import and invoke the real functions (e.g., the threshold evaluation function
used by your route, the function that builds retry events/constructs concurrency
keys, the HMAC signing function used for webhooks, the cron parser/poller and
error rate calculator), use mocks/stubs for external deps (Inngest client,
request/response objects) and assert meaningful outputs and side effects (HTTP
responses, emitted events, correct signature, retry attempt increment,
concurrency key format, parsed cron parts, and division-by-zero handling)
instead of asserting hardcoded local variables; remove or convert the trivial
literal assertions into these integration/unit calls and assertions.
- Implement CSV escaping helper to prevent injection attacks and handle special characters. - Add schema name validation to prevent SQL injection. - Refactor webhook delivery function to resolve secrets from the database if not provided. - Introduce timeout handling for HTTP requests in webhook delivery and notification evaluation. - Update webhook delivery logging to include response body and delivery ID for retries. - Improve error handling in Inngest admin routes and add fetch response error checks. - Enhance test coverage for Inngest functions and webhook delivery logic. - Update TypeScript configuration for better module resolution.
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/client/src/iac/provider.tsx (2)
34-39:⚠️ Potential issue | 🔴 CriticalReconnection logic doesn't restore event handlers.
The WebSocket reconnection in
onclosecreates a new WebSocket and assigns it towsRef.current, but it doesn't attachonopen,onclose, oronmessagehandlers. The reconnected socket will never:
- Set
wsReadytotrue- Handle ping/pong messages
- Attempt further reconnections if it closes again
🐛 Proposed fix: Extract WebSocket setup into a reusable function
useEffect(() => { - const wsUrl = `${config.url.replace(/^http/, "ws")}/betterbase/ws?project=${config.projectSlug ?? "default"}`; - const ws = new WebSocket(wsUrl); - - ws.onopen = () => { - setWsReady(true); - }; - ws.onclose = () => { - setWsReady(false); - // Reconnect after 3 seconds - setTimeout(() => { - wsRef.current = new WebSocket(wsUrl); - }, 3_000); - }; - - wsRef.current = ws; - - // Handle pings - ws.onmessage = (event) => { - const msg = JSON.parse(event.data); - if (msg.type === "ping") ws.send(JSON.stringify({ type: "pong" })); - }; + const wsUrl = `${config.url.replace(/^http/, "ws")}/betterbase/ws?project=${config.projectSlug ?? "default"}`; + + const connect = () => { + const ws = new WebSocket(wsUrl); + + ws.onopen = () => { + setWsReady(true); + }; + ws.onclose = () => { + setWsReady(false); + // Reconnect after 3 seconds + setTimeout(connect, 3_000); + }; + ws.onmessage = (event) => { + const msg = JSON.parse(event.data); + if (msg.type === "ping") ws.send(JSON.stringify({ type: "pong" })); + }; + + wsRef.current = ws; + }; + + connect(); return () => { - ws.close(); + wsRef.current?.close(); }; }, [config.url, config.projectSlug]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/iac/provider.tsx` around lines 34 - 39, The reconnection creates a fresh WebSocket but never re-attaches handlers, so extract the socket creation and handler wiring into a reusable helper (e.g., setupWebSocket) and call it both on initial mount and inside ws.onclose; ensure setupWebSocket creates new WebSocket(wsUrl), assigns wsRef.current, and attaches onopen (to setWsReady(true)), onmessage (to handle ping/pong and other messages), and onclose (to schedule another reconnection) so the reconnected socket restores wsReady, ping handling, and further reconnections.
45-48:⚠️ Potential issue | 🟡 MinorMissing error handling for JSON parsing.
If the server sends a malformed message,
JSON.parsewill throw and could crash the application. Consider wrapping in a try-catch.🛡️ Proposed fix
ws.onmessage = (event) => { - const msg = JSON.parse(event.data); - if (msg.type === "ping") ws.send(JSON.stringify({ type: "pong" })); + try { + const msg = JSON.parse(event.data); + if (msg.type === "ping") ws.send(JSON.stringify({ type: "pong" })); + } catch { + // Ignore malformed messages + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/iac/provider.tsx` around lines 45 - 48, Wrap the ws.onmessage handler's JSON.parse of event.data in a try-catch to prevent a thrown error from crashing the app; inside the catch, handle malformed messages (e.g., log the parse error and the raw event.data or ignore it) and return early, otherwise continue to inspect msg.type and call ws.send as before (refer to ws.onmessage, JSON.parse(event.data), msg.type, and ws.send).README.md (1)
87-96:⚠️ Potential issue | 🟡 MinorAdd language specifier to fenced code block.
The project structure code block lacks a language identifier, which affects syntax highlighting and accessibility. Per static analysis hint (MD040).
🔧 Suggested fix
-``` +```text my-app/ ├── betterbase/ │ ├── schema.ts # Define tables (Convex-style)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 87 - 96, Update the fenced code block showing the project tree in README.md to include a language specifier (e.g., "text") after the opening triple backticks so syntax highlighting and MD040 are satisfied; locate the block that begins with the triple backticks and contains "my-app/" and the betterbase files (schema.ts, queries/, mutations/, actions/, betterbase.config.ts) and change the opening fence to include the specifier.packages/server/src/lib/env.ts (1)
3-19:⚠️ Potential issue | 🔴 CriticalCritical:
PORTremoved from schema but required by server startup.Line 93 in
packages/server/src/index.tscallsNumber.parseInt(env.PORT). SincePORTis not inEnvSchema,env.PORTwill beundefined, causingNumber.parseInt(undefined)to returnNaN. The server will fail to bind to a valid port.Add
PORTback to the schema with a sensible default:const EnvSchema = z.object({ DATABASE_URL: z.string().min(1), BETTERBASE_JWT_SECRET: z.string().min(32, "JWT secret must be at least 32 characters"), BETTERBASE_ADMIN_EMAIL: z.string().email().optional(), BETTERBASE_ADMIN_PASSWORD: z.string().min(8).optional(), NODE_ENV: z.enum(["development", "production", "test"]).default("development"), + PORT: z.string().default("3001"), STORAGE_ENDPOINT: z.string().optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/env.ts` around lines 3 - 19, EnvSchema is missing the PORT field which causes env.PORT to be undefined (used by Number.parseInt(env.PORT) in index.ts); update the EnvSchema object to include PORT as a numeric/string port entry with a sensible default (e.g., "3000") and appropriate validation (e.g., z.string().default("3000") or z.coerce.number().default(3000)) so that env.PORT is always defined and Number.parseInt(env.PORT) returns a valid port; ensure the symbol EnvSchema is updated and re-exported/used where env is derived so index.ts's Number.parseInt(env.PORT) no longer yields NaN.
🧹 Nitpick comments (10)
docker/nginx/nginx.conf (2)
30-41: IP restriction directives are placed afterproxy_pass— move them before.In nginx,
allow/denydirectives should come beforeproxy_passfor clarity and to ensure the access control is evaluated before proxying. While nginx processes directives in a specific order regardless of placement, the current ordering is unconventional and could cause confusion during maintenance.🔧 Suggested reorder
location /inngest/ { + # IP-based access control - restricts to internal networks only + allow 10.0.0.0/8; + allow 172.16.0.0/12; + allow 192.168.0.0/16; + deny all; + rewrite ^/inngest/(.*) /$1 break; proxy_pass http://inngest; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - # IP-based access control - restricts to internal networks only - # Current protection: allow from 10.x, 172.16-31.x, 192.168.x ranges - allow 10.0.0.0/8; - allow 172.16.0.0/12; - allow 192.168.0.0/16; - deny all; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/nginx/nginx.conf` around lines 30 - 41, The allow/deny IP access directives in the location /inngest/ block are placed after proxy_pass; move the allow 10.0.0.0/8, allow 172.16.0.0/12, allow 192.168.0.0/16 and deny all lines to appear before the proxy_pass http://inngest; (and before proxy_set_header lines if you prefer) so access control for the location /inngest/ is declared prior to proxying; update the nginx.conf location /inngest/ block accordingly.
43-49: MissingX-Forwarded-ForandX-Forwarded-Protoheaders.Other location blocks include these headers for proper client IP and protocol forwarding. The
/api/inngestendpoint should be consistent, especially if the server logs or validates request origins.🔧 Suggested fix
location /api/inngest { proxy_pass http://betterbase_server; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; proxy_read_timeout 300s; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/nginx/nginx.conf` around lines 43 - 49, The /api/inngest location block is missing X-Forwarded-For and X-Forwarded-Proto headers; update the location /api/inngest configuration to set proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for and proxy_set_header X-Forwarded-Proto $scheme (in addition to the existing Host and X-Real-IP lines) so client IP and original protocol are forwarded consistently with other location blocks.packages/server/src/lib/webhook-dispatcher.ts (1)
33-48: Consider removing redundantsecret: nullfield from event payload.Since the comment at lines 9-11 and the
deliverWebhookfunction both confirm the secret is looked up at delivery time, includingsecret: nullin the event payload is redundant. It could be removed to simplify the event schema.🔧 Suggested simplification
webhooks.map((webhook: any) => ({ name: "betterbase/webhook.deliver" as const, data: { webhookId: webhook.id, webhookName: webhook.name, url: webhook.url, - secret: null, // Secret looked up at delivery time for security eventType, tableName, payload: record, attempt: 1, }, })),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/webhook-dispatcher.ts` around lines 33 - 48, The event payload sent via inngest.send (map over webhooks) unnecessarily includes "secret: null"; remove that property from the data object passed to the "betterbase/webhook.deliver" events and update any callers or type definitions if needed so the payload no longer expects a secret; ensure deliverWebhook (the consumer of "betterbase/webhook.deliver") continues to look up the secret at delivery time and that the event schema reflects the simplified shape (webhookId, webhookName, url, eventType, tableName, payload, attempt).apps/dashboard/src/lib/inngest-client.ts (1)
51-51: Consider defining return types instead ofany.
getRunandgetJobsuseany, which bypasses type checking. Define interfaces for these responses to catch integration bugs at compile time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/inngest-client.ts` at line 51, getRun and getJobs are returning fetchInngest<any>, which disables type checking; define proper response interfaces (e.g., RunResponse, JobResponse or JobsList) that match the API payload and update the signatures to getRun: (runId: string) => fetchInngest<RunResponse>(`${API_BASE}/runs/${runId}`) and getJobs: (...) => fetchInngest<JobsListResponse>(...), and update any consuming code to use those types so TypeScript can validate fields; create the interfaces near the top of the file or in a shared types module and use them as the generic parameter to fetchInngest instead of any.docker-compose.self-hosted.yml (1)
58-74: Consider pinning Inngest image version instead of using:latestfor reproducibility.While Inngest's official documentation recommends using the latest version, pinning to a specific version in production environments prevents unexpected behavior from breaking changes. The current stable version is v1.17.5.
🔧 Suggested fix (if pinning is preferred)
inngest: - image: inngest/inngest:latest + image: inngest/inngest:v1.17.5 container_name: betterbase-inngest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.self-hosted.yml` around lines 58 - 74, The Docker Compose service "inngest" currently uses image: inngest/inngest:latest which can introduce breaking changes; update the "inngest" service to pin a stable version (e.g., change image to inngest/inngest:v1.17.5) or replace with a controlled image tag via an env variable so the inngest service/container name and command remain unchanged while ensuring reproducible deployments.BetterBase_Inngest_Dashboard_Spec.md (2)
70-87: External fetch calls lack timeout handling.The proxy routes in this spec make
fetchcalls to the Inngest API without any timeout. If the Inngest API is slow or unresponsive, these requests will hang indefinitely, potentially exhausting server resources.Consider adding
AbortControllerwith a timeout (similar to what's done in the actualinngest.tsimplementation):🛠️ Suggested pattern for all fetch calls
inngestAdminRoutes.get("/functions", async (c) => { try { const baseUrl = getInngestBaseUrl(); const headers = await getInngestHeaders(); const envId = await getInngestEnv(); const url = envId ? `${baseUrl}/v1/environments/${envId}/functions` : `${baseUrl}/v1/functions`; - const res = await fetch(url, { headers }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + try { + const res = await fetch(url, { headers, signal: controller.signal }); + // ... rest of handler + } finally { + clearTimeout(timeoutId); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterBase_Inngest_Dashboard_Spec.md` around lines 70 - 87, The fetch to the Inngest API in the inngestAdminRoutes.get("/functions", ...) handler currently has no timeout; wrap the request with an AbortController and a short timeout (e.g., 5–10s): create an AbortController, pass controller.signal to fetch, start a setTimeout that calls controller.abort() and clears on success, and catch the abort error to return a suitable 504/timeout JSON response; update code around getInngestBaseUrl/getInngestHeaders/getInngestEnv and the fetch call to use this pattern and ensure the timeout is cleared in a finally block.
672-678: Code block missing language identifier.Per markdown best practices, fenced code blocks should have a language specified. Since this is a text-based execution order diagram, consider using
textorplaintext.-``` +```text Phase 1 — Backend Routes IDG-01 Inngest API proxy routes + instance settings Phase 2 — Frontend Dashboard IDG-02 Inngest Dashboard page + routing + nav<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@BetterBase_Inngest_Dashboard_Spec.mdaround lines 672 - 678, The fenced code
block showing the execution order diagram (the block containing "Phase 1 —
Backend Routes" and "Phase 2 — Frontend Dashboard") is missing a language
identifier; update that block to use a language like text or plaintext (e.g.,
change the opening fence to ```text) so the code fence follows Markdown best
practices and renders as intended, keeping the existing content exactly the
same.</details> </blockquote></details> <details> <summary>packages/server/src/lib/inngest.ts (3)</summary><blockquote> `205-206`: **Minor: Incorrect step comment.** The comment says "Step 2" but this is actually the third step in the function (after "resolve-secret" and "send-http-request"). ```diff - // Step 2: Persist the delivery record with response_body + // Step 3: Persist the delivery record with response_body ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/inngest.ts` around lines 205 - 206, The inline comment "Step 2: Persist the delivery record with response_body" is mislabeled—it's the third step after "resolve-secret" and "send-http-request"; update the comment to "Step 3: Persist the delivery record with response_body" (or otherwise correct the step number) near the block that persists the delivery record following the HTTP request to keep the step ordering consistent with the preceding "resolve-secret" and "send-http-request" comments. ``` </details> --- `349-353`: **Consider escaping LIKE special characters in search filter.** The search filter uses `ILIKE` with the user input wrapped in `%...%`. While this is safely parameterized (not SQL injection), characters like `%` and `_` in the search string have special meaning in LIKE patterns, potentially giving unexpected results. <details> <summary>🔧 Optional fix</summary> ```diff if (filters.search) { + // Escape LIKE special characters + const escapedSearch = filters.search.replace(/[%_\\]/g, "\\$&"); conditions.push(`(email ILIKE $${idx} OR name ILIKE $${idx})`); - params.push(`%${filters.search}%`); + params.push(`%${escapedSearch}%`); idx++; } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/inngest.ts` around lines 349 - 353, The search branch using filters.search builds ILIKE patterns without escaping LIKE metacharacters, so user input containing % or _ will be treated as wildcards; update the logic around conditions.push(...) and params.push(...) to escape %, _, and backslash in filters.search (e.g. replace occurrences of %, _, and \ with an escaped form), then wrap the escaped value with %...% when pushing to params, and include an ESCAPE '\\' on the ILIKE conditions (the same spot where conditions.push(`(email ILIKE $${idx} OR name ILIKE $${idx})`) and params.push(`%${filters.search}%`) are used) so the DB treats the escapes literally. ``` </details> --- `268-283`: **Email sending lacks timeout; consider escaping HTML.** Two concerns with the email notification: 1. **No timeout**: `transporter.sendMail()` can hang indefinitely if the SMTP server is unresponsive. Unlike the webhook path, there's no timeout protection. 2. **Potential HTML injection**: The `ruleName` and `metric` values come from event data and are interpolated directly into the HTML body. While this is admin-controlled data, it's good practice to escape HTML entities. <details> <summary>🛡️ Suggested improvements</summary> ```diff + // Set connection timeout + const transporter = nodemailer.default.createTransport({ host: smtp.host, port: smtp.port, secure: smtp.port === 465, requireTLS: smtp.use_tls, auth: { user: smtp.username, pass: smtp.password }, + connectionTimeout: 10000, + greetingTimeout: 5000, + socketTimeout: 10000, }); + const escapeHtml = (s: string) => s.replace(/[&<>"']/g, c => + ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[c] ?? c); + await transporter.sendMail({ from: `"${smtp.from_name}" <${smtp.from_email}>`, to: target, subject: `[Betterbase Alert] ${ruleName} threshold breached`, text: `Metric "${metric}" has reached ${currentValue} (threshold: ${threshold}).`, - html: `<p>Metric <strong>${metric}</strong> has reached <strong>${currentValue}</strong> (threshold: ${threshold}).</p>`, + html: `<p>Metric <strong>${escapeHtml(metric)}</strong> has reached <strong>${currentValue}</strong> (threshold: ${threshold}).</p>`, }); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/inngest.ts` around lines 268 - 283, Add a send timeout and escape interpolated values: when creating the SMTP transport (nodemailer.default.createTransport / transporter) set appropriate timeout options (e.g., connectionTimeout/greetingTimeout/socketTimeout) or wrap transporter.sendMail(...) in a Promise.race with a short timeout to ensure it can't hang; also sanitize/HTML-escape interpolated fields used in the email body and subject (ruleName, metric, currentValue, threshold and any target display) before embedding them in text/html to prevent HTML injection. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@apps/dashboard/src/lib/inngest-client.ts:
- Around line 13-36: The InngestStatus and InngestRun interfaces don't match the
server responses: update InngestStatus to make mode and url optional (so when
server returns { status: "error", error: err.message } it still validates) and
update InngestRun.output to allow null (e.g., string | null or string | null |
undefined) because the server may return output: null; adjust the interfaces
InngestStatus and InngestRun in apps/dashboard/src/lib/inngest-client.ts
accordingly.- Around line 4-11: fetchInngest currently swallows JSON parse errors by
returning {} which is then cast to T, allowing callers of fetchInngest (e.g.,
expecting InngestStatus or { functions: InngestFunction[] }) to receive an
invalid shape; update the fetchInngest implementation to not return the empty
object on res.json() failure: either rethrow the parse error (if res.ok) so
callers fail fast, or validate the parsed payload shape before casting and throw
a descriptive error when validation fails; make the change inside the
fetchInngest function and ensure the thrown error includes context (response
status and parse error) so callers can handle it safely.In
@packages/cli/src/commands/auth.ts:
- Around line 399-417: The current regex (socialRegex) is indentation-dependent
and can miss an existing socialProviders block; instead locate the
"socialProviders:" substring in authContent, then scan forward from its first
"{" using a brace counter to find the matching closing "}" (brace-aware block
extraction), extract that block (replace match[1] logic), merge
template.configCode into the providers block handling trailing commas as the
existing logic does, and replace the entire located block in authContent; update
references to socialRegex, match, existing, trimmed and newContent to use this
brace-aware extraction so formatting/indentation changes won't create a second
socialProviders block.In
@packages/cli/test/generate-crud.test.ts:
- Around line 11-19: The test file imports skip (and mock) but never uses skip,
so tests still run; update the test suite to actually skip by replacing the
top-level describe with describe.skip (use the existing describe symbol) or, if
you prefer not to skip, remove the unused skip (and mock if unused) from the
import list to silence the linter and avoid confusion—modify the import line
that currently includes "skip" and "mock" and change the test suite invocation
to describe.skip when you want to disable the whole file.In
@packages/client/src/index.ts:
- Line 34: Export the missing type so consumers can import the config type from
the package root: add a re-export for the BetterBaseReactConfig type alongside
the existing exports (i.e., export type { BetterBaseReactConfig } from
"./iac/provider") so that BetterbaseProvider, useBetterBaseContext and
BetterBaseReactConfig are all available from the main entry point.In
@packages/core/src/iac/db-context.ts:
- Around line 303-305: The delete method currently always calls _emitChange
after running _pool.query, causing false DELETE events when no row was removed;
change it to await and capture the query result (e.g., const res = await
this._pool.query(...)) and only call this._emitChange(table, "DELETE", id) if
res.rowCount > 0 so deletions emit events only when a row was actually deleted;
keep the same query and parameters, just branch on the returned rowCount in the
delete function.- Line 304: The DELETE SQL interpolates the table identifier directly in
DbContext (the method that calls this.pool.query(DELETE FROM "${this._schema}"."${table}" WHERE _id = $1, [id])`), so add validation of the
incoming table parameter against a PostgreSQL identifier regex (e.g.
/^[A-Za-z][A-Za-z0-9_]*$/ or the suggested stricter pattern) before building
the query; if the table name fails validation, throw an explicit error (e.g.
InvalidArgumentError) to prevent identifier injection, otherwise proceed to
execute the query as before.In
@packages/server/src/lib/inngest.ts:
- Around line 494-496: Replace the direct call to inngest.send(eventsToSend)
with a single idempotent step using step.run so sending occurs only once on
retries; e.g., wrap the send inside a step.run call (named like "send-events")
that awaits inngest.send(eventsToSend) and only executes when
eventsToSend.length > 0, ensuring the step name and payload reference the
existing eventsToSend and inngest.send symbols.- Around line 5-17: escapeCSVValue currently wraps values starting with
formula-injection characters in double quotes but doesn't prefix with a single
quote, so spreadsheets may still treat strings like "=SUM(A1)" as formulas;
update escapeCSVValue to detect values that start with any of = + - @ or
whitespace-control chars and prefix the escaped string with a single quote
before returning (while still doubling internal quotes and wrapping in quotes
when needed), preserving the null/undefined behavior and other
comma/quote/newline handling.In
@packages/server/src/routes/admin/project-scoped/webhooks.ts:
- Around line 100-113: Remove the secret from the event payload sent in the
retry path: in the inngest.send call that emits "betterbase/webhook.deliver"
(the block that builds data with webhookId, webhookName, url, secret, etc.),
delete the secret: webhook.secret ?? null entry so the payload no longer
contains webhook.secret; keep deliveryId, attempt, payload and tableName
intact—the worker should resolve the secret from the DB as in
webhook-dispatcher.- Around line 131-150: The test endpoint currently sends a real delivery event
("betterbase/webhook.deliver") containing the webhook.secret and a payload flag
_test: true which still causes the worker function (deliverWebhook in
inngest.ts) to perform an actual HTTP POST; remove the secret from the event
data (do not include webhook.secret) and either document that this is an
end-to-end test or, preferably, update the worker (deliverWebhook) to detect
payload._test === true and short-circuit the real POST (simulate a successful
delivery/log and return) so test deliveries never trigger side effects in
production.- Around line 87-113: The event schema for "betterbase/webhook.deliver" must
include deliveryId:string and the worker handler deliverWebhook must destructure
deliveryId from event.data and use it to update the pending delivery row instead
of inserting a new one; change the Inngest event schema (the
betterbase/webhook.deliver definition in inngest.ts) to add deliveryId: string,
update deliverWebhook to extract deliveryId and replace the INSERT into
betterbase_meta.webhook_deliveries with an UPDATE that sets status='success'
(and response/attempt_count/delivered_at as appropriate) WHERE id = deliveryId,
ensuring the query uses deliveryId to update the pending record created in the
sender.In
@README.md:
- Around line 26-31: The ASCII diagram in README.md has a broken top border and
a misaligned vertical bar: in the box row containing "Betterbase Core" replace
the erroneous extra hyphen in "──────-──────────────────────┐" with a continuous
box-drawing sequence so the top border is unbroken, and fix the following line
where the pipe for the "Database" column is misplaced (the line with "│ │
(SQLite, │ │") by moving the pipe character into column alignment so the
right-hand box border lines up with the "Database" box; search for the strings
"Betterbase Core", "Frontend", and "Database" in the README to locate and
correct the three affected ASCII-art lines.
Outside diff comments:
In@packages/client/src/iac/provider.tsx:
- Around line 34-39: The reconnection creates a fresh WebSocket but never
re-attaches handlers, so extract the socket creation and handler wiring into a
reusable helper (e.g., setupWebSocket) and call it both on initial mount and
inside ws.onclose; ensure setupWebSocket creates new WebSocket(wsUrl), assigns
wsRef.current, and attaches onopen (to setWsReady(true)), onmessage (to handle
ping/pong and other messages), and onclose (to schedule another reconnection) so
the reconnected socket restores wsReady, ping handling, and further
reconnections.- Around line 45-48: Wrap the ws.onmessage handler's JSON.parse of event.data in
a try-catch to prevent a thrown error from crashing the app; inside the catch,
handle malformed messages (e.g., log the parse error and the raw event.data or
ignore it) and return early, otherwise continue to inspect msg.type and call
ws.send as before (refer to ws.onmessage, JSON.parse(event.data), msg.type, and
ws.send).In
@packages/server/src/lib/env.ts:
- Around line 3-19: EnvSchema is missing the PORT field which causes env.PORT to
be undefined (used by Number.parseInt(env.PORT) in index.ts); update the
EnvSchema object to include PORT as a numeric/string port entry with a sensible
default (e.g., "3000") and appropriate validation (e.g.,
z.string().default("3000") or z.coerce.number().default(3000)) so that env.PORT
is always defined and Number.parseInt(env.PORT) returns a valid port; ensure the
symbol EnvSchema is updated and re-exported/used where env is derived so
index.ts's Number.parseInt(env.PORT) no longer yields NaN.In
@README.md:
- Around line 87-96: Update the fenced code block showing the project tree in
README.md to include a language specifier (e.g., "text") after the opening
triple backticks so syntax highlighting and MD040 are satisfied; locate the
block that begins with the triple backticks and contains "my-app/" and the
betterbase files (schema.ts, queries/, mutations/, actions/,
betterbase.config.ts) and change the opening fence to include the specifier.
Nitpick comments:
In@apps/dashboard/src/lib/inngest-client.ts:
- Line 51: getRun and getJobs are returning fetchInngest, which disables
type checking; define proper response interfaces (e.g., RunResponse, JobResponse
or JobsList) that match the API payload and update the signatures to getRun:
(runId: string) => fetchInngest(${API_BASE}/runs/${runId}) and
getJobs: (...) => fetchInngest(...), and update any consuming
code to use those types so TypeScript can validate fields; create the interfaces
near the top of the file or in a shared types module and use them as the generic
parameter to fetchInngest instead of any.In
@BetterBase_Inngest_Dashboard_Spec.md:
- Around line 70-87: The fetch to the Inngest API in the
inngestAdminRoutes.get("/functions", ...) handler currently has no timeout; wrap
the request with an AbortController and a short timeout (e.g., 5–10s): create an
AbortController, pass controller.signal to fetch, start a setTimeout that calls
controller.abort() and clears on success, and catch the abort error to return a
suitable 504/timeout JSON response; update code around
getInngestBaseUrl/getInngestHeaders/getInngestEnv and the fetch call to use this
pattern and ensure the timeout is cleared in a finally block.- Around line 672-678: The fenced code block showing the execution order diagram
(the block containing "Phase 1 — Backend Routes" and "Phase 2 — Frontend
Dashboard") is missing a language identifier; update that block to use a
language like text or plaintext (e.g., change the opening fence to ```text) so
the code fence follows Markdown best practices and renders as intended, keeping
the existing content exactly the same.In
@docker-compose.self-hosted.yml:
- Around line 58-74: The Docker Compose service "inngest" currently uses image:
inngest/inngest:latest which can introduce breaking changes; update the
"inngest" service to pin a stable version (e.g., change image to
inngest/inngest:v1.17.5) or replace with a controlled image tag via an env
variable so the inngest service/container name and command remain unchanged
while ensuring reproducible deployments.In
@docker/nginx/nginx.conf:
- Around line 30-41: The allow/deny IP access directives in the location
/inngest/ block are placed after proxy_pass; move the allow 10.0.0.0/8, allow
172.16.0.0/12, allow 192.168.0.0/16 and deny all lines to appear before the
proxy_pass http://inngest; (and before proxy_set_header lines if you prefer) so
access control for the location /inngest/ is declared prior to proxying; update
the nginx.conf location /inngest/ block accordingly.- Around line 43-49: The /api/inngest location block is missing X-Forwarded-For
and X-Forwarded-Proto headers; update the location /api/inngest configuration to
set proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for and
proxy_set_header X-Forwarded-Proto $scheme (in addition to the existing Host and
X-Real-IP lines) so client IP and original protocol are forwarded consistently
with other location blocks.In
@packages/server/src/lib/inngest.ts:
- Around line 205-206: The inline comment "Step 2: Persist the delivery record
with response_body" is mislabeled—it's the third step after "resolve-secret" and
"send-http-request"; update the comment to "Step 3: Persist the delivery record
with response_body" (or otherwise correct the step number) near the block that
persists the delivery record following the HTTP request to keep the step
ordering consistent with the preceding "resolve-secret" and "send-http-request"
comments.- Around line 349-353: The search branch using filters.search builds ILIKE
patterns without escaping LIKE metacharacters, so user input containing % or _
will be treated as wildcards; update the logic around conditions.push(...) and
params.push(...) to escape %, _, and backslash in filters.search (e.g. replace
occurrences of %, _, and \ with an escaped form), then wrap the escaped value
with %...% when pushing to params, and include an ESCAPE '\' on the ILIKE
conditions (the same spot where conditions.push((email ILIKE $${idx} OR name ILIKE $${idx})) and params.push(%${filters.search}%) are used) so the DB
treats the escapes literally.- Around line 268-283: Add a send timeout and escape interpolated values: when
creating the SMTP transport (nodemailer.default.createTransport / transporter)
set appropriate timeout options (e.g.,
connectionTimeout/greetingTimeout/socketTimeout) or wrap
transporter.sendMail(...) in a Promise.race with a short timeout to ensure it
can't hang; also sanitize/HTML-escape interpolated fields used in the email body
and subject (ruleName, metric, currentValue, threshold and any target display)
before embedding them in text/html to prevent HTML injection.In
@packages/server/src/lib/webhook-dispatcher.ts:
- Around line 33-48: The event payload sent via inngest.send (map over webhooks)
unnecessarily includes "secret: null"; remove that property from the data object
passed to the "betterbase/webhook.deliver" events and update any callers or type
definitions if needed so the payload no longer expects a secret; ensure
deliverWebhook (the consumer of "betterbase/webhook.deliver") continues to look
up the secret at delivery time and that the event schema reflects the simplified
shape (webhookId, webhookName, url, eventType, tableName, payload, attempt).</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `e7c0739e-921c-4ec2-a720-a2adfb85e9fa` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 9382d293d753e29a5a1bd6cc745e523c7715631a and fbcf2836b6df4d7e75a0c0e3fc54f23abe2c387f. </details> <details> <summary>📒 Files selected for processing (24)</summary> * `BetterBase_Inngest_Dashboard_Spec.md` * `CODEBASE_MAP.md` * `README.md` * `apps/dashboard/src/lib/inngest-client.ts` * `apps/dashboard/src/pages/settings/InngestDashboardPage.tsx` * `docker-compose.dev.yml` * `docker-compose.self-hosted.yml` * `docker/nginx/nginx.conf` * `packages/cli/src/commands/auth.ts` * `packages/cli/test/generate-crud.test.ts` * `packages/client/src/iac/index.ts` * `packages/client/src/iac/provider.tsx` * `packages/client/src/index.ts` * `packages/core/src/iac/db-context.ts` * `packages/server/migrations/014_inngest_support.sql` * `packages/server/src/index.ts` * `packages/server/src/lib/env.ts` * `packages/server/src/lib/inngest.ts` * `packages/server/src/lib/webhook-dispatcher.ts` * `packages/server/src/routes/admin/inngest.ts` * `packages/server/src/routes/admin/project-scoped/webhooks.ts` * `packages/server/test/inngest.test.ts` * `packages/server/test/routes.test.ts` * `packages/server/tsconfig.json` </details> <details> <summary>✅ Files skipped from review due to trivial changes (3)</summary> * docker-compose.dev.yml * packages/server/test/routes.test.ts * packages/server/migrations/014_inngest_support.sql </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (5)</summary> * packages/server/tsconfig.json * packages/server/src/index.ts * packages/server/test/inngest.test.ts * apps/dashboard/src/pages/settings/InngestDashboardPage.tsx * packages/server/src/routes/admin/inngest.ts </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+4
to
+11
| async function fetchInngest<T>(url: string, options?: RequestInit): Promise<T> { | ||
| const res = await fetch(url, options); | ||
| const data = await res.json().catch(() => ({})); | ||
| if (!res.ok) { | ||
| throw new Error(data.error ?? `HTTP ${res.status}`); | ||
| } | ||
| return data as T; | ||
| } |
There was a problem hiding this comment.
Type safety issue: fetchInngest can return {} instead of expected type T.
When res.json() throws (e.g., non-JSON response body), the fallback {} is cast to T, which silently returns an incorrect type. Callers expecting InngestStatus or { functions: InngestFunction[] } will receive an empty object instead.
Consider throwing when JSON parsing fails on a successful response, or validate the shape before casting:
🛡️ Proposed fix
async function fetchInngest<T>(url: string, options?: RequestInit): Promise<T> {
const res = await fetch(url, options);
- const data = await res.json().catch(() => ({}));
+ const data = await res.json().catch(() => {
+ if (res.ok) throw new Error(`Invalid JSON response from ${url}`);
+ return {};
+ });
if (!res.ok) {
throw new Error(data.error ?? `HTTP ${res.status}`);
}
return data as T;
}📝 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.
Suggested change
| async function fetchInngest<T>(url: string, options?: RequestInit): Promise<T> { | |
| const res = await fetch(url, options); | |
| const data = await res.json().catch(() => ({})); | |
| if (!res.ok) { | |
| throw new Error(data.error ?? `HTTP ${res.status}`); | |
| } | |
| return data as T; | |
| } | |
| async function fetchInngest<T>(url: string, options?: RequestInit): Promise<T> { | |
| const res = await fetch(url, options); | |
| const data = await res.json().catch(() => { | |
| if (res.ok) throw new Error(`Invalid JSON response from ${url}`); | |
| return {}; | |
| }); | |
| if (!res.ok) { | |
| throw new Error(data.error ?? `HTTP ${res.status}`); | |
| } | |
| return data as T; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/inngest-client.ts` around lines 4 - 11, fetchInngest
currently swallows JSON parse errors by returning {} which is then cast to T,
allowing callers of fetchInngest (e.g., expecting InngestStatus or { functions:
InngestFunction[] }) to receive an invalid shape; update the fetchInngest<T>
implementation to not return the empty object on res.json() failure: either
rethrow the parse error (if res.ok) so callers fail fast, or validate the parsed
payload shape before casting and throw a descriptive error when validation
fails; make the change inside the fetchInngest function and ensure the thrown
error includes context (response status and parse error) so callers can handle
it safely.
Comment on lines
+13
to
+36
| export interface InngestStatus { | ||
| status: "connected" | "error"; | ||
| mode: "self-hosted" | "cloud"; | ||
| url: string; | ||
| error?: string; | ||
| } | ||
|
|
||
| export interface InngestFunction { | ||
| id: string; | ||
| name: string; | ||
| status: "active" | "paused"; | ||
| createdAt: string; | ||
| triggers: { type: string; event?: string; cron?: string }[]; | ||
| } | ||
|
|
||
| export interface InngestRun { | ||
| id: string; | ||
| functionId: string; | ||
| status: "pending" | "running" | "complete" | "failed"; | ||
| startedAt: string; | ||
| endedAt?: string; | ||
| output?: string; | ||
| error?: string; | ||
| } |
There was a problem hiding this comment.
Interface definitions don't match server responses.
Based on packages/server/src/routes/admin/inngest.ts:
-
InngestStatus: The error path (lines 77-80) returns{ status: "error", error: err.message }withoutmodeorurl, but your interface marksmodeas required. -
InngestRun.output: Server setsoutput: r.response_code ? \HTTP ${r.response_code}...` : null(line 151), sooutputcan benull, not juststring | undefined`.
🔧 Suggested interface fixes
export interface InngestStatus {
status: "connected" | "error";
- mode: "self-hosted" | "cloud";
- url: string;
+ mode?: "self-hosted" | "cloud";
+ url?: string;
error?: string;
}
export interface InngestRun {
id: string;
functionId: string;
status: "pending" | "running" | "complete" | "failed";
startedAt: string;
endedAt?: string;
- output?: string;
+ output?: string | null;
error?: string;
}📝 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.
Suggested change
| export interface InngestStatus { | |
| status: "connected" | "error"; | |
| mode: "self-hosted" | "cloud"; | |
| url: string; | |
| error?: string; | |
| } | |
| export interface InngestFunction { | |
| id: string; | |
| name: string; | |
| status: "active" | "paused"; | |
| createdAt: string; | |
| triggers: { type: string; event?: string; cron?: string }[]; | |
| } | |
| export interface InngestRun { | |
| id: string; | |
| functionId: string; | |
| status: "pending" | "running" | "complete" | "failed"; | |
| startedAt: string; | |
| endedAt?: string; | |
| output?: string; | |
| error?: string; | |
| } | |
| export interface InngestStatus { | |
| status: "connected" | "error"; | |
| mode?: "self-hosted" | "cloud"; | |
| url?: string; | |
| error?: string; | |
| } | |
| export interface InngestFunction { | |
| id: string; | |
| name: string; | |
| status: "active" | "paused"; | |
| createdAt: string; | |
| triggers: { type: string; event?: string; cron?: string }[]; | |
| } | |
| export interface InngestRun { | |
| id: string; | |
| functionId: string; | |
| status: "pending" | "running" | "complete" | "failed"; | |
| startedAt: string; | |
| endedAt?: string; | |
| output?: string | null; | |
| error?: string; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/inngest-client.ts` around lines 13 - 36, The
InngestStatus and InngestRun interfaces don't match the server responses: update
InngestStatus to make mode and url optional (so when server returns { status:
"error", error: err.message } it still validates) and update InngestRun.output
to allow null (e.g., string | null or string | null | undefined) because the
server may return output: null; adjust the interfaces InngestStatus and
InngestRun in apps/dashboard/src/lib/inngest-client.ts accordingly.
Comment on lines
+399
to
+417
| const socialRegex = /socialProviders:\s*\{([\s\S]*?)\n {2}\}/; | ||
| const match = authContent.match(socialRegex); | ||
|
|
||
| if (match) { | ||
| // Add to existing socialProviders - find the closing brace of the last provider | ||
| const existing = match[1]; | ||
|
|
||
| // Check if existing content ends with a closing brace (provider object) | ||
| const trimmed = existing.trim(); | ||
| let newContent: string; | ||
|
|
||
| if (trimmed.endsWith("}")) { | ||
| // Add comma and new provider | ||
| newContent = `${trimmed.slice(0, -1)},\n${template.configCode}\n }`; | ||
| } else { | ||
| newContent = `${trimmed}\n${template.configCode}\n }`; | ||
| } | ||
|
|
||
| authContent = authContent.replace( | ||
| socialRegex, | ||
| `socialProviders: {\n${newContent}`, | ||
| ); | ||
| authContent = authContent.replace(socialRegex, `socialProviders: {\n${newContent}`); |
There was a problem hiding this comment.
socialProviders parsing is indentation-coupled and can mis-detect existing config.
At Line 399, the regex requires an exact \n {2}\} closing pattern. If src/auth/index.ts is formatted with different indentation (e.g., 4 spaces, tabs, or formatter changes), match can fail and the code at Line 418+ will create a second socialProviders block instead of updating the existing one.
💡 Suggested fix (brace-aware block extraction, not whitespace regex)
- const socialRegex = /socialProviders:\s*\{([\s\S]*?)\n {2}\}/;
- const match = authContent.match(socialRegex);
+ const socialStart = authContent.indexOf("socialProviders:");
+ let socialBlock:
+ | { start: number; openBrace: number; closeBrace: number; body: string }
+ | null = null;
+
+ if (socialStart !== -1) {
+ const openBrace = authContent.indexOf("{", socialStart);
+ let depth = 0;
+ for (let i = openBrace; i < authContent.length; i++) {
+ const ch = authContent[i];
+ if (ch === "{") depth++;
+ if (ch === "}") depth--;
+ if (depth === 0) {
+ socialBlock = {
+ start: socialStart,
+ openBrace,
+ closeBrace: i,
+ body: authContent.slice(openBrace + 1, i),
+ };
+ break;
+ }
+ }
+ }
- if (match) {
- const existing = match[1];
+ if (socialBlock) {
+ const existing = socialBlock.body;
const trimmed = existing.trim();
let newContent: string;
if (trimmed.endsWith("}")) {
newContent = `${trimmed.slice(0, -1)},\n${template.configCode}\n }`;
} else {
newContent = `${trimmed}\n${template.configCode}\n }`;
}
- authContent = authContent.replace(socialRegex, `socialProviders: {\n${newContent}`);
+ authContent =
+ authContent.slice(0, socialBlock.start) +
+ `socialProviders: {\n${newContent}` +
+ authContent.slice(socialBlock.closeBrace + 1);
} else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/auth.ts` around lines 399 - 417, The current regex
(socialRegex) is indentation-dependent and can miss an existing socialProviders
block; instead locate the "socialProviders:" substring in authContent, then scan
forward from its first "{" using a brace counter to find the matching closing
"}" (brace-aware block extraction), extract that block (replace match[1] logic),
merge template.configCode into the providers block handling trailing commas as
the existing logic does, and replace the entire located block in authContent;
update references to socialRegex, match, existing, trimmed and newContent to use
this brace-aware extraction so formatting/indentation changes won't create a
second socialProviders block.
|
|
||
| // IaC exports | ||
| export { BetterbaseProvider, useBetterBaseContext, type BetterBaseConfig } from "./iac/provider"; | ||
| export { BetterbaseProvider, useBetterBaseContext } from "./iac/provider"; |
There was a problem hiding this comment.
Missing BetterBaseReactConfig type export.
BetterbaseProvider is exported here but its config type BetterBaseReactConfig is not. Consumers who import from the main package entry point (import { BetterbaseProvider } from "...") cannot access the type needed to correctly type their config props without reaching into the internal ./iac subpath.
Consider also exporting BetterBaseReactConfig for API consistency:
Proposed fix
// IaC exports
-export { BetterbaseProvider, useBetterBaseContext } from "./iac/provider";
+export { BetterbaseProvider, useBetterBaseContext, type BetterBaseReactConfig } from "./iac/provider";📝 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.
Suggested change
| export { BetterbaseProvider, useBetterBaseContext } from "./iac/provider"; | |
| export { BetterbaseProvider, useBetterBaseContext, type BetterBaseReactConfig } from "./iac/provider"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/src/index.ts` at line 34, Export the missing type so
consumers can import the config type from the package root: add a re-export for
the BetterBaseReactConfig type alongside the existing exports (i.e., export type
{ BetterBaseReactConfig } from "./iac/provider") so that BetterbaseProvider,
useBetterBaseContext and BetterBaseReactConfig are all available from the main
entry point.
Comment on lines
+494
to
+496
| if (eventsToSend.length > 0) { | ||
| await inngest.send(eventsToSend); | ||
| } |
There was a problem hiding this comment.
Wrap inngest.send() in a step for idempotency.
Calling inngest.send() outside of a step.run() means that if the function fails after sending events (e.g., before returning), the retry will send duplicate events. Wrapping the send in a step ensures it only executes once.
🔒 Proposed fix
- if (eventsToSend.length > 0) {
- await inngest.send(eventsToSend);
- }
+ if (eventsToSend.length > 0) {
+ await step.run("send-evaluation-events", async () => {
+ await inngest.send(eventsToSend);
+ });
+ }📝 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.
Suggested change
| if (eventsToSend.length > 0) { | |
| await inngest.send(eventsToSend); | |
| } | |
| if (eventsToSend.length > 0) { | |
| await step.run("send-evaluation-events", async () => { | |
| await inngest.send(eventsToSend); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/lib/inngest.ts` around lines 494 - 496, Replace the
direct call to inngest.send(eventsToSend) with a single idempotent step using
step.run so sending occurs only once on retries; e.g., wrap the send inside a
step.run call (named like "send-events") that awaits inngest.send(eventsToSend)
and only executes when eventsToSend.length > 0, ensuring the step name and
payload reference the existing eventsToSend and inngest.send symbols.
Comment on lines
+87
to
+113
| // Insert a pending delivery record FIRST so we can track it | ||
| // Then include the delivery ID in the event for the worker to update | ||
| const { rows: newDelivery } = await pool.query( | ||
| `INSERT INTO betterbase_meta.webhook_deliveries | ||
| (webhook_id, event_type, payload, status, response_code, response_body, duration_ms, delivered_at) | ||
| VALUES ($1, 'RETRY', $2, $3, $4, $5, $6, NOW())`, | ||
| [webhook.id, JSON.stringify(syntheticPayload), status, responseCode, responseBody, duration], | ||
| (webhook_id, event_type, payload, status, attempt_count) | ||
| VALUES ($1, 'RETRY', $2, 'pending', $3) | ||
| RETURNING id`, | ||
| [webhook.id, JSON.stringify(payload), attempt], | ||
| ); | ||
|
|
||
| const deliveryId = newDelivery[0].id; | ||
|
|
||
| // Send event to Inngest with delivery ID included | ||
| await inngest.send({ | ||
| name: "betterbase/webhook.deliver", | ||
| data: { | ||
| webhookId: webhook.id, | ||
| webhookName: webhook.name, | ||
| url: webhook.url, | ||
| secret: webhook.secret ?? null, | ||
| eventType: "RETRY", | ||
| tableName: webhook.table_name, | ||
| payload, | ||
| attempt, | ||
| deliveryId, // Include so worker can update the specific row | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if deliverWebhook uses deliveryId to UPDATE existing records
rg -n 'deliveryId|UPDATE.*webhook_deliveries' packages/server/src/lib/inngest.tsRepository: weroperking/Betterbase
Length of output: 48
🏁 Script executed:
rg -n 'function deliverWebhook|const deliverWebhook|deliverWebhook\s*=' packages/server/src/lib/inngest.ts -A 30Repository: weroperking/Betterbase
Length of output: 999
🏁 Script executed:
rg -n 'webhook_deliveries' packages/server/src/lib/inngest.tsRepository: weroperking/Betterbase
Length of output: 121
🏁 Script executed:
cat packages/server/src/lib/inngest.ts | head -250Repository: weroperking/Betterbase
Length of output: 7408
🏁 Script executed:
rg -n 'log-failed-delivery|on.*error|catch' packages/server/src/lib/inngest.ts -A 15Repository: weroperking/Betterbase
Length of output: 1725
🏁 Script executed:
sed -n '200,280p' packages/server/src/lib/inngest.tsRepository: weroperking/Betterbase
Length of output: 2564
Event schema missing deliveryId field and worker creates duplicate records instead of updating.
The betterbase/webhook.deliver event schema in packages/server/src/lib/inngest.ts (lines 40-54) does not define a deliveryId field, even though packages/server/src/routes/admin/project-scoped/webhooks.ts (lines 89-95) attempts to pass it when sending the event.
Consequently:
- The
deliverWebhookfunction cannot destructuredeliveryIdfromevent.data - The worker always inserts a new record with status
'success'(line 212) instead of updating the pending record created at line 89-95 - This leaves orphaned pending records that are never resolved
The fix requires:
- Add
deliveryId: stringto the event schema definition - Update
deliverWebhookto destructure and usedeliveryId - Replace the
INSERTwith anUPDATEstatement usingdeliveryIdto mark the pending record as delivered
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/project-scoped/webhooks.ts` around lines 87
- 113, The event schema for "betterbase/webhook.deliver" must include
deliveryId:string and the worker handler deliverWebhook must destructure
deliveryId from event.data and use it to update the pending delivery row instead
of inserting a new one; change the Inngest event schema (the
betterbase/webhook.deliver definition in inngest.ts) to add deliveryId: string,
update deliverWebhook to extract deliveryId and replace the INSERT into
betterbase_meta.webhook_deliveries with an UPDATE that sets status='success'
(and response/attempt_count/delivered_at as appropriate) WHERE id = deliveryId,
ensuring the query uses deliveryId to update the pending record created in the
sender.
Comment on lines
+100
to
+113
| await inngest.send({ | ||
| name: "betterbase/webhook.deliver", | ||
| data: { | ||
| webhookId: webhook.id, | ||
| webhookName: webhook.name, | ||
| url: webhook.url, | ||
| secret: webhook.secret ?? null, | ||
| eventType: "RETRY", | ||
| tableName: webhook.table_name, | ||
| payload, | ||
| attempt, | ||
| deliveryId, // Include so worker can update the specific row | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Security: secret is included in event payload, contradicting the security pattern.
webhook-dispatcher.ts explicitly excludes the secret from event payloads (lines 9-11, 21-23), but this retry handler still includes secret: webhook.secret ?? null. For consistency and security, remove the secret from the payload—the worker already resolves it from the database.
🔒 Proposed fix
await inngest.send({
name: "betterbase/webhook.deliver",
data: {
webhookId: webhook.id,
webhookName: webhook.name,
url: webhook.url,
- secret: webhook.secret ?? null,
eventType: "RETRY",
tableName: webhook.table_name,
payload,
attempt,
deliveryId,
},
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/project-scoped/webhooks.ts` around lines 100
- 113, Remove the secret from the event payload sent in the retry path: in the
inngest.send call that emits "betterbase/webhook.deliver" (the block that builds
data with webhookId, webhookName, url, secret, etc.), delete the secret:
webhook.secret ?? null entry so the payload no longer contains webhook.secret;
keep deliveryId, attempt, payload and tableName intact—the worker should resolve
the secret from the DB as in webhook-dispatcher.
Comment on lines
+131
to
+150
|
|
||
| // Test deliveries go through Inngest too — provides identical trace visibility | ||
| await inngest.send({ | ||
| name: "betterbase/webhook.deliver", | ||
| data: { | ||
| webhookId: webhook.id, | ||
| webhookName: webhook.name, | ||
| url: webhook.url, | ||
| secret: webhook.secret ?? null, | ||
| eventType: "TEST", | ||
| tableName: webhook.table_name, | ||
| payload: { id: "test-123", example: "data", _test: true }, | ||
| attempt: 1, | ||
| }, | ||
| }); | ||
|
|
||
| return c.json({ | ||
| success: true, | ||
| message: "Test event sent to Inngest. Check the Inngest dashboard for delivery trace.", | ||
| }); |
There was a problem hiding this comment.
Test endpoint sends real HTTP request to webhook URL.
The _test: true flag in the payload (line 142) suggests this might be intended as a dry-run, but deliverWebhook (per context snippet at inngest.ts:98-180) unconditionally performs the HTTP POST. If the webhook URL is a production endpoint, this will trigger real side effects.
If this is intentional (end-to-end testing), consider documenting this behavior. If a dry-run is desired, the worker needs to check for _test: true.
Also, remove secret from the payload per the same security pattern as retry:
- secret: webhook.secret ?? null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/project-scoped/webhooks.ts` around lines 131
- 150, The test endpoint currently sends a real delivery event
("betterbase/webhook.deliver") containing the webhook.secret and a payload flag
_test: true which still causes the worker function (deliverWebhook in
inngest.ts) to perform an actual HTTP POST; remove the secret from the event
data (do not include webhook.secret) and either document that this is an
end-to-end test or, preferably, update the worker (deliverWebhook) to detect
payload._test === true and short-circuit the real POST (simulate a successful
delivery/log and return) so test deliveries never trigger side effects in
production.
Comment on lines
+26
to
31
| │ ┌─────────────────┐ ┌──────-──────────────────────┐ ┌────────────┐ │ | ||
| │ │ Frontend │ │ Betterbase Core │ │ Database │ │ | ||
| │ │ (React, │─────▶ | │───▶│ (SQLite, │ │ | ||
| │ │ Vue, │ │ Auth │ Realtime │ Storage │ │ Postgres)│ │ | ||
| │ │ Mobile) │ │ RLS │ Vector │ Functions│ └────────────┘ │ | ||
| │ └─────────────────┘ └────────────────────────────┘ │ |
There was a problem hiding this comment.
ASCII diagram has formatting issues.
Line 26 has an extra hyphen breaking the box border, and line 28 has a misplaced pipe character:
🔧 Suggested fixes
-│ ┌─────────────────┐ ┌──────-──────────────────────┐ ┌────────────┐ │
+│ ┌─────────────────┐ ┌─────────────────────────────┐ ┌────────────┐ │
│ │ Frontend │ │ Betterbase Core │ │ Database │ │
-│ │ (React, │─────▶ | │───▶│ (SQLite, │
+│ │ (React, │─────▶ │ │───▶│ (SQLite, │🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 26 - 31, The ASCII diagram in README.md has a broken
top border and a misaligned vertical bar: in the box row containing "Betterbase
Core" replace the erroneous extra hyphen in "──────-──────────────────────┐"
with a continuous box-drawing sequence so the top border is unbroken, and fix
the following line where the pipe for the "Database" column is misplaced (the
line with "│ │ (SQLite, │ │") by moving the pipe character into column
alignment so the right-hand box border lines up with the "Database" box; search
for the strings "Betterbase Core", "Frontend", and "Database" in the README to
locate and correct the three affected ASCII-art lines.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Cannot open a stacked PR on a forked repository. |
- Add missing delete() method to DatabaseWriter (core) - Fix duplicate BetterBaseConfig export in client package - Add error handling for bun add better-auth (cli auth command) - Skip generate-crud tests (Bun 1.3.x mock.module() framework issue) - Install inngest@3.52.7 and fix server typecheck errors (62 errors resolved) - Add skipLibCheck to server tsconfig - Fix mock typing with proper as any[] casts in server tests - Fix env.PORT access pattern and inngest function type assertions - Fix coderabbit.yaml tone_instructions character limit (250 max)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
packages/server/src/routes/admin/inngest.ts (2)
42-49:⚠️ Potential issue | 🟠 MajorPreserve upstream HTTP status instead of turning every Inngest error into a 500.
fetchWithErrorCheck()dropsres.status, and these handlers then hardcode500, so upstream 401/404/429 responses from Inngest are misreported as internal errors. Return the upstream status/payload from the helper and pass them through in the catch blocks.Also applies to: 115-116, 168-169, 213-214, 332-333
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/routes/admin/inngest.ts` around lines 42 - 49, The helper fetchWithErrorCheck currently discards the response status causing all upstream HTTP errors to be surfaced as 500s; update fetchWithErrorCheck to return or throw including the original res.status and parsed body (e.g., return { status: res.status, body: data } or throw an error object with status and body) so callers can inspect the upstream status, then update the Inngest-related catch/response handlers (the handlers that currently hardcode 500 at the call sites referenced around lines 115-116, 168-169, 213-214, 332-333) to forward the upstream status and payload to the HTTP response instead of always returning 500. Ensure unique symbols mentioned: fetchWithErrorCheck and the Inngest handlers' catch blocks are changed to read status/body from the helper result/error and use res.status(status).json(body).
134-156:⚠️ Potential issue | 🟠 MajorSelf-hosted run lookup is still keyed to the wrong identifier.
/functions/:id/runsreceives an Inngest function id likedeliver-webhook, but this query filtersbetterbase_meta.webhook_deliveries.webhook_id. Since/functionsabove returns function ids, non-webhook workflows can never return runs here, and the run-detail response below reports a webhook id asfunctionId.Also applies to: 193-197
packages/server/test/inngest.test.ts (1)
11-26:⚠️ Potential issue | 🟠 MajorThis suite is still replacing the module under test with hand-written exports.
After Line 11, every
import("../src/lib/inngest")resolves to this stub, so the export, send, DB, and default-config assertions can stay green even ifpackages/server/src/lib/inngest.tsor the real env wiring regresses. Mock the external Inngest/db dependencies, then import the actual module under test.Also applies to: 45-243, 246-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/inngest.test.ts` around lines 11 - 26, The test currently replaces the entire module under test by calling mock.module("../src/lib/inngest", ...) with hand-written exports (mockInngestCreateFunction, mockInngestSend, deliverWebhook, etc.), which masks regressions in packages/server/src/lib/inngest.ts; instead, mock only the external dependencies (e.g., the DB client and any external network/send functions) by stubbing those specific imports (mock the lower-level clients/functions used by the module) and then import the real module under test via a dynamic import (await import("../src/lib/inngest")) so that symbols like createFunction, send, allInngestFunctions, deliverWebhook, evaluateNotificationRule, exportProjectUsers, and pollNotificationRules come from the actual implementation while external calls are faked by the mocks.packages/server/test/routes.test.ts (1)
81-154:⚠️ Potential issue | 🟠 MajorThese specs still don't exercise the real Inngest code.
They only assert local lambdas and string literals, so regressions in
packages/server/src/lib/inngest.tsor the notification routes won't fail this suite. Line 124 is already checking the wrong concurrency key: the real function config uses the literal pathevent.data.webhookId, not a dynamicevent.data.${webhookId}string.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/routes.test.ts` around lines 81 - 154, The tests currently use local lambdas and literals instead of exercising the real Inngest implementation and also assert the wrong concurrency key; replace the inline helpers (evaluateThreshold, calculateNextAttempt, parseCronExpression, calculateErrorRate, signature generation and concurrencyKey string) with imports of the actual functions from the Inngest module used by your routes (use the real threshold/evaluation, retry calculation, cron parsing, signature/HMAC helper and concurrency-key function from the Inngest library), and update the concurrency key expectation to the literal path "event.data.webhookId" (not a dynamic `event.data.${webhookId}`) so the tests exercise the real code in your notification routes and inngest logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/test/generate-crud.test.ts`:
- Around line 79-81: The whole suite is currently disabled by describe.skip
which removes coverage for runGenerateCrudCommand; change this by removing
describe.skip and instead keep a single minimal, non-flaky smoke test that calls
runGenerateCrudCommand and asserts basic success, while marking the
unstable/flaky cases as test.skip. Specifically, in generate-crud.test.ts:
replace describe.skip("runGenerateCrudCommand", ...) with
describe("runGenerateCrudCommand", ...), add one compact test (e.g., "smoke:
generates routes") that invokes runGenerateCrudCommand with the simplest valid
inputs and a clean mocked environment and asserts expected side-effects, and
move the existing flaky tests to individually skipped tests (test.skip(...)) and
ensure any use of mock.module() is isolated and cleaned up in afterEach/afterAll
to avoid global mock state corruption.
In `@packages/server/src/routes/admin/inngest.ts`:
- Around line 96-104: The current mapping in the allInngestFunctions
transformation fabricates trigger metadata by splitting fnAny.id (in the map
that builds id/name/status/createdAt/triggers), which produces incorrect event
names and misclassifies cron pollers; instead, import and use the explicit
function metadata map from ../../lib/inngest (or a provided mapping object) to
populate the triggers and capability fields for each function (lookup by
fnAny.id and fall back to a safe default like an empty triggers array and
correct type), replacing the split-based triggers construction so the dashboard
shows accurate trigger and capability data.
- Around line 259-271: The "poll-notification-rules" test is exercising the
evaluate workflow by sending eventName "betterbase/notification.evaluate" so the
poller isn't tested; update the admin test logic for the
"poll-notification-rules" entry (and the similar entry later) to either (A)
disallow/skip manual "Test" for cron-triggered functions (detect by key
"poll-notification-rules" or by cron metadata) and return a clear message, or
(B) add a dedicated poller test path that sends the correct poll event (e.g., a
"betterbase/notification.poll" or whatever the poller expects) and/or invokes
the poller handler directly instead of calling evaluateNotificationRule; ensure
the change references the "poll-notification-rules" entry and the
evaluateNotificationRule/poller handler so the admin Test actually exercises the
polling behavior.
---
Duplicate comments:
In `@packages/server/src/routes/admin/inngest.ts`:
- Around line 42-49: The helper fetchWithErrorCheck currently discards the
response status causing all upstream HTTP errors to be surfaced as 500s; update
fetchWithErrorCheck to return or throw including the original res.status and
parsed body (e.g., return { status: res.status, body: data } or throw an error
object with status and body) so callers can inspect the upstream status, then
update the Inngest-related catch/response handlers (the handlers that currently
hardcode 500 at the call sites referenced around lines 115-116, 168-169,
213-214, 332-333) to forward the upstream status and payload to the HTTP
response instead of always returning 500. Ensure unique symbols mentioned:
fetchWithErrorCheck and the Inngest handlers' catch blocks are changed to read
status/body from the helper result/error and use res.status(status).json(body).
In `@packages/server/test/inngest.test.ts`:
- Around line 11-26: The test currently replaces the entire module under test by
calling mock.module("../src/lib/inngest", ...) with hand-written exports
(mockInngestCreateFunction, mockInngestSend, deliverWebhook, etc.), which masks
regressions in packages/server/src/lib/inngest.ts; instead, mock only the
external dependencies (e.g., the DB client and any external network/send
functions) by stubbing those specific imports (mock the lower-level
clients/functions used by the module) and then import the real module under test
via a dynamic import (await import("../src/lib/inngest")) so that symbols like
createFunction, send, allInngestFunctions, deliverWebhook,
evaluateNotificationRule, exportProjectUsers, and pollNotificationRules come
from the actual implementation while external calls are faked by the mocks.
In `@packages/server/test/routes.test.ts`:
- Around line 81-154: The tests currently use local lambdas and literals instead
of exercising the real Inngest implementation and also assert the wrong
concurrency key; replace the inline helpers (evaluateThreshold,
calculateNextAttempt, parseCronExpression, calculateErrorRate, signature
generation and concurrencyKey string) with imports of the actual functions from
the Inngest module used by your routes (use the real threshold/evaluation, retry
calculation, cron parsing, signature/HMAC helper and concurrency-key function
from the Inngest library), and update the concurrency key expectation to the
literal path "event.data.webhookId" (not a dynamic `event.data.${webhookId}`) so
the tests exercise the real code in your notification routes and inngest logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b3092e3-6bbb-4698-a1d3-59ca5e3abc24
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.coderabbit.yamlpackages/cli/test/generate-crud.test.tspackages/server/package.jsonpackages/server/src/index.tspackages/server/src/routes/admin/inngest.tspackages/server/test/api-keys.test.tspackages/server/test/audit.test.tspackages/server/test/iac-routes.test.tspackages/server/test/inngest.test.tspackages/server/test/instance.test.tspackages/server/test/project-scoped.test.tspackages/server/test/routes.test.tspackages/server/tsconfig.json
✅ Files skipped from review due to trivial changes (5)
- .coderabbit.yaml
- packages/server/test/api-keys.test.ts
- packages/server/test/instance.test.ts
- packages/server/tsconfig.json
- packages/server/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/src/index.ts
Comment on lines
+79
to
+81
| // Skipped: generate CRUD tests have framework issues with mock.module() in Bun 1.3.x | ||
| // This is a known limitation where global mock state can corrupt subsequent test runs. | ||
| describe.skip("runGenerateCrudCommand", () => { |
There was a problem hiding this comment.
Skipping the full suite removes coverage for critical CRUD generation behavior.
At Line 81, describe.skip(...) disables all checks for runGenerateCrudCommand (route generation, router patching, schema/error paths), so regressions will pass CI unnoticed. Please keep at least a minimal non-flaky smoke path active and skip only the unstable cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/test/generate-crud.test.ts` around lines 79 - 81, The whole
suite is currently disabled by describe.skip which removes coverage for
runGenerateCrudCommand; change this by removing describe.skip and instead keep a
single minimal, non-flaky smoke test that calls runGenerateCrudCommand and
asserts basic success, while marking the unstable/flaky cases as test.skip.
Specifically, in generate-crud.test.ts: replace
describe.skip("runGenerateCrudCommand", ...) with
describe("runGenerateCrudCommand", ...), add one compact test (e.g., "smoke:
generates routes") that invokes runGenerateCrudCommand with the simplest valid
inputs and a clean mocked environment and asserts expected side-effects, and
move the existing flaky tests to individually skipped tests (test.skip(...)) and
ensure any use of mock.module() is isolated and cleaned up in afterEach/afterAll
to avoid global mock state corruption.
Comment on lines
+96
to
+104
| const functions = allInngestFunctions.map((fn) => { | ||
| const fnAny = fn as unknown as { id: string }; | ||
| return { | ||
| id: fnAny.id, | ||
| name: fnAny.id, | ||
| status: "active", | ||
| createdAt: new Date().toISOString(), | ||
| triggers: [{ type: "event", event: `betterbase/${fnAny.id.split("-").pop()}` }], | ||
| }; |
There was a problem hiding this comment.
Self-hosted trigger metadata is being fabricated incorrectly.
Splitting the function id here produces bogus trigger events (deliver-webhook → betterbase/webhook, evaluate-notification-rule → betterbase/rule) and even marks the cron poller as event-driven. The dashboard will show the wrong trigger and capability data unless this comes from an explicit map shared with ../../lib/inngest.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/inngest.ts` around lines 96 - 104, The
current mapping in the allInngestFunctions transformation fabricates trigger
metadata by splitting fnAny.id (in the map that builds
id/name/status/createdAt/triggers), which produces incorrect event names and
misclassifies cron pollers; instead, import and use the explicit function
metadata map from ../../lib/inngest (or a provided mapping object) to populate
the triggers and capability fields for each function (lookup by fnAny.id and
fall back to a safe default like an empty triggers array and correct type),
replacing the split-based triggers construction so the dashboard shows accurate
trigger and capability data.
Comment on lines
+259
to
+271
| "poll-notification-rules": { | ||
| // Cron-triggered function - can't be manually triggered | ||
| eventName: "betterbase/notification.evaluate", | ||
| payload: { | ||
| ruleId: "cron-test", | ||
| ruleName: "Cron Test", | ||
| metric: "error_rate", | ||
| threshold: 0, | ||
| channel: "email", | ||
| target: "admin@example.com", | ||
| currentValue: 100, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
poll-notification-rules currently tests the wrong workflow.
This entry sends betterbase/notification.evaluate, so the "Test" action for poll-notification-rules never exercises the poller—it runs evaluateNotificationRule and reports success. Either reject cron functions here or add a dedicated poller test path.
Also applies to: 288-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/inngest.ts` around lines 259 - 271, The
"poll-notification-rules" test is exercising the evaluate workflow by sending
eventName "betterbase/notification.evaluate" so the poller isn't tested; update
the admin test logic for the "poll-notification-rules" entry (and the similar
entry later) to either (A) disallow/skip manual "Test" for cron-triggered
functions (detect by key "poll-notification-rules" or by cron metadata) and
return a clear message, or (B) add a dedicated poller test path that sends the
correct poll event (e.g., a "betterbase/notification.poll" or whatever the
poller expects) and/or invokes the poller handler directly instead of calling
evaluateNotificationRule; ensure the change references the
"poll-notification-rules" entry and the evaluateNotificationRule/poller handler
so the admin Test actually exercises the polling behavior.
This was referenced Mar 29, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests