✨ server: add runtime entrypoints for api and hooks - #1227
Conversation
🦋 Changeset detectedLatest commit: 060dacc The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change configures service-specific infrastructure modules and adds supervised API and hook runtime entrypoints. The entrypoints resolve secrets and dependencies, construct services, register cleanup handlers, and add startup and failure tests. ChangesService runtime setup
Estimated code review effort: 3 (Moderate) | ~30 minutes Merge Risk: 🟡 Moderate · up to The new runtime entrypoints initialize several credentialed clients concurrently, but a later startup failure may leave earlier connections or client state without cleanup until the process exits. This creates a material availability and resource-lifecycle risk that should be fixed or explicitly accepted before merging. Sequence Diagram(s)sequenceDiagram
participant APIEntryPoint
participant SecretManagerServiceClient
participant ServiceFactories
participant createAPI
participant supervise
APIEntryPoint->>SecretManagerServiceClient: Resolve API secrets
APIEntryPoint->>ServiceFactories: Initialize API dependencies
APIEntryPoint->>createAPI: Pass resolved configuration
APIEntryPoint->>supervise: Run API promise
sequenceDiagram
participant HookEntryPoint
participant SecretManagerServiceClient
participant SignerResolver
participant HookFactory
participant supervise
HookEntryPoint->>SecretManagerServiceClient: Resolve hook secrets
HookEntryPoint->>SignerResolver: Resolve required signers
HookEntryPoint->>HookFactory: Pass resolved configuration
HookEntryPoint->>supervise: Run hook promise
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## allow #1227 +/- ##
==========================================
- Coverage 73.24% 73.23% -0.02%
==========================================
Files 288 295 +7
Lines 13397 13504 +107
Branches 4610 4611 +1
==========================================
+ Hits 9813 9889 +76
- Misses 3243 3274 +31
Partials 341 341
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1897561 to
a5cc012
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5cc0120e6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| activity: { secrets: ["alchemy-webhooks-key", "onesignal-api-key", "postgres-url"] }, | ||
| block: { secrets: ["alchemy-webhooks-key", "onesignal-api-key"], signers: ["executor"] }, |
There was a problem hiding this comment.
Route Alchemy callbacks to the new hook services
When the GCP deployment enables these activity and block services, their standalone Hono apps listen on /, but server/hooks/activity.ts:214 and server/hooks/block.ts:625 still register callbacks at ${appOrigin}/hooks/activity and ${appOrigin}/hooks/block. I checked the deployment wiring: .do/app.yaml:45-50 continues routing those paths to the existing DigitalOcean server, while infra/index.ts:106-128 creates isolated Cloud Run services without any domain or path routing. Consequently the new instances never receive the Alchemy events they initialize themselves for; route these callback paths to the Cloud Run services or register their actual service URLs before enabling them.
Useful? React with 👍 / 👎.
| secret("block-alchemy-webhooks-key"), | ||
| secret("block-onesignal-api-key"), | ||
| secret("redis-url"), | ||
| signer("executor"), |
There was a problem hiding this comment.
Authorize the account used to execute block actions
On a stack provisioned from this repository, signer("executor") resolves the newly provisioned ${stack}-executor KMS key, but the contracts grant the keeper role only to acct("keeper") in contracts/script/ExaPlugin.s.sol:40, and contracts/src/ExaPlugin.sol:548-554 rejects other senders. The block hook uses this account for executeProposal, setProposalNonce, and withdraw, so every such transaction will revert unless an out-of-band role migration has already occurred; use the authorized keeper key or grant the executor address the required role as part of the deployment.
Useful? React with 👍 / 👎.
| secret("panda-sardine-api-key"), | ||
| secret("sardine-api-url"), | ||
| secret("panda-segment-write-key"), | ||
| signer("settler"), |
There was a problem hiding this comment.
Authorize the Panda settler before switching keys
When Panda processes settlement, collection, or synchronous refund events, the wallet now signs with the newly provisioned ${stack}-settler KMS key. The affected collectCredit and collectDebit selectors require KEEPER_ROLE in contracts/src/ExaPlugin.sol:390-400, and Refunder.refund is likewise keeper-only in contracts/src/Refunder.sol:28-43, while the deployment scripts grant those roles to acct("keeper"), not settler. These webhook paths will therefore revert on stacks managed solely by this repository unless the settler is explicitly granted both roles or the entrypoint continues using the keeper account.
Useful? React with 👍 / 👎.
| }, | ||
| activity: { secrets: ["alchemy-webhooks-key", "onesignal-api-key", "postgres-url"] }, | ||
| block: { secrets: ["alchemy-webhooks-key", "onesignal-api-key"], signers: ["executor"] }, | ||
| bridge: { |
There was a problem hiding this comment.
Remove Redis access from hooks that do not use it
Provisioning either the bridge or manteca service implicitly appends every entry from common to its secret list in modules() at lines 103-105, so both public-facing service accounts receive Secret Manager access to redis-url even though their entrypoints and hook constructors never read Redis. A compromise of either webhook service can therefore disclose the shared Redis credentials and access unrelated queues and application data; add redis-url only to the services that consume it or split the common secret set by module type.
Useful? React with 👍 / 👎.
a5cc012 to
c89edcc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c89edccf19
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| common: ["redis-url", "sentry-dsn"], | ||
| crema: ["redis-address", "redis-password", "redis-username"], | ||
| services: { | ||
| api: { |
There was a problem hiding this comment.
Pass the deployment domain to the new API service
When the production or base-sepolia Cloud Run API receives browser traffic, its container has APP_STACK but no APP_DOMAIN (infra/index.ts:64-68), so common/domain.js falls back to sandbox.exactly.app. The API consequently configures CORS, CSRF, Better Auth, and Persona redirects for the sandbox origin, causing requests from web.exactly.app or base-sepolia.exactly.app to be rejected or redirected incorrectly; provide the stack's actual domain to this service at runtime.
Useful? React with 👍 / 👎.
| common: ["redis-url", "sentry-dsn"], | ||
| crema: ["redis-address", "redis-password", "redis-username"], | ||
| services: { | ||
| api: { |
There was a problem hiding this comment.
Provide the activity webhook ID to the standalone API
When enqueueing an account subscription fails during a Redis outage, server/workers/subscribe/queue.ts falls back to calling Alchemy with the module-level webhookId. In the monolith this value was initialized by the colocated activity hook or ALCHEMY_ACTIVITY_ID, but the new API runs in a separate process and this service definition supplies neither; the fallback therefore throws no active webhook, and createCredential suppresses that failure, leaving the new account unsubscribed from activity events. Pass the existing alchemyActivityId config to the API service as well as the subscribe worker.
Useful? React with 👍 / 👎.
| ], | ||
| }, | ||
| activity: { secrets: ["alchemy-webhooks-key", "onesignal-api-key", "postgres-url"] }, | ||
| block: { secrets: ["alchemy-webhooks-key", "onesignal-api-key"], signers: ["executor"] }, |
There was a problem hiding this comment.
Keep scheduled block work on an always-running process
When a proposal or withdrawal is scheduled for a future unlock, server/hooks/block.ts:482 and server/hooks/block.ts:598 rely on in-process timers after the originating webhook response has finished. This definition moves that hook to a request-driven Cloud Run service whose minimum instance count is optional and whose container is not configured for always-allocated CPU; after it becomes idle, the instance may be suspended or scaled to zero, so the timer may not execute until an unrelated matching block event cold-starts the service and reloads Redis. Run this scheduler as an always-on worker or configure durable scheduled execution rather than attaching it to the webhook service lifecycle.
Useful? React with 👍 / 👎.
c89edcc to
e012e5b
Compare
e012e5b to
01c872f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01c872f552
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| secrets: ["manteca-api-key", "onesignal-api-key", "postgres-url", "segment-write-key", "webhooks-key"], | ||
| shared: ["manteca-api-url"], | ||
| }, | ||
| panda: { |
There was a problem hiding this comment.
Keep post-response hook work alive
When callbacks are directed to these standalone Cloud Run services, request-based CPU can be suspended immediately after a handler responds because the service template does not enable always-allocated CPU. This is distinct from the already-flagged block timers: the newly inspected Panda and Persona handlers acknowledge webhooks while unawaited work is still running—for example, server/hooks/panda.ts:150-154 leaves event publication pending, and server/hooks/persona.ts:462-494 leaves Pax enrollment and document upload pending—so those side effects can be abandoned despite the provider receiving a 200 response. Await this work, move it to a durable queue, or configure instance-based CPU allocation for the affected hook services.
Useful? React with 👍 / 👎.
01c872f to
54a2389
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54a23895a0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| supervise( | ||
| "panda", | ||
| Promise.all([ | ||
| signer("issuer"), |
There was a problem hiding this comment.
Authorize the KMS issuer before signing card operations
On stacks managed solely by this repository, signer("issuer") resolves the generated ${stack}-issuer KMS key, while contracts/script/IssuerChecker.s.sol:16-20 configures IssuerChecker with the unrelated acct("issuer") address. The Panda hook uses this account in signIssuerOp for collections and refunds (server/hooks/panda.ts:450,1161), and contracts/src/IssuerChecker.sol:68-72 rejects signatures not produced by the configured current or previous issuer. Thus these operations still revert even after fixing the separately flagged settler permissions; migrate IssuerChecker to the KMS address or continue signing with the authorized issuer.
Useful? React with 👍 / 👎.
54a2389 to
2e1299f
Compare
2e1299f to
2503611
Compare
2503611 to
939a778
Compare
939a778 to
5dc097a
Compare
5dc097a to
a9b4bf1
Compare
a9b4bf1 to
8e03390
Compare
7a77fdd to
6a7fcc8
Compare
6a7fcc8 to
e66ad27
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e66ad276a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| common: ["redis-url", "sentry-dsn"], | ||
| crema: ["redis-address", "redis-password", "redis-username"], | ||
| services: { | ||
| api: { |
There was a problem hiding this comment.
Forward the production domain into Pulumi
On a freshly managed production stack, .github/workflows/server-production.yaml:7-11 passes web.exactly.app to the reusable deployment, but .github/workflows/server-deploy.yaml:41 never forwards that input as exa:domain; therefore infra/index.ts:102 gives every newly enabled service APP_DOMAIN=production.exactly.app. The API then configures CORS, CSRF, passkeys, and cookies for the wrong host. The fresh evidence beyond the earlier missing-variable comment is that APP_DOMAIN now exists, but it is populated from an unset Pulumi configuration rather than the workflow's domain input.
Useful? React with 👍 / 👎.
e66ad27 to
0323f02
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0323f02004
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .then(([redis, bullmq]) => [redis, bullmq, createPoke(bullmq)] as const), | ||
| ]).then(([alchemy, database, onesignal, [redis, bullmq, poke]]) => | ||
| own( | ||
| activity({ alchemy, database, onesignal, poke, redis }), |
There was a problem hiding this comment.
Retain the activity signing key fallback
Pass the configured activity signing key into this hook, or otherwise preserve its startup fallback. The monolith supplies ALCHEMY_ACTIVITY_KEY in server/index.ts:105-108, while server/hooks/activity.ts:190-205 makes Alchemy reconciliation part of ready; this entrypoint supplies no key and infra/utils/modules.ts:31 provisions none. During a transient Alchemy dashboard failure on a Cloud Run cold start, ready rejects and supervise terminates the service even though the existing signing key could continue authenticating callbacks.
Useful? React with 👍 / 👎.
0323f02 to
47213f1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47213f1b08
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const response = await handle.app.request("/"); | ||
| expect(response.status).toBe(200); | ||
| await expect(response.json()).resolves.toStrictEqual({ status: "ok" }); |
There was a problem hiding this comment.
Exercise entrypoint cleanup in the tests
Call and await handle.close() here and in the parameterized hook success test, then assert each owned resource is closed. Both tests currently stop after the response assertions, so the newly added database, Redis, BullMQ, KMS, Secret Manager, and Segment cleanup callbacks are never executed or covered, contrary to the repository's requirement that every new server line have complete Vitest coverage. .agents/rules/server.mdL69-L71
Useful? React with 👍 / 👎.
47213f1 to
0af82ef
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0af82ef958
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| crema: ["redis-address", "redis-password", "redis-username"], | ||
| services: { | ||
| api: { | ||
| env: { ALCHEMY_ACTIVITY_ID: "alchemyActivityId" }, |
There was a problem hiding this comment.
Configure alchemyActivityId before requiring it
Adding this environment mapping makes module() call config.require("alchemyActivityId") while constructing the API service, but none of the checked stack configurations (base, base-sepolia, production, or sandbox) defines exa:alchemyActivityId, and the reusable deployment workflow does not supply it. Therefore every repository-configured pulumi up now aborts before provisioning any infrastructure; define this key for each stack or remove the requirement if the API does not consume it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 622e9622-efe4-42a6-ab84-b29f330c27b8
📒 Files selected for processing (4)
infra/utils/modules.tsserver/api/bin.tsserver/test/api/bin.test.tsserver/test/hooks/bin.test.ts
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| common: ["redis-url", "sentry-dsn"], | ||
| crema: ["redis-address", "redis-password", "redis-username"], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add trailing commas to compact configuration literals.
Several arrays and inline object literals omit an internal trailing comma. Add trailing commas consistently in this TypeScript file.
As per coding guidelines, use trailing commas in all structures. Based on learnings, this applies to TypeScript files.
Proposed formatting change
- common: ["redis-url", "sentry-dsn"],
+ common: ["redis-url", "sentry-dsn",],Also applies to: 29-30, 32-33, 36-37, 40-41, 44-46, 58-58, 62-75
Sources: Coding guidelines, Learnings
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a56ca6bdfe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ], | ||
| }, | ||
| activity: { secrets: ["alchemy-webhooks-key", "onesignal-api-key", "postgres-url"] }, | ||
| block: { secrets: ["alchemy-webhooks-key", "onesignal-api-key"], signers: ["executor"] }, |
There was a problem hiding this comment.
Fund the new transaction-signing KMS accounts
On a freshly provisioned stack, this generated executor key—and the settler key configured below—has no native balance: infra/meta.ts:189-201 only creates the KMS keys, and a repo-wide search found no step that funds their derived addresses. Both hooks pass these accounts to createWallet, whose exaSend path prepares and broadcasts ordinary EIP-1559 transactions (server/utils/wallet.ts:117-160), so even after fixing the already-flagged contract permissions, block execution and Panda collection transactions fail for insufficient gas. Fund the new addresses during migration or retain funded signer accounts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a1246b9c1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| secrets: [ | ||
| "auth-secret", |
There was a problem hiding this comment.
Populate the newly namespaced service secrets
On a freshly provisioned stack, every namespaced value generated from these lists (for example, api-auth-secret) has no readable version: infra/meta.ts:203-213 creates only secretmanager.Secret containers, while each entrypoint immediately requests versions/latest through server/utils/secret.ts:5-10. Consequently all new API and hook processes fail during startup unless an operator manually copies every existing credential into the new names. Fresh evidence beyond the previously flagged meta prerequisite is that running the meta program still creates no SecretVersion; add an automated secret migration or version-provisioning step before deploying these services.
Useful? React with 👍 / 👎.
Summary by CodeRabbit
New Features
Tests