[docs] Plan gateway triggers: research, proposal, and WP/WL/WS breakdown#4732
[docs] Plan gateway triggers: research, proposal, and WP/WL/WS breakdown#4732junaway wants to merge 4 commits into
Conversation
dosubot
Bot
added
the
size:XXL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds comprehensive design documentation for the Gateway Triggers feature—a new inbound provider-event domain peer to tools and webhooks. The PR includes a proposal establishing goals and non-goals, research documenting the Composio external trigger model and HMAC verification requirements, gap analysis enumerating extract/mimic/net-new work items, architectural pattern definitions showing mimic vs share/extract relationships, a payload mapping specification reusing webhook template resolution, a seven-unit work breakdown plan with three synchronized lenses (Work Packages DAG, Work Lanes GitButler tree, Work Streams parallel builds), an orchestration runbook with exact branch commands and subagent templates, and detailed per-WP specification and status files for WP0–WP6 covering connection extraction, events catalog, resolver promotion, subscriptions/deliveries, ingress/dispatch, and frontend UI. Also updates ChangesGateway Triggers Design Set
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds a new design-document set under docs/designs/gateway-triggers/ that researches Composio triggers, proposes the gateway-triggers architecture, enumerates the gap vs current gateway-tools/webhooks/billing patterns, and breaks delivery into Work Packages / Work Lanes / Work Streams with per-WP specs and status trackers.
Changes:
- Introduces research + proposal + gap analysis + mapping/mimics docs for the new “gateway triggers” domain (inbound provider events invoking workflows).
- Adds an execution plan (
plan.md) that models dependencies across WP/WL/WS views. - Adds per-work-package spec and status tracker docs (WP0–WP6) plus a GitButler runbook for lane/stream kickoff.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/designs/gateway-triggers/gap.md | Enumerates required build items and decisions vs current state. |
| docs/designs/gateway-triggers/mapping.md | Specifies event→workflow input mapping approach (reusing resolver semantics). |
| docs/designs/gateway-triggers/mimics.md | Maps triggers components to existing patterns (tools/webhooks/billing). |
| docs/designs/gateway-triggers/plan.md | Defines WP/WL/WS breakdown and dependency structure for implementation. |
| docs/designs/gateway-triggers/proposal.md | High-level architecture proposal and goals/non-goals for triggers. |
| docs/designs/gateway-triggers/research.md | Research on Composio triggers + internal precedents in the codebase. |
| docs/designs/gateway-triggers/wp/WL-runbook.md | GitButler lane creation runbook + subagent launch prompt templates. |
| docs/designs/gateway-triggers/wp/WP0-specs.md | WP0 spec: extract shared connections domain + migration strategy. |
| docs/designs/gateway-triggers/wp/WP0-status.md | WP0 tracker/status checklist. |
| docs/designs/gateway-triggers/wp/WP1-specs.md | WP1 spec: triggers domain skeleton + events catalog + adapter. |
| docs/designs/gateway-triggers/wp/WP1-status.md | WP1 tracker/status checklist. |
| docs/designs/gateway-triggers/wp/WP2-specs.md | WP2 spec: promote resolver into SDK + repoint webhooks. |
| docs/designs/gateway-triggers/wp/WP2-status.md | WP2 tracker/status checklist. |
| docs/designs/gateway-triggers/wp/WP3-specs.md | WP3 spec: subscriptions + deliveries schema + CRUD surface. |
| docs/designs/gateway-triggers/wp/WP3-status.md | WP3 tracker/status checklist. |
| docs/designs/gateway-triggers/wp/WP4-specs.md | WP4 spec: Composio ingress + verification + dispatch semantics. |
| docs/designs/gateway-triggers/wp/WP4-status.md | WP4 tracker/status checklist. |
| docs/designs/gateway-triggers/wp/WP5-specs.md | WP5 spec: web UI for catalog + connections. |
| docs/designs/gateway-triggers/wp/WP5-status.md | WP5 tracker/status checklist. |
| docs/designs/gateway-triggers/wp/WP6-specs.md | WP6 spec: web UI for subscription management + deliveries. |
| docs/designs/gateway-triggers/wp/WP6-status.md | WP6 tracker/status checklist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| A GitButler series is linear: each branch has exactly **one** `--anchor` parent (two parents | ||
| is a merge commit, which collapses the stack — `vibes/AGENTS.md`: "series need linear | ||
| history"). So the WP DAG must be **projected onto a tree**: every WP fan-in is resolved by |
| - **GitButler stacking caveat (from `vibes/AGENTS.md`):** keep the WL tree a true GitButler | ||
| stack (`--anchor`); do **not** sync branches by merging them into each other — a | ||
| merge-based series can collapse to a single addressable tip on unapply/re-apply. Snapshot |
| in **both** editions — EE ships the `oss/` tree and runs it from there (no copy in | ||
| `core_ee`). **Not** the parked legacy `core` tree (frozen at `park00000000`) and **not** | ||
| `core_ee` (EE-only divergence; `gateway_connections` is shared schema). See | ||
| `application/docs/designs/oss-ee-convergence/migration-chains-and-edition-switch.md`. |
| - Internal: `api/oss/src/core/tools/`, `api/oss/src/apis/fastapi/tools/router.py`, | ||
| `api/oss/src/dbs/postgres/tools/`, `api/oss/src/core/webhooks/`, | ||
| `vibes/docs/designs/gateway-tools/` |
| How to create the WL branches in **`application/`** and spin up the WS subagents. Nothing | ||
| here is executed yet — these are the exact commands and prompts to run at kickoff. |
| > You are implementing **WP{k}** of the gateway-triggers feature in the `application/` repo. | ||
| > Read your spec at `vibes/docs/designs/gateway-triggers/wp/WP{k}-specs.md` and the parent | ||
| > design docs it links (`../plan.md`, `../gap.md`, `../mapping.md`, `../mimics.md`, | ||
| > `../research.md`). |
| > `application/api/AGENTS.md` (layering, DTOs, exceptions) and the migration rule in WP0 | ||
| > (`core_oss`, not the parked `core` tree). Write acceptance tests in both editions per the |
| > Keep `vibes/docs/designs/gateway-triggers/wp/WP{k}-status.md` updated as you progress (this | ||
| > file is fine to edit — it is notes, not git). List the files you changed in your final | ||
| > report so the orchestrator can commit them to the right lane. |
|
|
||
| ## Notes | ||
| - Lanes are not created yet; this PR is the plan only. | ||
| - The migration-chain rule cross-references docs/designs/oss-ee-convergence in application/. |
| Each WP lists scope, the gap items it closes, dependencies, and the acceptance bar. "AC" | ||
| follows the house rule: ungated endpoints get acceptance tests in **both** editions (OSS | ||
| basic account, EE inline business+developer account) — see `feedback_oss_ee_test_accounts`. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
docs/designs/gateway-triggers/wp/WP0-specs.md (1)
40-52: ⚡ Quick winContracts are clearly frozen; verify the
usagereturn type and shape.The
ConnectionsGatewayInterfacecontracts are explicit and well-defined. One minor clarity point: line 49 specifiesusage(*, project_id, connection_id) -> Usagewith comment"used by tools / N subs", but the return type nameUsageis not defined in the contract block. For implementability, specify the shape (e.g.,Usage: { used_by_tools: bool, subscription_count: int }or similar).Proposed clarification
```text ConnectionsGatewayInterface: initiate_connection(*, project_id, provider, integration, ...) -> Connection get_connection_status(*, project_id, connection_id) -> Status refresh_connection(*, project_id, connection_id) -> Connection revoke_connection(*, project_id, connection_id) -> None # revoke-for-everyone (C7) list_connections(*, project_id, ...) -> list[Connection] # backs /tools|/triggers/connections - usage(*, project_id, connection_id) -> Usage # "used by tools / N subs" + usage(*, project_id, connection_id) -> Usage # "used by tools / N subs" Connection DTO: { id (ca_*), project_id, provider, integration, slug, status, ... } +Usage DTO: { used_by_tools: bool, subscription_count: int } # or similar gateway_connections columns: (unchanged from tool_connections; already domain-neutral)Alternatively, inline the shape in the return type annotation. </details> <!-- cr-comment:v1:8193eb9a098ee8a5b62c45a9 --> </blockquote></details> <details> <summary>docs/designs/gateway-triggers/wp/WP1-specs.md (1)</summary><blockquote> `34-42`: _💤 Low value_ **Clarify EventType vs Event DTO naming in the contracts.** The contracts section uses `EventType` (line 36) as the return type for `get_event()`, but the DTO section (line 41) labels it `Event DTO`. This is a minor naming inconsistency that could cause confusion during implementation. Clarify whether the type is `Event` or `EventType` and use it consistently throughout. <details> <summary>Proposed clarification</summary> ```diff ```text TriggersGatewayInterface: list_events(*, provider, integration) -> list[Event] - get_event(*, event_key) -> EventType # carries trigger_config JSON Schema + get_event(*, event_key) -> Event # carries trigger_config JSON Schema create_subscription(*, project_id, event_key, connected_account_id, trigger_config) -> "ti_*" set_subscription_status(*, trigger_id, enabled: bool) -> None delete_subscription(*, trigger_id) -> None Catalog HTTP: GET /triggers/catalog/providers/{p}/integrations/{i}/events[/{event_key}] Event DTO: { key, provider, integration, trigger_config: <JSONSchema>, ... }Alternatively, clarify the distinction if `EventType` and `Event` are intentionally different types. </details> <!-- cr-comment:v1:b13a049fd2849e5b7b3f8992 --> </blockquote></details> <details> <summary>docs/designs/gateway-triggers/wp/WP2-specs.md (1)</summary><blockquote> `35-37`: _💤 Low value_ **Clarify functional deps statement — WP2 sequences after WP1 but declares "None in-feature."** The functional deps section states "None in-feature. Root in the §1 DAG," but the header (line 3) shows WP2 anchors WL1 (the WP1 branch). This appears contradictory: WP2 is sequenced after WP1 in the lane structure, yet has no feature-level dependencies. For clarity, rephrase to note that WP2 has no *feature-specific* dependencies (it reshuffles existing webhook code, independent of trigger architecture), but its sequencing follows WP1 for organizational/testing purposes. <details> <summary>Proposed clarification</summary> ```diff ## Functional deps -None in-feature. Root in the §1 DAG. +None on WP0/WP1 feature work. (WP2 refactors existing webhook code, independent of +trigger architecture, but is sequenced after WP1 for integration/testing purposes per the +lane structure in plan.md.)Or, if "Root in the §1 DAG" means work-package DAG (distinct from lane sequencing), clarify that distinction in the preamble or cite plan.md §X more precisely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5cdd0ed4-d39d-4b9d-87ad-118cd8923bbd
📒 Files selected for processing (21)
docs/designs/gateway-triggers/gap.mddocs/designs/gateway-triggers/mapping.mddocs/designs/gateway-triggers/mimics.mddocs/designs/gateway-triggers/plan.mddocs/designs/gateway-triggers/proposal.mddocs/designs/gateway-triggers/research.mddocs/designs/gateway-triggers/wp/WL-runbook.mddocs/designs/gateway-triggers/wp/WP0-specs.mddocs/designs/gateway-triggers/wp/WP0-status.mddocs/designs/gateway-triggers/wp/WP1-specs.mddocs/designs/gateway-triggers/wp/WP1-status.mddocs/designs/gateway-triggers/wp/WP2-specs.mddocs/designs/gateway-triggers/wp/WP2-status.mddocs/designs/gateway-triggers/wp/WP3-specs.mddocs/designs/gateway-triggers/wp/WP3-status.mddocs/designs/gateway-triggers/wp/WP4-specs.mddocs/designs/gateway-triggers/wp/WP4-status.mddocs/designs/gateway-triggers/wp/WP5-specs.mddocs/designs/gateway-triggers/wp/WP5-status.mddocs/designs/gateway-triggers/wp/WP6-specs.mddocs/designs/gateway-triggers/wp/WP6-status.md
|
|
||
| - `POST /triggers/composio/events/` — read raw body **before** parsing. | ||
| - HMAC-SHA256 verify over `{id}.{ts}.{body}` with `COMPOSIO_WEBHOOK_SECRET`; 401 bad sig; | ||
| 200 no-op when secret unset; add `COMPOSIO_WEBHOOK_SECRET` to `env`. | ||
| - Recover `project_id` from `metadata.user_id`; route `metadata.trigger_id` → local | ||
| subscription; 200-skip unknown/disabled; optional `target`-style env fan-out guard (I5). | ||
| - One-time project webhook-URL registration with Composio (I6). | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file to see its full content and structure
cat -n docs/designs/gateway-triggers/wp/WP4-specs.md | head -100Repository: Agenta-AI/agenta
Length of output: 3162
🏁 Script executed:
# Check if there are any references or links to parent design documents
rg "metadata\.user_id|project_id|lookup|recovery" docs/designs/gateway-triggers/wp/WP4-specs.md -A 3 -B 3Repository: Agenta-AI/agenta
Length of output: 1132
🏁 Script executed:
# Look for related design documents that might provide context
fd . docs/designs/gateway-triggers/ -type f -name "*.md" | head -20Repository: Agenta-AI/agenta
Length of output: 231
🏁 Script executed:
# Check parent design documents referenced on lines 5-6
cat -n docs/designs/gateway-triggers/plan.md | head -150Repository: Agenta-AI/agenta
Length of output: 9053
🏁 Script executed:
# Also check mapping.md which is referenced
cat -n docs/designs/gateway-triggers/mapping.md | head -100Repository: Agenta-AI/agenta
Length of output: 4289
🏁 Script executed:
# Check if there's more detail about user_id → project_id mapping elsewhere
rg "user_id.*project_id|project_id.*user_id|metadata" docs/designs/gateway-triggers/ -A 2 -B 2Repository: Agenta-AI/agenta
Length of output: 20964
Clarify handling of missing or null metadata.user_id.
The project_id recovery mechanism is documented in research.md: metadata.user_id directly echoes back the str(project_id) that was registered with Composio (no database lookup required). However, WP4-specs does not explicitly address:
- Error handling if
metadata.user_idis absent, null, or malformed. - Whether the existing "200-skip unknown/disabled" condition applies to missing
user_id, or only to unknown/disabledtrigger_id.
This detail should be clarified in WP4-specs to ensure robust ingress handling.
|
|
||
| ## Functional deps (fan-in) | ||
|
|
||
| - **WP3** — reads the subscription, writes a `deliveries` row (DTO + DAO surface). |
There was a problem hiding this comment.
Missing dedup_seen stub dependency from WP3 DAO contract.
WP3-specs line 39 freezes dedup_seen(event_id) as part of the DAO surface for WP4. WP4-specs line 35 requires "Real metadata.id dedup against deliveries" to prevent duplicate invocations. However, WP4-specs line 45–46 only lists stubs for get_subscription_by_trigger_id and write_delivery, omitting dedup_seen.
Add dedup_seen to the stubs-needed section to match the frozen DAO contract and acknowledge the implementation dependency.
Suggested clarification
## Stubs needed (until deps merge)
- Subscription DTO/DAO (WP3) — stub `get_subscription_by_trigger_id` + `write_delivery`.
+ — stub `get_subscription_by_trigger_id` + `write_delivery` + `dedup_seen`.
- `resolve_target_fields` (WP2) — import against the frozen signature.Also applies to: 45-46
| - [ ] system `user_id` (M6) | ||
| - [ ] async dispatch (M7) | ||
| - [ ] metadata.id dedup (I4) + delivery rows + retry (M9) | ||
| - [ ] Stub WP3 DAO + WP2 resolver until merged |
There was a problem hiding this comment.
Update checklist to explicitly include dedup_seen stub dependency.
Line 23 says "Stub WP3 DAO + WP2 resolver until merged," but per the gap identified in WP4-specs, the checklist should explicitly call out that the WP3 DAO stubs include dedup_seen(event_id) alongside get_subscription_by_trigger_id and write_delivery.
For consistency with WP3-specs line 39 (which freezes all three DAO methods), update the checklist item to be explicit:
- Stub WP3 DAO (
get_subscription_by_trigger_id,write_delivery,dedup_seen) + WP2 resolver until merged
Suggested update
- [ ] Stub WP3 DAO + WP2 resolver until merged
+ [ ] Stub WP3 DAO (`get_subscription_by_trigger_id`, `write_delivery`, `dedup_seen`) + WP2 resolver until mergedAlso applies to: 13-25
Adds the gateway-triggers design set under docs/designs/gateway-triggers/: research, proposal, gap, mimics, mapping, plan, and the wp/ specs + status trackers + lane/stream runbook. Triggers are the inbound dual of webhooks: Composio provider events invoke Agenta workflows. plan.md frames the build as three views over seven units: Work Packages (functional DAG, fan-in), Work Lanes (GitButler merge tree, no fan-in), Work Streams (parallel subagent assignments against frozen contracts). No application code. WP0 notes the one migration subtlety: the connection rename lands in the shared core_oss chain, not the parked core tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document the staging model that bit us: changes assign to the stack (not a branch), so commit one lane at a time and verify each commit. Note that but rub by path goes stale after a mutation (use cliId), how to split one file across stacked lanes, branch-ref vs workspace-applied divergence, and that but push prints nothing on success (verify SHAs against the remote). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
junaway
force-pushed
the
gateway-triggers-docs
branch
from
12d1d06 to
2936e88
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
docs/designs/gateway-triggers/gap.md (1)
131-140: 💤 Low valueRefactor the out-of-scope list to reduce repeated sentence openings.
The seven bullets all begin with "No", which violates ENGLISH_WORD_REPEAT_BEGINNING_RULE.
Example restructuring: combine related non-goals into compound sentences or vary the phrasing (e.g., "The outbound
webhooksdomain is unchanged" instead of "- No merge with / routing through...").docs/designs/gateway-triggers/proposal.md (1)
76-98: 💤 Low valueRefactor the non-goals list to reduce repeated sentence openings.
Similar to the out-of-scope section in
gap.md, the nine non-goals begin with "Not" or "No", which creates repetitive phrasing (flagged by ENGLISH_WORD_REPEAT_BEGINNING_RULE).Consolidate or vary phrasing: e.g., "The outbound
webhooksdomain and workflow hooks are unchanged; triggers are a separate inbound domain" (combines lines 78–84) or "Triggers invoke workflows through the existingWorkflowsServiceseam; we do not build a parallel runner" (lines 90–91).docs/designs/gateway-triggers/research.md (1)
289-289: 💤 Low valueMinor Markdown formatting: Remove blank line in blockquote (MD028).
Line 289 has a blank line inside a blockquote block, which violates the
no-blanks-blockquoterule. This is cosmetic but worth fixing for Markdown linting compliance.docs/designs/gateway-triggers/wp/WL-runbook.md (1)
128-128: 💤 Low valueAdd language specifier to the fenced code block (MD040).
The code fence at line 128 is missing a language tag. Since the content is a draft PR body (Markdown), specify
```markdownor use a blank```(if the intent is plain text without syntax highlighting).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fd65bbaf-33d8-4349-b31a-76a6f305f824
📒 Files selected for processing (22)
AGENTS.mddocs/designs/gateway-triggers/gap.mddocs/designs/gateway-triggers/mapping.mddocs/designs/gateway-triggers/mimics.mddocs/designs/gateway-triggers/plan.mddocs/designs/gateway-triggers/proposal.mddocs/designs/gateway-triggers/research.mddocs/designs/gateway-triggers/wp/WL-runbook.mddocs/designs/gateway-triggers/wp/WP0-specs.mddocs/designs/gateway-triggers/wp/WP0-status.mddocs/designs/gateway-triggers/wp/WP1-specs.mddocs/designs/gateway-triggers/wp/WP1-status.mddocs/designs/gateway-triggers/wp/WP2-specs.mddocs/designs/gateway-triggers/wp/WP2-status.mddocs/designs/gateway-triggers/wp/WP3-specs.mddocs/designs/gateway-triggers/wp/WP3-status.mddocs/designs/gateway-triggers/wp/WP4-specs.mddocs/designs/gateway-triggers/wp/WP4-status.mddocs/designs/gateway-triggers/wp/WP5-specs.mddocs/designs/gateway-triggers/wp/WP5-status.mddocs/designs/gateway-triggers/wp/WP6-specs.mddocs/designs/gateway-triggers/wp/WP6-status.md
✅ Files skipped from review due to trivial changes (15)
- docs/designs/gateway-triggers/wp/WP3-status.md
- docs/designs/gateway-triggers/wp/WP4-status.md
- docs/designs/gateway-triggers/wp/WP6-specs.md
- docs/designs/gateway-triggers/wp/WP1-status.md
- docs/designs/gateway-triggers/wp/WP1-specs.md
- docs/designs/gateway-triggers/wp/WP2-status.md
- docs/designs/gateway-triggers/wp/WP0-status.md
- docs/designs/gateway-triggers/wp/WP2-specs.md
- docs/designs/gateway-triggers/wp/WP5-specs.md
- docs/designs/gateway-triggers/mimics.md
- docs/designs/gateway-triggers/wp/WP5-status.md
- docs/designs/gateway-triggers/wp/WP3-specs.md
- docs/designs/gateway-triggers/mapping.md
- docs/designs/gateway-triggers/wp/WP0-specs.md
- docs/designs/gateway-triggers/wp/WP4-specs.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/designs/gateway-triggers/wp/WP6-status.md
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| in **both** editions — EE ships the `oss/` tree and runs it from there (no copy in | ||
| `core_ee`). **Not** the parked legacy `core` tree (frozen at `park00000000`) and **not** | ||
| `core_ee` (EE-only divergence; `gateway_connections` is shared schema). See | ||
| `application/docs/designs/oss-ee-convergence/migration-chains-and-edition-switch.md`. |
| - **Domain** — create `core/gateway/connections/` (service + DAO + `ConnectionsGatewayInterface`) | ||
| and `dbs/postgres/gateway/connections/` (DBE + DAO + mappings). **No router.** |
| - Create `core/gateway/connections/` (service + DAO + `ConnectionsGatewayInterface`) and | ||
| `dbs/postgres/gateway/connections/` (DBE + DAO + mappings). **No router.** |
| |---|---|---|---| | ||
| | C1 | `gateway_connections` table — rename `tool_connections` (+ `uq_`/`ix_`), no data transform | extract | `dbes.py:38`; table already domain-neutral | | ||
| | C2 | Migration authored **once in the shared `core_oss` chain** (runs in both editions), **not** the parked legacy `core` tree nor EE-only `core_ee` | extract | rename op only; `core` is frozen at `park00000000`; `gateway_connections` is shared schema. See `oss-ee-convergence/migration-chains-and-edition-switch.md` | | ||
| | C3 | `core/gateway/connections/` — service + DAO + interface, **no router** | extract | from `ToolsService` connection code (`service.py:138-383`) | |
| | Aspect | before (tools-owned) | after (shared) | | ||
| |--------|----------------------|----------------| | ||
| | Table | `tool_connections` | `gateway_connections` (renamed; already domain-neutral) | | ||
| | Code | `core/tools` connection code + `ComposioToolsAdapter` auth methods | `core/gateway/connections/` + a `ConnectionsGatewayInterface` auth adapter | |
| **No `apis/fastapi/gateway/connections/` router** — the HTTP surface is the per-domain | ||
| `/tools/connections` and `/triggers/connections`, both delegating to this one service | ||
| over the same rows. |
| - **Permission** — introduce a dedicated **`VIEW_TRIGGERS`** permission (mirror the tools | ||
| triad in `api/ee/src/core/access/permissions/types.py`: add a `# Triggers` block and | ||
| register `VIEW_TRIGGERS` into the viewer `default_permissions`). Catalog routes gate on | ||
| `Permission.VIEW_TRIGGERS` — **do NOT reuse `VIEW_TOOLS`**. |
| - **`subscriptions` table** (FlagsDBA enabled/valid, DataDBA): `ti_*`, `trigger_config`, | ||
| `inputs_fields` (the mapping template), destination `references`/`selector`, the bound | ||
| **workflow ref**, **FK → `gateway_connections`**. Many per connection. | ||
| - **`deliveries` table** (modeled on `webhook_deliveries`): resolved `inputs`, workflow | ||
| `references`, `result`/`error`, plus the `metadata.id` **dedup column** (I4). |
|
|
||
| ## Functional deps (fan-in) | ||
|
|
||
| - **WP0** — `subscriptions` FKs `gateway_connections`. |
|
|
||
| ## Decisions to lock first | ||
|
|
||
| - **Idempotency store (I4)** — lean: a `metadata.id` dedup column on `deliveries`. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/designs/gateway-triggers/wp/WP5-status.md (1)
70-70: 💤 Low valueMinor style improvement: replace "exactly" with a synonym.
The word "exactly" is flagged by LanguageTool as overused. Consider replacing it with "precisely" or restructuring the sentence for clarity. For example: "This is the WP0 dep WS5 stubs until it merges" (removing the adverb entirely) or "This replaces the WP0 dep WS5 stubs once it merges."
📝 Proposed wording alternative
- shared rows). This is exactly the WP0 dep WS5 stubs until it merges; unit tests cover the + shared rows). This is the WP0 dep WS5 stubs until it merges; unit tests cover the
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 84771369-e396-4ec5-8db7-f5f8d4b1fdfe
📒 Files selected for processing (2)
docs/designs/gateway-triggers/wp/WP3-status.mddocs/designs/gateway-triggers/wp/WP5-status.md
| - [x] Migration in `core_oss` (`oss000000003`, down_revision `oss000000002`; runs in both editions) | ||
| - [x] Subscription CRUD routes + adapter calls (ti_* create / set-status / delete) | ||
| - [x] Delivery read routes (`/triggers/deliveries`, `/{id}`, `/query`) | ||
| - [x] DAO surface for WP4: `get_subscription_by_trigger_id`, `write_delivery`, `dedup_seen` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify DAO method signatures for WP4 contract fulfillment.
# Test: Check for DAO methods in the trigger_subscriptions/deliveries DAO
echo "=== Checking trigger DAO methods ==="
ast-grep --pattern $'class $_ {
$$$
get_subscription_by_trigger_id($_, $_) {
$$$
}
$$$
}'
echo -e "\n=== Checking write_delivery method ==="
ast-grep --pattern $'class $_ {
$$$
write_delivery($_, $_) {
$$$
}
$$$
}'
echo -e "\n=== Checking dedup_seen method ==="
ast-grep --pattern $'class $_ {
$$$
dedup_seen($_, $_) {
$$$
}
$$$
}'
# Fallback: Search for these methods by name in trigger-related files
echo -e "\n=== Fallback: Searching by method name ==="
rg -n 'def (get_subscription_by_trigger_id|write_delivery|dedup_seen)' --type pyRepository: Agenta-AI/agenta
Length of output: 217
🏁 Script executed:
# First, understand repository structure
echo "=== Repository structure ==="
find . -type f -name "*.py" | grep -i dao | head -20
echo -e "\n=== Looking for trigger/subscription related files ==="
find . -type f \( -name "*.py" -o -name "*.ts" -o -name "*.js" \) | grep -i trigger | head -20
echo -e "\n=== Searching for these method names across all files ==="
rg -n '(get_subscription_by_trigger_id|write_delivery|dedup_seen)' --max-count=20
echo -e "\n=== Checking WP-related design docs ==="
find . -type f -name "*.md" | grep -i "wp\|gateway" | head -20Repository: Agenta-AI/agenta
Length of output: 3213
🏁 Script executed:
# Let's also search for DAO-related classes or structures
echo "=== Searching for DAO classes ==="
rg -n 'class.*DAO|class.*Dao' --type py | head -20
echo -e "\n=== Searching for subscription/trigger related classes ==="
rg -n 'class.*[Ss]ubscription|class.*[Tt]rigger' --type py | head -20Repository: Agenta-AI/agenta
Length of output: 3597
🏁 Script executed:
# Check the subscriptions DAO file
echo "=== Checking subscriptions DAO ==="
cat -n api/ee/src/dbs/postgres/subscriptions/dao.py
echo -e "\n=== Checking WP3-status.md context ==="
cat -n docs/designs/gateway-triggers/wp/WP3-status.md | head -50
echo -e "\n=== Checking WP3-specs.md DAO requirements ==="
grep -A 5 -B 5 "get_subscription_by_trigger_id\|write_delivery\|dedup_seen" docs/designs/gateway-triggers/wp/WP3-specs.mdRepository: Agenta-AI/agenta
Length of output: 7233
DAO methods claimed in line 22 do not exist in the codebase. The subscriptions DAO (api/ee/src/dbs/postgres/subscriptions/dao.py) currently implements only create, read, and update methods. The three methods flagged as complete—get_subscription_by_trigger_id, write_delivery, and dedup_seen—are absent. The design documents (WP3-specs.md, WP3-status.md lines 28–29) outline the architectural intent (idempotent delivery upserts on event_id dedup, dedup checks), but these methods must be implemented before WP4 can proceed. Remove the checkmark on line 22 or implement these methods with proper signatures matching the contract frozen in WP3-specs.md.
| - EE permissions: added `EDIT_TRIGGERS` to EDITOR_PERMISSIONS and `RUN_TRIGGERS` to ANNOTATOR_PERMISSIONS | ||
| (parallel to `EDIT_TOOLS`/`RUN_TOOLS`) so the developer role can actually exercise subscription CRUD — | ||
| the enum values existed but were ungranted to every role except owner. See blocker note below. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify EE permission enum definitions and role assignments.
# Test 1: Check for EDIT_TRIGGERS and RUN_TRIGGERS enum definitions
echo "=== Checking for permission enum definitions ==="
rg -n -A2 -B2 'EDIT_TRIGGERS|RUN_TRIGGERS' --type py --glob '!**/test*' | head -40
# Test 2: Check for role assignment to EDITOR_PERMISSIONS and ANNOTATOR_PERMISSIONS
echo -e "\n=== Checking EDITOR_PERMISSIONS and ANNOTATOR_PERMISSIONS assignments ==="
rg -n 'EDITOR_PERMISSIONS|ANNOTATOR_PERMISSIONS' --type py -A5 -B2 | grep -A5 'TRIGGERS\|EDIT\|RUN' | head -50
# Test 3: Verify these permissions are not accidentally applied to other roles
echo -e "\n=== Checking all role definitions ==="
rg -n 'PERMISSIONS\s*=\s*\{' --type py -A20 | head -100Repository: Agenta-AI/agenta
Length of output: 2585
🏁 Script executed:
# Read the full permissions types file to see all enum definitions
head -300 api/ee/src/core/access/permissions/types.py | tail -100Repository: Agenta-AI/agenta
Length of output: 3352
🏁 Script executed:
# Check the complete EDITOR_PERMISSIONS and ANNOTATOR_PERMISSIONS definitions
rg -A 30 'class.*Permission|ANNOTATOR_PERMISSIONS\s*=' api/ee/src/core/access/permissions/types.py | head -150Repository: Agenta-AI/agenta
Length of output: 1907
🏁 Script executed:
# Broader search for any trigger-related permissions (case-insensitive)
rg -i 'trigger' api/ee/src/core/access/permissions/types.pyRepository: Agenta-AI/agenta
Length of output: 42
EDIT_TRIGGERS and RUN_TRIGGERS permissions are not defined in the codebase.
Lines 45–47 claim these permissions have been "added" to EDITOR_PERMISSIONS and ANNOTATOR_PERMISSIONS. However, neither EDIT_TRIGGERS nor RUN_TRIGGERS exist in api/ee/src/core/access/permissions/types.py. The Permission enum defines EDIT_TOOLS and RUN_TOOLS (lines 232, 253 in the permissions file), but no trigger-related equivalents.
If these permissions were intended as part of this change, they need to be defined in the Permission enum class and added to the respective role permission lists before merging. If the implementation is still pending, update the status declaration to reflect that this work is incomplete.
Also applies to: 58-60
Adds the gateway-triggers design set under docs/designs/gateway-triggers/: research, proposal, gap, mimics, mapping, plan, and the wp/ specs + status trackers + lane/stream runbook. Triggers are the inbound dual of webhooks: Composio provider events invoke Agenta workflows.
plan.md frames the build as three views over seven units: Work Packages (functional DAG, fan-in), Work Lanes (GitButler merge tree, no fan-in), Work Streams (parallel subagent assignments against frozen contracts).
No application code. WP0 notes the one migration subtlety: the connection rename lands in the shared core_oss chain, not the parked core tree.