Rename server to API and enhance Alchemy integration#45
Conversation
…vironment configuration
Preview DeploymentCommit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new Alchemy-backed API app with runtime and health endpoint, a GitHub Actions workflow to deploy/destroy per-PR preview stages, workspace/catalog updates to include Alchemy, moves lint override to Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repo (checkout & setup)
participant PNPM as pnpm / Alchemy CLI
participant Alchemy as Alchemy / Cloud service
participant GH_API as GitHub API
GH->>Repo: Trigger on PR event (opened/synchronize/reopened/closed)
Repo->>Repo: Setup Node 22, pnpm 10.14.0, cache, install
alt PR opened/synchronized/reopened
Repo->>PNPM: Run `pnpm dlx alchemy deploy --stage pr-{PR_NUMBER}` (from `apps/api`)
PNPM->>Alchemy: Create/Update preview worker (realm-api / api)
Alchemy-->>PNPM: Return preview URL
PNPM->>GH_API: Post/update PR comment with SHA & preview URL
GH_API-->>GH: Comment published
else PR closed
Repo->>Repo: Safety check (fail if prod)
Repo->>PNPM: Run `pnpm dlx alchemy destroy --stage pr-{PR_NUMBER}` (from `apps/api`)
PNPM->>Alchemy: Destroy preview worker
Alchemy-->>PNPM: Teardown confirmed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: |
Preview DeploymentCommit: |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
apps/api/src/env.ts (1)
1-3: Emptybindingsyields an emptyEnvtype.The
workeris created withbindings: {}inalchemy.run.ts, soEnvcurrently resolves to{}. This works as a scaffold for future bindings (KV, D1, secrets), but any code expecting actual bindings fromEnvwill have no type information untilbindingsis populated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/env.ts` around lines 1 - 3, Env currently resolves to an empty object because worker.bindings is `{}`; update the typing so consumers get meaningful binding types by either (A) widening Env to a fallback type (e.g., make Env a union/extension of typeof worker.bindings with Record<string, unknown> or Record<string, any>) or (B) declare an explicit interface for expected bindings and intersect it with typeof worker.bindings; change the export in apps/api/src/env.ts (the Env type) accordingly and/or add the proper bindings object or declaration when creating `worker` in alchemy.run.ts so that `worker.bindings` carries the intended keys for consumers to use.apps/api/alchemy.run.ts (1)
49-49: Add guard for invalidPULL_REQUESTvalue.
Number(process.env.PULL_REQUEST)returnsNaNif the value is not a valid numeric string, which could cause unexpected behavior when passed asissueNumber.🛡️ Defensive improvement
if (process.env.PULL_REQUEST) { + const issueNumber = Number(process.env.PULL_REQUEST); + if (Number.isNaN(issueNumber)) { + throw new Error(`Invalid PULL_REQUEST number: ${process.env.PULL_REQUEST}`); + } await GitHubComment("preview-comment", { owner: "ahargunyllib", - repository: "ahargunyllib/realm", - issueNumber: Number(process.env.PULL_REQUEST), + repository: "realm", + issueNumber,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/alchemy.run.ts` at line 49, Guard against invalid PULL_REQUEST by validating the conversion used for issueNumber: replace the direct Number(process.env.PULL_REQUEST) call with logic that parses process.env.PULL_REQUEST (e.g., parseInt base 10) and checks isNaN; if the parsed value is NaN, set issueNumber to undefined (or a safe default) instead of NaN so callers of issueNumber don't receive an invalid numeric value; update the assignment where issueNumber is set to use this validated value..github/workflows/api-deployment.yaml (2)
21-21: Consider using explicitgithub.event.pull_request.number.While
github.event.numberis valid in pull_request contexts,github.event.pull_request.numberis more explicit and safer if the workflow is extended to support other event types in the future.♻️ Suggested refinement
- STAGE: ${{ github.event_name == 'pull_request' && format('pr-{0}', github.event.number) || (github.ref == 'refs/heads/main' && 'prod' || github.ref_name) }} + STAGE: ${{ github.event_name == 'pull_request' && format('pr-{0}', github.event.pull_request.number) || (github.ref == 'refs/heads/main' && 'prod' || github.ref_name) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/api-deployment.yaml at line 21, Update the STAGE expression to use the explicit pull request number field instead of the generic event number: replace references to github.event.number with github.event.pull_request.number in the STAGE assignment so the conditional that builds the pr-{number} name uses the explicit github.event.pull_request.number value; ensure the rest of the expression (the fallback checks for github.ref == 'refs/heads/main' -> 'prod' and github.ref_name) remains unchanged.
76-98: Consider extracting duplicated setup steps into a composite action.The setup steps (checkout, Node.js setup, pnpm setup, cache, install) are duplicated between the
deployandcleanupjobs. Extracting these into a reusable composite action would improve maintainability and reduce duplication.This is a "nice-to-have" refactor and can be deferred to a future cleanup PR if the team prefers to keep the workflow explicit for now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/api-deployment.yaml around lines 76 - 98, The workflow duplicates the initial setup steps (actions/checkout@v6, actions/setup-node@v6 with node-version, pnpm/action-setup@v5, actions/cache@v5 for ~/.pnpm-store, and the pnpm install --frozen-lockfile run) across jobs; extract these into a reusable composite action and replace the duplicated blocks in both the deploy and cleanup jobs with a single call to that composite. Create a composite action that exposes inputs for node-version and pnpm version, includes the cache and install steps, and then update the jobs to use uses: ./path/to/composite with appropriate inputs (refer to the "Setup Node.js", "Setup pnpm", "Cache pnpm store", and "Install dependencies" steps from the diff).
🤖 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/api-deployment.yaml:
- Line 5: The pull_request event types list is missing "closed", so the cleanup
job (the cleanup job that checks github.event.action == 'closed') will never
run; update the pull_request types array to include "closed" alongside opened,
synchronize, and reopened so the cleanup job can trigger when PRs are closed and
remove preview environments. Locate the pull_request trigger definition and add
"closed" to the types: [opened, synchronize, reopened] list.
- Line 64: The workflow sets GITHUB_SHA but the runtime (alchemy.run.ts) reads
process.env.COMMIT_SHA, causing undefined; update the workflow env mapping so it
exports COMMIT_SHA: ${{ github.sha }} (or export both COMMIT_SHA and GITHUB_SHA)
where GITHUB_SHA is currently defined so process.env.COMMIT_SHA is populated for
alchemy.run.ts line that builds the PR comment body.
- Around line 55-66: Add the missing CLOUDFLARE_ACCOUNT_ID environment variable
to the Deploy step so the alchemy deploy run in the "Deploy" job
(working-directory: apps/api, run: pnpm dlx alchemy deploy --stage ${{ env.STAGE
}}) has the same Cloudflare account context as the cleanup job; add
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} under the env block
alongside CLOUDFLARE_API_TOKEN and CLOUDFLARE_EMAIL.
- Line 21: The STAGE interpolation in the workflow (the STAGE variable) contains
unreachable production logic because the workflow only triggers on pull_request;
either add a push trigger for main so github.event_name can be "push" (update
the workflow "on:" section to include push: branches: [main] and the same paths
filter as pull_request) to enable 'prod' evaluation, or remove the production
branch clause (github.ref == 'refs/heads/main && 'prod' ) from the STAGE
expression and keep only the PR-based staging logic; update the
.github/workflows/api-deployment.yaml STAGE assignment accordingly.
- Line 47: The workflow currently caches the wrong pnpm store entry ("path:
~/.pnpm-store"); update the cache step to point to pnpm's actual store by either
(a) using a dynamic command to get the store path (run `pnpm store path` and
feed that value into the cache path), or (b) replace the literal path with the
correct default store location for the runner version (use the store path
referenced in the review). Apply this change for both occurrences of the cache
step (the deploy and cleanup jobs) so the pnpm store is actually cached.
In `@apps/api/alchemy.run.ts`:
- Around line 46-55: The GitHubComment call is passing the full "owner/repo" in
the repository field; change it to just the repository name so owner and
repository are separate. Locate the GitHubComment invocation (identifier:
GitHubComment) and update the repository argument from "ahargunyllib/realm" to
"realm" while leaving owner as "ahargunyllib" and keeping other fields
(issueNumber, body) unchanged.
In `@package.json`:
- Around line 7-16: Remove the unsupported "catalog" object from the
package.json "workspaces" block and convert "workspaces" to the simple array
form (i.e., ensure "workspaces" only contains the packages array like
["apps/*","packages/*"]); move any catalog definitions into pnpm-workspace.yaml
using the top-level "catalog" or "catalogs" field instead of keeping "catalog"
inside package.json.
---
Nitpick comments:
In @.github/workflows/api-deployment.yaml:
- Line 21: Update the STAGE expression to use the explicit pull request number
field instead of the generic event number: replace references to
github.event.number with github.event.pull_request.number in the STAGE
assignment so the conditional that builds the pr-{number} name uses the explicit
github.event.pull_request.number value; ensure the rest of the expression (the
fallback checks for github.ref == 'refs/heads/main' -> 'prod' and
github.ref_name) remains unchanged.
- Around line 76-98: The workflow duplicates the initial setup steps
(actions/checkout@v6, actions/setup-node@v6 with node-version,
pnpm/action-setup@v5, actions/cache@v5 for ~/.pnpm-store, and the pnpm install
--frozen-lockfile run) across jobs; extract these into a reusable composite
action and replace the duplicated blocks in both the deploy and cleanup jobs
with a single call to that composite. Create a composite action that exposes
inputs for node-version and pnpm version, includes the cache and install steps,
and then update the jobs to use uses: ./path/to/composite with appropriate
inputs (refer to the "Setup Node.js", "Setup pnpm", "Cache pnpm store", and
"Install dependencies" steps from the diff).
In `@apps/api/alchemy.run.ts`:
- Line 49: Guard against invalid PULL_REQUEST by validating the conversion used
for issueNumber: replace the direct Number(process.env.PULL_REQUEST) call with
logic that parses process.env.PULL_REQUEST (e.g., parseInt base 10) and checks
isNaN; if the parsed value is NaN, set issueNumber to undefined (or a safe
default) instead of NaN so callers of issueNumber don't receive an invalid
numeric value; update the assignment where issueNumber is set to use this
validated value.
In `@apps/api/src/env.ts`:
- Around line 1-3: Env currently resolves to an empty object because
worker.bindings is `{}`; update the typing so consumers get meaningful binding
types by either (A) widening Env to a fallback type (e.g., make Env a
union/extension of typeof worker.bindings with Record<string, unknown> or
Record<string, any>) or (B) declare an explicit interface for expected bindings
and intersect it with typeof worker.bindings; change the export in
apps/api/src/env.ts (the Env type) accordingly and/or add the proper bindings
object or declaration when creating `worker` in alchemy.run.ts so that
`worker.bindings` carries the intended keys for consumers to use.
🪄 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: ec8ffc3a-8414-41fe-8374-1b216bb286f7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.github/workflows/api-deployment.yaml.gitignoreapps/api/.env.exampleapps/api/alchemy.run.tsapps/api/package.jsonapps/api/src/env.tsapps/api/src/index.tsapps/api/tsconfig.jsonapps/server/package.jsonapps/server/worker-configuration.d.tsapps/server/wrangler.jsoncbiome.jsoncpackage.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (2)
- apps/server/wrangler.jsonc
- apps/server/package.json
Preview DeploymentCommit: |
Preview DeploymentCommit: |
… preview deployment comment
Preview DeploymentCommit: |
Preview DeploymentCommit: |
Preview DeploymentCommit: |
Description
Rename the
apps/serverdirectory toapps/apiand migrate the API to use Alchemy. Update the Alchemy configuration and scripts for improved state management and environment configuration. Add a GitHub Actions workflow for API deployment, integrating necessary secrets for secure operations.Summary by CodeRabbit
New Features
Chores