feat(web): Add email agent support (DB, API, UI, tests)#335
feat(web): Add email agent support (DB, API, UI, tests)#335cungminh2710 merged 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryThis PR adds an inbound email agent feature: a DB migration adding
Confidence Score: 4/5Safe to merge with the non-atomic re-enable edge case addressed; the bug only triggers if the org is deleted between two DB calls. One P1 finding: the two-step enable flow can return HTTP 200 with enabled: false when the organization is deleted between the two DB calls. In practice this is unlikely, but it is a real correctness defect on the changed code path. All previous review concerns (admin guard, unused router export, error state UI) appear to have been resolved. apps/hyperlocalise-web/src/api/routes/agent-email/agent-email.route.ts — enable/re-enable control flow (lines 150–191)
|
| Filename | Overview |
|---|---|
| apps/hyperlocalise-web/src/api/routes/agent-email/agent-email.route.ts | New authenticated GET + PATCH endpoints for the email agent; admin guard and alias-generation retry loop are present, but the enable path has a non-atomic two-step DB flow that can return stale state or a misleading 200 when the org is concurrently deleted. |
| apps/hyperlocalise-web/src/api/routes/agent-email/agent-email.test.ts | Integration tests cover disabled state, first enable + alias persistence + re-enable, and member-role blocking; implicit role in the second test is a minor clarity gap. |
| apps/hyperlocalise-web/src/components/app/agent-page-content.tsx | React Query hooks for fetch and mutation with loading skeleton, error state, and toast feedback; isError is correctly surfaced to disable the switch and show an inline message. |
| apps/hyperlocalise-web/drizzle/0004_gigantic_stellaris.sql | Adds email_agent_enabled boolean (NOT NULL, default false) and nullable inbound_email_alias text with a unique index; migration looks correct for PostgreSQL NULL semantics on the unique index. |
| apps/hyperlocalise-web/src/lib/database/schema.ts | Schema columns and unique index declaration match the migration; no issues. |
| apps/hyperlocalise-web/src/api/app.ts | New route wired at /orgs/:organizationSlug/agent-email; clean and consistent with other org-scoped routes. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/hyperlocalise-web/src/api/routes/agent-email/agent-email.route.ts
Line: 150-191
Comment:
**Non-atomic re-enable returns stale `enabled: false`**
On the re-enable path (alias already set, `emailAgentEnabled = false`), `ensureInboundAlias` returns a snapshot with `emailAgentEnabled: false` from its SELECT, and the handler then issues a *second* separate UPDATE. If that second UPDATE finds no rows (e.g., the org was just deleted), the code falls through to the final `return` and responds with `{ enabled: false }` and HTTP 200 instead of the expected 404 error. The two-step read-then-write also creates a window where a concurrent disable could win, leaving the response claiming `enabled: true` while the database holds `false`.
A single idempotent `UPDATE ... SET email_agent_enabled = $1 RETURNING ...` (combined with a separate alias-generation step only when the alias is still NULL) would eliminate both issues.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/hyperlocalise-web/src/api/routes/agent-email/agent-email.test.ts
Line: 67-69
Comment:
**Implicit role assumption in enable test**
`fixture.createWorkosIdentity()` is used without an explicit role, relying on the fixture's default to be admin/owner for the PATCH to succeed. Since the PATCH now enforces admin-only (tested explicitly in the third test case), using `createWorkosIdentityWithRole("admin")` here makes the intent clear and guards against a future fixture default change silently breaking the test.
```suggestion
const identity = fixture.createWorkosIdentityWithRole("admin");
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "." | Re-trigger Greptile
Motivation
Description
0004_large_email_agent.sqland update the migration journal to addemail_agent_enabledandinbound_email_aliascolumns and a unique index oninbound_email_aliasfororganizations.schema.tsto includeemailAgentEnabledandinboundEmailAliascolumns and the corresponding unique index declaration.`createAgentEmailRoutesinagent-email.route.tsproviding authenticatedGETandPATCHendpoints that generate and persist a unique alias (with retry on unique-constraint conflicts) and returnemailAgentstate.`app.ts), add frontend support inAgentPageContentto fetch and toggle the email agent viareact-query, show loadingSkeleton, copy address to clipboard, and show toasts, and add an API integration test (agent-email.test.ts).Testing
src/api/routes/agent-email/agent-email.test.tswhich exercised enable/disable flows and alias persistence, and it passed.Codex Task