Repository navigation
feat(inbound-webhooks): cursor-paged submissions read for project keys - #259
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
BenAbdou1001
left a comment
BenAbdou1001
left a comment
There was a problem hiding this comment.
Acceptance criteria (from #239)
- 250 submissions at limit 100: three pages, each once, oldest first (e2e)
- Rows arriving between pages are neither skipped nor repeated (e2e)
- Malformed cursor is 400 (e2e, unit) - now including cursors that pass
Date.parsebut not Postgres, see fixes - Another project's webhook is 404, unknown webhook 404 (e2e, unit)
- Every read creates a
submissions.readaudit entry (e2e) - Documented; OpenAPI reference and CLI types regenerated (
pnpm docs:apire-run: no diff) - Functional reqs: project key, opaque (receivedAt, id) cursor, limit 50/200, dateFrom/dateTo, member-endpoint fields, project throttle, member endpoint untouched
Fixes pushed
- 01bd817
fix(database): rebuild the submissions cursor index only once. Production re-applies every migration on each boot (DEPLOY.md). The generatedDROP INDEX+CREATE INDEXin 0010 passed the CI re-apply check but dropped and rebuilt the index on every boot (index OID changed per run, and a plain CREATE INDEX blocks writes on the table while it builds). 0010 is now aDOblock that rebuilds only while the index still lacksid; 0004'sIF NOT EXISTSthen no-ops. Verified: three consecutive applies of the concatenated migrations keep the same OID and the 3-column definition. Entity and snapshot unchanged. - 9a6b0d9
fix(inbound-webhooks): answer 400 for a cursor Postgres cannot parse.decodeCursorvalidatesrwithDate.parse, which accepts values Postgres rejects ("1","2026-02-31 ..."), so a tampered cursor answered 500.DrizzleExceptionFilternow maps 22007/22008 (invalid datetime) to 400 like 22P02. The e2e for malformed cursors covers both values (failed with 500 before the fix).
Validation
@mcid/api: lint, typecheck, build pass;test:ci1418/1418 with coverage thresholds (one unrelated flake under parallel turbo load, passed on rerun with and without the change)- e2e
test/inbound-webhook*against Postgres 16: 74/74 - Not run locally: Redis-dependent e2e, web/cli suites (no web/cli source changed beyond generated files)
Left for a human
See the inline comments: scope enforcement (issue lists 403) and the late-commit ceiling the author already noted.
| }; | ||
| } | ||
|
|
||
| @Get(':id/submissions') |
There was a problem hiding this comment.
Owner decision: #239 lists 403 when the project lacks the scope, but nothing here declares a @RequireScope, matching the sibling GET /project/inbound-webhooks. Any key whose project owns the webhook can read its submissions. Fine if webhook ownership is the intended authorisation; otherwise add a scope to both routes in a follow-up.
| }); | ||
| const rows = found.slice(0, f.limit); | ||
| const last = rows[rows.length - 1]; | ||
| const nextCursor = |
There was a problem hiding this comment.
Follow-up, not blocking: as the PR notes, received_at is now() at insert (transaction start), so a submission whose transaction commits after a later-started one was already served past the cursor is never returned. Rare at current volume; a bigserial sequence column (or reading only rows older than a few seconds) would close it before the Event Platform relies on it for completeness.
… the project read
BenAbdou1001
left a comment
BenAbdou1001
left a comment
There was a problem hiding this comment.
Follow-up to the earlier review, both owner decisions are now fixed:
- 730088b project reads withhold rows received in the last 10 seconds, so a late-committing submission is never skipped (a sequence column would not have fixed this: it is assigned at insert, not commit).
- 9c26dad the route requires the new
read_inbound_submissionsscope and aserverIdquery parameter, as the other scoped project routes do. Contract change for the Event Platform: sendserverIdand have the scope granted.
CI green on 9c26dad.
Closes #239
What and why
Adds
GET /api/project/inbound-webhooks/:id/submissionsso a project backend can pull its own webhook's submissions incrementally with its API key, oldest first, on a stable keyset cursor. The member endpoint and its offset paging are unchanged.received_at,id).received_atis carried as Postgres text so microseconds survive; rows sharing a timestamp are ordered by id.limitdefault 50, max 200.dateFrom/dateTofilter onreceivedAt. Items haveid,payload,receivedAt,origin.submissions.readaudit entry. The controller's existing project throttle (30/min) applies.idx_inbound_webhook_submissions_webhooknow covers (webhook_id,received_at,id) (migration 0010).InboundWebhookProjectController; no new module.Acceptance criteria
pnpm docs:api,generate:api-types)How it was checked
pnpm lint,pnpm typecheck: pass (existing web warnings only)jest src/modules/inbound-webhooks: 225 passedpnpm run test:e2e -- test/inbound-webhookagainst Postgres 16: 74 passedtests/features/docs,tests/app/docs-routes.test.tsx): 716 passedpnpm build, full e2e suite, Redis-dependent e2e (left to CI)Notes
received_atis set bynow()at insert, so a transaction that commits late with an earlier timestamp than an already-served cursor can be missed. Negligible at current volumes; a sequence column would remove it.