feat: Webhooks implementation and design refinement#3632
Draft
ashrafchowdury wants to merge 3 commits intomainfrom
Draft
feat: Webhooks implementation and design refinement#3632ashrafchowdury wants to merge 3 commits intomainfrom
ashrafchowdury wants to merge 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mmabrouk
reviewed
Feb 5, 2026
Member
mmabrouk
left a comment
There was a problem hiding this comment.
Reviewing solely against the new backend requirements in AGENTS.md.
Must-fix (does not comply with AGENTS backend patterns)
- Layering violations (core/db depend on API + entrypoints)
- Core imports API schemas (AGENTS: core should not depend on
apis/fastapi/*):api/oss/src/core/webhooks/service.pyimportsoss.src.apis.fastapi.webhooks.schemas
- DB layer imports API schemas (AGENTS: DB should not depend on API layer):
api/oss/src/dbs/postgres/webhooks/dao.pyimportsoss.src.apis.fastapi.webhooks.schemas
- Core imports entrypoints for worker wiring via global lazy singleton (AGENTS: wire concrete deps in
api/entrypoints/*only; avoid core -> entrypoints):api/oss/src/core/webhooks/trigger.pyimportsentrypoints.worker_webhooks
Expected direction: Router -> Service -> DAO Interface -> DAO Impl -> DB.
- DBE placement doesn’t follow the new-stack structure
- Webhook DB models are added into legacy/monolithic
api/oss/src/models/db_models.py(WebhookSubscriptionDB,WebhookEventDB,WebhookDeliveryDB). - AGENTS requires new DBEs to live under
api/oss/src/dbs/postgres/<domain>/dbes.py(plus optionaldbas.py+mappings.py).
- Returning DBEs from service/router contracts
api/oss/src/core/webhooks/service.pyreturnsWebhookSubscriptionDBand lists of DBEs.- Router response models serialize DBEs via
from_attributes = True(api/oss/src/apis/fastapi/webhooks/schemas.py). - AGENTS explicitly says: do not return DBEs from router/service contracts; introduce DTOs + mapping.
- Endpoint conventions don’t follow AGENTS patterns
AGENTS conventions:
POST /queryfor filtering/search (payload support + cursor windowing)POST /{id}/archiveandPOST /{id}/unarchivefor lifecycle transitions- response envelopes with
count+ payload
Current API uses:
GET /webhooks/for list,DELETE /webhooks/{id}for “delete” (but implementation actually archives viaarchived_at)- no
/query, noWindowing
Migration seam / coupling note
- Webhook triggering is added into legacy router:
api/oss/src/routers/variants_router.pycallstrigger_webhook(...). - AGENTS says avoid net-new features in
api/oss/src/routers/*; if deploy still lives there, treat this as a temporary migration seam and keep the coupling minimal (especially avoid core importing entrypoints to make it work).
Style/consistency (lower priority)
- API contracts are in
schemas.py; AGENTS convention ismodels.pyfor request/response models. - New code doesn’t follow the keyword-only + grouped
#argument style seen in new-stack services (not mandatory everywhere, but it’s the convention documented).
What I’d expect to align with AGENTS
api/oss/src/core/webhooks/dtos.py(+ optionalinterfaces.py) and a DAO interface.- Move DBEs into
api/oss/src/dbs/postgres/webhooks/dbes.pyand addmappings.pyto isolate ORM. - Make
api/oss/src/dbs/postgres/webhooks/dao.pyaccept/return core DTOs (no imports from API schemas). - Rework router to follow
/query+ archive/unarchive patterns (or document why webhooks is an explicit exception). - Remove core -> entrypoints worker import; keep DI at composition roots (
api/entrypoints/*).
…re/webhooks # Conflicts: # api/entrypoints/routers.py # api/oss/src/routers/variants_router.py
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
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.
This PR implements the new Webhooks feature, including backend APIs, worker tasks, and frontend UI components.
Documentation
Please read the README.md and DESIGN.md files for detailed design and implementation documentation.