feat: add monorepo packages, rename apps/server to apps/api - #44
Open
ahargunyllib wants to merge 3 commits into
Open
feat: add monorepo packages, rename apps/server to apps/api#44ahargunyllib wants to merge 3 commits into
ahargunyllib wants to merge 3 commits into
Conversation
- Rename apps/server → apps/api (wrangler name: realm-api) - Add D1, KV, R2, Queue bindings stubs to wrangler.jsonc - Add tRPC v11 + OpenAPI Swagger (via @asteasolutions/zod-to-openapi) to apps/api - Add packages/infra: alchemy.run.ts deploying all CF resources (D1, KV, R2, Queue, Worker) - Add packages/db: drizzle-orm/d1 with schema/ and queries/ structure + drizzle.config.ts - Add packages/kv: typed KVStore wrapper for CF KV binding - Add packages/email: Brevo transactional email via @getbrevo/brevo - Add packages/queue: typed QueueProducer wrapper for CF Queue binding - Add packages/storage: R2Storage wrapper for CF R2 binding - Add packages/core: pure domain logic, zero CF dependencies - Add packages/api: tRPC router, context, DTOs, OpenAPI registry - Add CI/CD: production/preview API deployment + infra deployment workflows - Update biome.jsonc: apps/server → apps/api override path https://claude.ai/code/session_016qUw4Pd4o45sbnfPsBeQpp
- Fix noNamespaceImport: replace `import * as schema` with direct export
- Fix noNamespaceImport: use named imports from @getbrevo/brevo
- Fix noParameterProperties: explicit class property declarations in KV, Queue, Storage
- Fix useConsistentTypeDefinitions: interface → type in api/context, email, queue
- Fix noEmptySource: replace comment-only files with export {}
- Fix useLiteralKeys: process.env.X instead of process.env["X"] in drizzle.config
- Fix useBlockStatements: wrap single-line if bodies in email service
- Fix useAwait: remove async from Storage methods that return Promises directly
- Add explicit return type annotation for generateOpenAPIDocument (openapi3-ts portability)
- Add openapi3-ts as direct dep in @realm/api
- Fix wrangler.jsonc: max_wait_time_ms → max_batch_timeout, add BREVO_API_KEY var
- Regenerate worker-configuration.d.ts with BREVO_API_KEY in CloudflareBindings
https://claude.ai/code/session_016qUw4Pd4o45sbnfPsBeQpp
📝 WalkthroughWalkthroughThis PR establishes a new Cloudflare Workers-based API architecture with monorepo structure. It adds three GitHub Actions workflows for preview and production deployments (API and infrastructure), removes the legacy server entry point, creates a new Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Hono as Hono App<br/>(apps/api)
participant Context as Context Factory<br/>(packages/api)
participant TRPC as TRPC Router<br/>(packages/api)
participant Services as Services<br/>(db, kv, email,<br/>queue, storage)
participant CFResources as Cloudflare<br/>Resources
Client->>Hono: HTTP Request
Hono->>Context: createContext(env)
Context->>Services: Initialize all services<br/>(createDb, createKV, etc.)
Services->>CFResources: Connect to D1, KV,<br/>R2, Queue, API
CFResources-->>Services: Resource handles
Services-->>Context: Service instances
Context-->>Hono: Context object
alt /trpc/* route
Hono->>TRPC: fetchRequestHandler()
TRPC->>Services: Execute procedure<br/>(e.g., health query)
Services-->>TRPC: Result
TRPC-->>Hono: RPC response
else /swagger routes
Hono-->>Client: Swagger UI / OpenAPI spec
else Queue message
Hono->>Services: Process MessageBatch
Services->>Services: Log messages
end
Hono-->>Client: HTTP Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview DeploymentCommit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (15)
packages/core/src/index.ts (1)
1-3: Consider whether this barrel file is necessary.This file only re-exports everything from
@realm/utilswithout adding any domain logic. Consumers could import directly from@realm/utilsinstead. If this is intended as a stable facade for future domain logic additions, consider adding a comment explaining the architectural intent. As per coding guidelines: "Avoid barrel files (index files that re-export everything)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` around lines 1 - 3, This barrel file only re-exports everything via the statement `export * from "@realm/utils"` and should either be removed to force consumers to import `@realm/utils` directly or kept as an intentional stable facade — update the PR by either deleting this file and updating any imports that use this package to import from `@realm/utils` directly, or retain it but add a clear top-of-file comment stating the architectural intent (that this package serves as a stable facade for future domain logic) and consider narrowing the exports instead of using a wildcard to follow the "avoid barrel files" guideline.packages/core/package.json (1)
18-21: Consider whetherzoddependency is needed yet.The
zoddependency is declared butpackages/core/src/index.tsonly re-exports from@realm/utilswithout using Zod. If this is for planned domain validation logic, consider adding it when actually needed to keep the dependency footprint minimal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/package.json` around lines 18 - 21, The package currently declares the "zod" dependency in packages/core/package.json but packages/core/src/index.ts only re-exports from "@realm/utils" and doesn't use Zod; remove the "zod" entry from the dependencies to reduce footprint (or move it to devDependencies if it's only used in tests/build-time tools), and add it back to package.json only when code in packages/core (e.g., schema validation logic) actually imports Zod.packages/db/src/queries/index.ts (1)
1-1: Empty module placeholder.This file exports nothing but is re-exported by the parent
index.ts. While this provides scaffolding for future query implementations, exporting empty modules can clutter the public API surface.Consider either:
- Adding a TODO comment explaining the intent, or
- Removing the re-export from
packages/db/src/index.tsuntil queries are implementedAs per coding guidelines: "Don't export empty modules that don't change anything."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/queries/index.ts` at line 1, The module currently exports nothing (empty module) which pollutes the public API; either add a clear TODO comment at the top of packages/db/src/queries/index.ts stating intended future exports (e.g., "TODO: add query re-exports such as getUserQuery, listOrdersQuery") to make the placeholder explicit, or remove the re-export of this file from the parent index.ts until actual queries (functions/classes) are implemented; update whichever you choose so the API surface no longer exposes an empty module.packages/db/src/schema/index.ts (1)
1-1: Empty module export is placeholder scaffolding.This file exports nothing but serves as the Drizzle schema entry point referenced by
drizzle.config.ts. Consider adding a brief comment explaining this is a placeholder for future schema definitions, or removing the file until schemas are added. As per coding guidelines: "Don't export empty modules that don't change anything."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/schema/index.ts` at line 1, The file currently contains only an empty module export (`export {}`) used as the Drizzle schema entry point referenced by drizzle.config.ts; either remove this file until you have actual schema exports or add a short explanatory comment above the `export {}` indicating it is an intentional placeholder for future Drizzle schema exports (so reviewers understand it isn’t accidental), and ensure any downstream references in drizzle.config.ts are updated if you remove the file.apps/api/src/index.ts (1)
19-19: Avoid regenerating the OpenAPI spec on every request.
generateOpenAPIDocument()can be computed once at module load and reused.Proposed change
import { appRouter, createContext, generateOpenAPIDocument } from "@realm/api"; const app = new Hono<{ Bindings: CloudflareBindings }>(); +const openApiDocument = generateOpenAPIDocument(); @@ -app.get("/swagger/spec", (c) => c.json(generateOpenAPIDocument())); +app.get("/swagger/spec", (c) => c.json(openApiDocument));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/index.ts` at line 19, The route handler currently calls generateOpenAPIDocument() on every request; instead call generateOpenAPIDocument() once at module load, store the result in a const (e.g., swaggerSpec) and return that cached object from the handler (replace app.get("/swagger/spec", (c) => c.json(generateOpenAPIDocument())) with a handler that uses swaggerSpec). Ensure generateOpenAPIDocument() is invoked outside request-handling code so the spec is reused across requests.packages/api/src/index.ts (1)
1-6:index.tsis a barrel export; prefer direct subpath exports.This central re-export layer conflicts with the repo rule for
index.*files. Prefer consumers importing from explicit modules (e.g.,@realm/api/router,@realm/api/context, etc.) and keep root exports minimal.As per coding guidelines "
**/index.{ts,tsx,js,jsx}: Avoid barrel files (index files that re-export everything)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/index.ts` around lines 1 - 6, This file is a barrel index re-exporting many symbols (appRouter, AppRouter, createContext, Context, Env, publicProcedure, protectedProcedure, router, generateOpenAPIDocument, registry); split these into direct subpath exports instead of a central index barrel. Replace the central re-exports with minimal root exports (if any) and update this file to export only what must remain at the package root, then move each export to its explicit subpath so consumers import from modules like "@realm/api/router" and "@realm/api/context"; ensure the symbols appRouter/AppRouter come from "./router", createContext/Context/Env come from "./context", TRPC helpers (publicProcedure/protectedProcedure/router) come from "./trpc", and OpenAPI helpers (generateOpenAPIDocument/registry) come from "./openapi" and remove the broad index-style re-exports to comply with the index.* rule..github/workflows/preview-api-deployment.yaml (3)
16-18: Concurrency group includes commit SHA, preventing in-progress cancellation.The concurrency group
preview-api-${{ github.event.pull_request.number }}-${{ github.sha }}creates a unique group per commit. Combined withcancel-in-progress: false, this means multiple deployments for the same PR can run simultaneously.If you want new commits to cancel stale deployments, remove
${{ github.sha }}from the group and setcancel-in-progress: true:Suggested change
concurrency: - group: preview-api-${{ github.event.pull_request.number }}-${{ github.sha }} - cancel-in-progress: false + group: preview-api-${{ github.event.pull_request.number }} + cancel-in-progress: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/preview-api-deployment.yaml around lines 16 - 18, The concurrency group currently includes `${{ github.sha }}` (concurrency: group: preview-api-${{ github.event.pull_request.number }}-${{ github.sha }}) and has `cancel-in-progress: false`, which prevents new commits from cancelling prior runs; remove `${{ github.sha }}` so the group becomes per-PR (e.g., preview-api-${{ github.event.pull_request.number }}) and set `cancel-in-progress: true` to ensure new commits cancel stale deployments.
60-63: Add fallback handling if regex parsing fails.If
wrangler versions uploadoutput format changes,VERSION_IDorPREVIEW_URLcould be empty. The workflow continues and posts an incomplete comment (line 69 only guards onpreview_url).Consider adding validation:
- name: Validate deployment outputs if: steps.deploy.outputs.version_id == '' || steps.deploy.outputs.preview_url == '' run: | echo "::warning::Failed to parse deployment output" cat apps/api/deploy-output.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/preview-api-deployment.yaml around lines 60 - 63, The workflow currently extracts VERSION_ID and PREVIEW_URL from deploy-output.txt using grep but does not validate the results, so empty values could be written to GITHUB_OUTPUT; add a validation step after setting VERSION_ID and PREVIEW_URL that checks if either variable is empty (referencing VERSION_ID and PREVIEW_URL and the file deploy-output.txt) and, if so, emits a warning and prints deploy-output.txt (and fails or short-circuits the job as desired) — also update the conditional that posts comments to use the outputs (steps.deploy.outputs.version_id and steps.deploy.outputs.preview_url) to guard against empty values.
68-78: Consider updating existing comments instead of creating new ones.Each deployment will create a new PR comment. For active PRs with multiple commits, this can clutter the conversation. Consider using
--edit-lastor finding/updating an existing deployment comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/preview-api-deployment.yaml around lines 68 - 78, The workflow currently always creates a new PR comment in the "Comment on PR" step using `gh pr comment`, which clutters PRs; update the run command to edit the most recent deployment comment instead of creating a new one by using `gh pr comment --edit-last --body "...")` (keep the same environment variables like `${{ github.sha }}`, `${{ steps.deploy.outputs.version_id }}` and `${{ steps.deploy.outputs.preview_url }}` and the `if: steps.deploy.outputs.preview_url != ''` check) so the action updates the last deployment comment rather than appending a new one.packages/api/src/context.ts (1)
20-27: Consider parameterizingQueueProducertype.
QueueProduceris defined asQueueProducer<T = unknown>in@realm/queue, but theContexttype uses the unparameterized version. If you have a known message type, parameterizing improves type safety for queue operations.export type Context = { db: Db; kv: KVStore; storage: R2Storage; - queue: QueueProducer; + queue: QueueProducer<YourMessageType>; email: EmailService; env: Env; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/context.ts` around lines 20 - 27, The Context type uses the unparameterized QueueProducer which loses message-type safety; make the Context type generic (or set the concrete message type) so the queue field is typed as QueueProducer<TMessage> instead of raw QueueProducer — e.g., change Context to export type Context<TMessage = unknown> (or replace with your domain message type) and update the queue property to queue: QueueProducer<TMessage>; ensure any consumers constructing Context pass the proper type.packages/email/src/index.ts (3)
22-31: Consider wrapping API errors with domain-specific context.The
send()method propagates Brevo SDK errors directly. Wrapping errors with additional context (e.g., which recipients failed) can improve debuggability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/src/index.ts` around lines 22 - 31, The send method currently lets Brevo SDK errors bubble up; update the send function to catch errors from this.api.sendTransacEmail and rethrow a new, domain-specific error that includes contextual details (e.g., recipients from opts.to and subject from opts.subject) while preserving the original error as the cause or inner error; reference the SendEmailOptions type, the SendSmtpEmail instance, and the this.api.sendTransacEmail call so you locate the try/catch surrounding the API call and include the original error when constructing the wrapped error.
17-20: Consider validatingapiKeybefore initializing the API client.If
BREVO_API_KEYis empty or undefined (which it currently is inwrangler.jsonc), the service will be created but fail at runtime whensend()is called. Early validation provides clearer error messages.Suggested validation
constructor(apiKey: string) { + if (!apiKey) { + throw new Error("BREVO_API_KEY is required"); + } this.api = new TransactionalEmailsApi(); this.api.setApiKey(TransactionalEmailsApiApiKeys.apiKey, apiKey); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/src/index.ts` around lines 17 - 20, Validate the apiKey passed into the constructor of the class that creates a new TransactionalEmailsApi: check that apiKey is a non-empty string before calling new TransactionalEmailsApi() and this.api.setApiKey(...); if invalid, throw a clear error (or reject) so callers see a descriptive failure early (reference: constructor, TransactionalEmailsApi, and send()).
34-34: Add explicit return type annotation.-export const createEmailService = (apiKey: string) => new EmailService(apiKey); +export const createEmailService = (apiKey: string): EmailService => new EmailService(apiKey);As per coding guidelines: "Use explicit types for function parameters and return values when they enhance clarity".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/src/index.ts` at line 34, createEmailService lacks an explicit return type; update its signature to include the concrete return type (EmailService) so callers and tooling know what is returned. Modify the createEmailService declaration to annotate the return type as EmailService (keeping the parameter apiKey: string and the implementation new EmailService(apiKey)) and add any necessary type import if EmailService is defined in another module.packages/storage/src/index.ts (1)
29-29: Add explicit return type annotation.-export const createStorage = (bucket: R2Bucket) => new R2Storage(bucket); +export const createStorage = (bucket: R2Bucket): R2Storage => new R2Storage(bucket);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage/src/index.ts` at line 29, The exported factory createStorage currently returns new R2Storage(bucket) without an explicit return type; add a precise return type annotation to the function signature (e.g., annotate createStorage as returning R2Storage or the appropriate interface/type) so callers see the API contract and TypeScript enforces it—update the createStorage(...) => new R2Storage(bucket) declaration to include : R2Storage (or the storage interface type) after the parameter list.apps/api/wrangler.jsonc (1)
13-25: Document the placeholder value replacement process.
database_idand KVidare set to"FILL_FROM_ALCHEMY". Whilepackages/infra/alchemy.run.tsprovisions these resources, it's unclear how these placeholder values get replaced in the wrangler config beforewrangler deployruns.Consider adding a comment explaining the deployment workflow or automating the value injection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/wrangler.jsonc` around lines 13 - 25, Add a short comment in wrangler.jsonc above the d1_databases and kv_namespaces entries explaining that the "FILL_FROM_ALCHEMY" placeholders for database_id and KV id are replaced prior to deployment by the infra provisioning step implemented in packages/infra/alchemy.run.ts (which emits the actual IDs and injects them into the Wrangler config or environment before running wrangler deploy); alternatively, wire an automated injector: have alchemy.run.ts output the IDs to a single source (env vars or a generated wrangler.json) and update the deployment script to replace the "FILL_FROM_ALCHEMY" tokens with those outputs before executing wrangler deploy, referencing database_id and kv_namespaces.id for the replacements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/production-api-deployment.yaml:
- Line 33: Replace the hardcoded checkout ref (the line containing "ref: main")
so the workflow checks out the exact merged commit SHA instead of the moving
main branch head; update the actions/checkout@... step to use the pull request's
merge commit like github.event.pull_request.merge_commit_sha (e.g., set the
checkout "ref" to ${{ github.event.pull_request.merge_commit_sha }}) to ensure
the deployed artifact matches the merged PR.
In @.github/workflows/production-infra-deployment.yaml:
- Around line 12-14: The workflow currently cancels in-progress production infra
deployments via the concurrency setting (group: production-infra,
cancel-in-progress: true); change this to avoid cancellation—either remove the
cancel-in-progress key or set cancel-in-progress to false so running infra
applies are allowed to finish and not be terminated mid-apply.
- Line 28: Replace the non-deterministic "ref: main" with the merged commit SHA
so deployments use the exact commit that triggered the run; update the checkout
action's ref to use the PR merge SHA (e.g. set ref: ${{
github.event.pull_request.merge_commit_sha }} with a fallback to ${{ github.sha
}}), ensuring the checkout step (the ref field) pins to a specific commit
instead of the branch tip.
In `@apps/api/wrangler.jsonc`:
- Around line 39-45: The wrangler config declares a consumer for "realm-queue"
but the service does not export a queue() handler, so implement and export an
async queue(event) function in apps/api/src/index.ts that processes batched
messages for "realm-queue" (respecting max_batch_size/max_batch_timeout),
deserializes each record, handles errors per-record, and acknowledges or retries
as appropriate; ensure the exported symbol is named queue and wired into
existing routing/processing helpers used by other handlers in that file.
- Around line 9-12: Remove the plaintext BREVO_API_KEY from the "vars" object in
wrangler.jsonc and configure it as a Cloudflare Workers secret instead; run
wrangler secret put BREVO_API_KEY during deployment (or CI) to set the value,
update any startup/runtime code expecting process.env.BREVO_API_KEY to read from
the secret as-is, and add a short note to the README/deployment instructions
stating that BREVO_API_KEY must be provided via Cloudflare secrets rather than
committed in vars.
In `@packages/api/src/openapi.ts`:
- Around line 7-19: The OpenAPI registry is never populated and the Zod OpenAPI
extension may not run before document generation; import the module that runs
extendZodWithOpenApi(z) (dtos/index.ts) at the top of this file so the Zod
.openapi() side-effect is guaranteed, and then register your tRPC routes with
the registry (use registry.registerPath(...) for each route defined on
appRouter) before calling generateOpenAPIDocument()/new
OpenApiGeneratorV3(registry.definitions); add clear TODOs next to registry and
generateOpenAPIDocument referencing appRouter and registry.registerPath so
maintainers wire each route into the registry.
In `@packages/api/src/routers/index.ts`:
- Line 1: Remove the no-op "export {}" module placeholder from this file and
either delete the file until there are router exports or replace it with the
real router exports (e.g., export your routers or a combined default export).
Specifically, remove the standalone "export {}" statement in index.ts and add
the appropriate exports for your routers (or leave the file absent) so the
module surface is meaningful.
In `@packages/api/src/trpc.ts`:
- Around line 7-8: The exported protectedProcedure currently equals
publicProcedure (both are t.procedure) and provides no auth enforcement; either
remove the protectedProcedure export or implement authentication middleware and
apply it to protectedProcedure. To fix, add an auth middleware function (e.g.,
authMiddleware) that checks the request context (Context) for an authenticated
user/session and throws an unauthorized error if missing, then change
protectedProcedure to use t.procedure.use(authMiddleware); alternatively, if you
prefer to postpone auth, delete the protectedProcedure export to avoid
misleading consumers. Ensure you reference and update the symbols
protectedProcedure, publicProcedure, t.procedure, and the Context checks in your
fix.
In `@packages/db/drizzle.config.ts`:
- Around line 9-11: Replace the silent defaults for Cloudflare D1 credentials
with explicit validation: read process.env.CLOUDFLARE_ACCOUNT_ID,
CLOUDFLARE_D1_DATABASE_ID, and CLOUDFLARE_D1_TOKEN into the variables referenced
as accountId, databaseId, and token in drizzle.config.ts, check each for
presence, and throw a clear Error (or process.exit) listing any missing env
var(s) so the application fails fast during startup instead of using "" and
failing later.
In `@packages/infra/alchemy.run.ts`:
- Around line 27-29: The Worker is bound to QUEUE ("realm-queue") but
apps/api/src/index.ts only exports the Hono app (app.fetch) and lacks a queue
handler, so queued messages won't be processed; update apps/api/src/index.ts to
export a default object that includes fetch: app.fetch and a queue: async
(batch, env) => { ... } handler to iterate and process messages from the batch
(matching how QUEUE/"realm-queue" is produced), implementing message processing
logic using the existing app/context as needed; ensure the exported symbol is
the default export so the Worker binding picks up the queue() handler.
- Line 46: Remove the stray debug console.log call: delete the statement
console.log("API URL:", api.url); or replace it with a structured logging call
consistent with the project's logger (e.g., use the existing logger instance) if
this information must be emitted; locate the exact statement referencing api.url
in packages/infra/alchemy.run.ts and ensure no other console.log/debugger/alert
calls remain in that module.
- Around line 14-17: The deployment fails because D1Database(...) in
alchemy.run.ts references migrationsDir: "../db/drizzle" which doesn't exist;
either pre-generate and commit migrations under packages/db/drizzle by running
`drizzle-kit generate` in packages/db (ensure the generated files are
committed), or update the deployment/build pipeline to run `drizzle-kit
generate` in the packages/db workspace before executing alchemy.run.ts so the
"../db/drizzle" directory exists at runtime; verify the D1Database call
(D1Database("db", { name: "realm-db", migrationsDir: "../db/drizzle" })) points
to the correct generated migrations path.
In `@packages/storage/src/index.ts`:
- Around line 8-14: The upload method's declared return type is incorrect:
change the return type of upload(key: string, body: ReadableStream | ArrayBuffer
| string, options?: R2PutOptions) from Promise<R2Object> to Promise<R2Object |
null> because R2Bucket.put(...) can return null when conditional preconditions
fail (similar to download()); update the function signature for upload to
reflect Promise<R2Object | null> so callers properly handle the nullable
response from R2Bucket.put.
---
Nitpick comments:
In @.github/workflows/preview-api-deployment.yaml:
- Around line 16-18: The concurrency group currently includes `${{ github.sha
}}` (concurrency: group: preview-api-${{ github.event.pull_request.number }}-${{
github.sha }}) and has `cancel-in-progress: false`, which prevents new commits
from cancelling prior runs; remove `${{ github.sha }}` so the group becomes
per-PR (e.g., preview-api-${{ github.event.pull_request.number }}) and set
`cancel-in-progress: true` to ensure new commits cancel stale deployments.
- Around line 60-63: The workflow currently extracts VERSION_ID and PREVIEW_URL
from deploy-output.txt using grep but does not validate the results, so empty
values could be written to GITHUB_OUTPUT; add a validation step after setting
VERSION_ID and PREVIEW_URL that checks if either variable is empty (referencing
VERSION_ID and PREVIEW_URL and the file deploy-output.txt) and, if so, emits a
warning and prints deploy-output.txt (and fails or short-circuits the job as
desired) — also update the conditional that posts comments to use the outputs
(steps.deploy.outputs.version_id and steps.deploy.outputs.preview_url) to guard
against empty values.
- Around line 68-78: The workflow currently always creates a new PR comment in
the "Comment on PR" step using `gh pr comment`, which clutters PRs; update the
run command to edit the most recent deployment comment instead of creating a new
one by using `gh pr comment --edit-last --body "...")` (keep the same
environment variables like `${{ github.sha }}`, `${{
steps.deploy.outputs.version_id }}` and `${{ steps.deploy.outputs.preview_url
}}` and the `if: steps.deploy.outputs.preview_url != ''` check) so the action
updates the last deployment comment rather than appending a new one.
In `@apps/api/src/index.ts`:
- Line 19: The route handler currently calls generateOpenAPIDocument() on every
request; instead call generateOpenAPIDocument() once at module load, store the
result in a const (e.g., swaggerSpec) and return that cached object from the
handler (replace app.get("/swagger/spec", (c) =>
c.json(generateOpenAPIDocument())) with a handler that uses swaggerSpec). Ensure
generateOpenAPIDocument() is invoked outside request-handling code so the spec
is reused across requests.
In `@apps/api/wrangler.jsonc`:
- Around line 13-25: Add a short comment in wrangler.jsonc above the
d1_databases and kv_namespaces entries explaining that the "FILL_FROM_ALCHEMY"
placeholders for database_id and KV id are replaced prior to deployment by the
infra provisioning step implemented in packages/infra/alchemy.run.ts (which
emits the actual IDs and injects them into the Wrangler config or environment
before running wrangler deploy); alternatively, wire an automated injector: have
alchemy.run.ts output the IDs to a single source (env vars or a generated
wrangler.json) and update the deployment script to replace the
"FILL_FROM_ALCHEMY" tokens with those outputs before executing wrangler deploy,
referencing database_id and kv_namespaces.id for the replacements.
In `@packages/api/src/context.ts`:
- Around line 20-27: The Context type uses the unparameterized QueueProducer
which loses message-type safety; make the Context type generic (or set the
concrete message type) so the queue field is typed as QueueProducer<TMessage>
instead of raw QueueProducer — e.g., change Context to export type
Context<TMessage = unknown> (or replace with your domain message type) and
update the queue property to queue: QueueProducer<TMessage>; ensure any
consumers constructing Context pass the proper type.
In `@packages/api/src/index.ts`:
- Around line 1-6: This file is a barrel index re-exporting many symbols
(appRouter, AppRouter, createContext, Context, Env, publicProcedure,
protectedProcedure, router, generateOpenAPIDocument, registry); split these into
direct subpath exports instead of a central index barrel. Replace the central
re-exports with minimal root exports (if any) and update this file to export
only what must remain at the package root, then move each export to its explicit
subpath so consumers import from modules like "@realm/api/router" and
"@realm/api/context"; ensure the symbols appRouter/AppRouter come from
"./router", createContext/Context/Env come from "./context", TRPC helpers
(publicProcedure/protectedProcedure/router) come from "./trpc", and OpenAPI
helpers (generateOpenAPIDocument/registry) come from "./openapi" and remove the
broad index-style re-exports to comply with the index.* rule.
In `@packages/core/package.json`:
- Around line 18-21: The package currently declares the "zod" dependency in
packages/core/package.json but packages/core/src/index.ts only re-exports from
"@realm/utils" and doesn't use Zod; remove the "zod" entry from the dependencies
to reduce footprint (or move it to devDependencies if it's only used in
tests/build-time tools), and add it back to package.json only when code in
packages/core (e.g., schema validation logic) actually imports Zod.
In `@packages/core/src/index.ts`:
- Around line 1-3: This barrel file only re-exports everything via the statement
`export * from "@realm/utils"` and should either be removed to force consumers
to import `@realm/utils` directly or kept as an intentional stable facade —
update the PR by either deleting this file and updating any imports that use
this package to import from `@realm/utils` directly, or retain it but add a
clear top-of-file comment stating the architectural intent (that this package
serves as a stable facade for future domain logic) and consider narrowing the
exports instead of using a wildcard to follow the "avoid barrel files"
guideline.
In `@packages/db/src/queries/index.ts`:
- Line 1: The module currently exports nothing (empty module) which pollutes the
public API; either add a clear TODO comment at the top of
packages/db/src/queries/index.ts stating intended future exports (e.g., "TODO:
add query re-exports such as getUserQuery, listOrdersQuery") to make the
placeholder explicit, or remove the re-export of this file from the parent
index.ts until actual queries (functions/classes) are implemented; update
whichever you choose so the API surface no longer exposes an empty module.
In `@packages/db/src/schema/index.ts`:
- Line 1: The file currently contains only an empty module export (`export {}`)
used as the Drizzle schema entry point referenced by drizzle.config.ts; either
remove this file until you have actual schema exports or add a short explanatory
comment above the `export {}` indicating it is an intentional placeholder for
future Drizzle schema exports (so reviewers understand it isn’t accidental), and
ensure any downstream references in drizzle.config.ts are updated if you remove
the file.
In `@packages/email/src/index.ts`:
- Around line 22-31: The send method currently lets Brevo SDK errors bubble up;
update the send function to catch errors from this.api.sendTransacEmail and
rethrow a new, domain-specific error that includes contextual details (e.g.,
recipients from opts.to and subject from opts.subject) while preserving the
original error as the cause or inner error; reference the SendEmailOptions type,
the SendSmtpEmail instance, and the this.api.sendTransacEmail call so you locate
the try/catch surrounding the API call and include the original error when
constructing the wrapped error.
- Around line 17-20: Validate the apiKey passed into the constructor of the
class that creates a new TransactionalEmailsApi: check that apiKey is a
non-empty string before calling new TransactionalEmailsApi() and
this.api.setApiKey(...); if invalid, throw a clear error (or reject) so callers
see a descriptive failure early (reference: constructor, TransactionalEmailsApi,
and send()).
- Line 34: createEmailService lacks an explicit return type; update its
signature to include the concrete return type (EmailService) so callers and
tooling know what is returned. Modify the createEmailService declaration to
annotate the return type as EmailService (keeping the parameter apiKey: string
and the implementation new EmailService(apiKey)) and add any necessary type
import if EmailService is defined in another module.
In `@packages/storage/src/index.ts`:
- Line 29: The exported factory createStorage currently returns new
R2Storage(bucket) without an explicit return type; add a precise return type
annotation to the function signature (e.g., annotate createStorage as returning
R2Storage or the appropriate interface/type) so callers see the API contract and
TypeScript enforces it—update the createStorage(...) => new R2Storage(bucket)
declaration to include : R2Storage (or the storage interface type) after the
parameter list.
🪄 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: cc6de8a7-649d-4827-bdf4-28b1afc407c2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (44)
.github/workflows/preview-api-deployment.yaml.github/workflows/production-api-deployment.yaml.github/workflows/production-infra-deployment.yamlapps/api/package.jsonapps/api/src/index.tsapps/api/tsconfig.jsonapps/api/worker-configuration.d.tsapps/api/wrangler.jsoncapps/server/src/index.tsapps/server/wrangler.jsoncbiome.jsoncpackages/api/package.jsonpackages/api/src/context.tspackages/api/src/dtos/index.tspackages/api/src/index.tspackages/api/src/openapi.tspackages/api/src/router.tspackages/api/src/routers/index.tspackages/api/src/trpc.tspackages/api/tsconfig.jsonpackages/core/package.jsonpackages/core/src/index.tspackages/core/tsconfig.jsonpackages/db/drizzle.config.tspackages/db/package.jsonpackages/db/src/index.tspackages/db/src/queries/index.tspackages/db/src/schema/index.tspackages/db/tsconfig.jsonpackages/email/package.jsonpackages/email/src/index.tspackages/email/tsconfig.jsonpackages/infra/alchemy.run.tspackages/infra/package.jsonpackages/infra/tsconfig.jsonpackages/kv/package.jsonpackages/kv/src/index.tspackages/kv/tsconfig.jsonpackages/queue/package.jsonpackages/queue/src/index.tspackages/queue/tsconfig.jsonpackages/storage/package.jsonpackages/storage/src/index.tspackages/storage/tsconfig.json
💤 Files with no reviewable changes (2)
- apps/server/src/index.ts
- apps/server/wrangler.jsonc
- Add @types/node to packages/infra, packages/db, apps/api devDeps - Add @cloudflare/workers-types to apps/api devDeps - Add @types/node to pnpm-workspace.yaml catalog - Remove protectedProcedure (misleading alias) from trpc.ts and index.ts - Import ./dtos/index in openapi.ts to ensure extendZodWithOpenApi side-effect runs - Delete empty packages/api/src/routers/index.ts (no-op export) - Replace drizzle.config.ts ?? "" fallbacks with fail-fast requireEnv() - Create packages/db/drizzle/.gitkeep for alchemy migrationsDir reference - Remove console.log from infra alchemy.run.ts; prefix unused var with _ - Fix R2Storage.upload() return type to Promise<R2Object | null> - Remove BREVO_API_KEY from wrangler.jsonc vars (use CF secret instead) - Add queue handler and use ExportedHandler<Env> in apps/api/src/index.ts - Switch Hono bindings type from CloudflareBindings to Env from @realm/api - Fix production-api-deployment.yaml: use merge_commit_sha instead of ref: main - Fix production-infra-deployment.yaml: cancel-in-progress false, merge_commit_sha, add pnpm --filter @realm/db generate step before infra deploy - Regenerate worker-configuration.d.ts after wrangler.jsonc changes https://claude.ai/code/session_016qUw4Pd4o45sbnfPsBeQpp
Preview DeploymentCommit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/index.ts`:
- Around line 27-34: In the queue handler (the function starting with "queue:
(batch: MessageBatch, _env: Env) =>" iterating over batch.messages), remove the
console.log call and replace it with a structured logging call using your
application's logger (e.g., call _env.logger.info or processLogger.info if
available) or delete the loop if it was placeholder code; ensure you log the
relevant fields from message.body (not raw JSON.stringify output) in a
structured object to preserve typing and avoid sensitive data exposure.
🪄 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: 4e5eb4c5-d439-4011-a288-1681163ddbf6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.github/workflows/production-api-deployment.yaml.github/workflows/production-infra-deployment.yamlapps/api/package.jsonapps/api/src/index.tsapps/api/worker-configuration.d.tsapps/api/wrangler.jsoncpackages/api/src/index.tspackages/api/src/openapi.tspackages/api/src/trpc.tspackages/db/drizzle.config.tspackages/db/drizzle/.gitkeeppackages/db/package.jsonpackages/infra/alchemy.run.tspackages/infra/package.jsonpackages/storage/src/index.tspnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (7)
- packages/db/drizzle.config.ts
- .github/workflows/production-infra-deployment.yaml
- packages/api/src/index.ts
- .github/workflows/production-api-deployment.yaml
- apps/api/wrangler.jsonc
- packages/infra/package.json
- pnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/api/src/trpc.ts
- packages/infra/alchemy.run.ts
- packages/api/src/openapi.ts
- packages/db/package.json
- apps/api/package.json
Comment on lines
+27
to
+34
| export default { | ||
| fetch: app.fetch, | ||
| queue: (batch: MessageBatch, _env: Env) => { | ||
| for (const message of batch.messages) { | ||
| console.log("consumed from queue:", JSON.stringify(message.body)); | ||
| } | ||
| }, | ||
| } satisfies ExportedHandler<Env>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Cloudflare Workers types are configured in tsconfig
# Check if `@cloudflare/workers-types` is in dependencies
echo "=== Checking package.json for `@cloudflare/workers-types` ==="
fd -t f 'package.json' apps/api --exec cat {} | jq '.devDependencies["@cloudflare/workers-types"] // .dependencies["@cloudflare/workers-types"] // "NOT FOUND"'
# Check tsconfig for types configuration
echo -e "\n=== Checking tsconfig for types array ==="
fd -t f 'tsconfig.json' apps/api --exec cat {} | jq '.compilerOptions.types // "types not specified"'Repository: ahargunyllib/realm
Length of output: 202
🏁 Script executed:
# Check the full tsconfig.json configuration
fd -t f 'tsconfig.json' apps/api --exec cat {}Repository: ahargunyllib/realm
Length of output: 304
🏁 Script executed:
# Verify if the types are actually working without explicit configuration
# by checking if there are any type errors in the file or if the types resolve
fd -t f 'index.ts' apps/api --exec head -50 {}Repository: ahargunyllib/realm
Length of output: 940
🏁 Script executed:
# Check the base tsconfig that's being extended
fd -t f 'base.json' packages/tsconfig --exec cat {}Repository: ahargunyllib/realm
Length of output: 646
🏁 Script executed:
# Verify that the types can be resolved by checking if there are any TypeScript compilation issues
# Look for any type resolution in the extended config or if types are imported elsewhere
find apps/api -name '*.ts' -o -name '*.tsx' | xargs grep -l "MessageBatch\|ExportedHandler" | head -5Repository: ahargunyllib/realm
Length of output: 118
🏁 Script executed:
# Check the worker-configuration.d.ts file to see if types are declared there
cat apps/api/worker-configuration.d.tsRepository: ahargunyllib/realm
Length of output: 50375
🏁 Script executed:
# Check if `@cloudflare/workers-types` is properly included in the environment
# Look for reference comments or imports that pull in the Cloudflare Workers types
grep -r "reference\|cloudflare\|workers-types" apps/api --include="*.ts" --include="*.d.ts" --include="*.json"Repository: ahargunyllib/realm
Length of output: 6448
Remove console.log from production code.
The queue handler uses console.log which violates coding guidelines. Replace with structured logging or remove if this is placeholder code.
Proposed fix to remove console.log
export default {
fetch: app.fetch,
queue: (batch: MessageBatch, _env: Env) => {
for (const message of batch.messages) {
- console.log("consumed from queue:", JSON.stringify(message.body));
+ // TODO: Implement proper queue message processing
+ message.ack();
}
},
} satisfies ExportedHandler<Env>;📝 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 default { | |
| fetch: app.fetch, | |
| queue: (batch: MessageBatch, _env: Env) => { | |
| for (const message of batch.messages) { | |
| console.log("consumed from queue:", JSON.stringify(message.body)); | |
| } | |
| }, | |
| } satisfies ExportedHandler<Env>; | |
| export default { | |
| fetch: app.fetch, | |
| queue: (batch: MessageBatch, _env: Env) => { | |
| for (const message of batch.messages) { | |
| // TODO: Implement proper queue message processing | |
| message.ack(); | |
| } | |
| }, | |
| } satisfies ExportedHandler<Env>; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/index.ts` around lines 27 - 34, In the queue handler (the
function starting with "queue: (batch: MessageBatch, _env: Env) =>" iterating
over batch.messages), remove the console.log call and replace it with a
structured logging call using your application's logger (e.g., call
_env.logger.info or processLogger.info if available) or delete the loop if it
was placeholder code; ensure you log the relevant fields from message.body (not
raw JSON.stringify output) in a structured object to preserve typing and avoid
sensitive data exposure.
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.
https://claude.ai/code/session_016qUw4Pd4o45sbnfPsBeQpp
Summary by CodeRabbit
Release Notes
New Features
Chores