Repository navigation
docs(adr): ADR-054 one-file integrations via module extensions, ADR-055 recurring triggers - #669
seanspeaks wants to merge 4 commits into
Conversation
…55 recurring triggers ADR-054 (Proposed): defineIntegration/defineApp/use and extension contract 2 make a production integration fit in one file. Records the decisions so far: the event-driven path ships stable in 2.0.1; the sync pieces ship under @friggframework/core/experimental until two production apps use them; handlers are (payload, ctx) with no this, and module APIs live under ctx.modules by author-chosen keys. Inlines both target files as Appendix A, with the encryption setting fixed to fieldLevelEncryptionMethod: 'kms'. ADR-055 (Proposed): one tick function per integration, a stateless slot-versus-hash due check, overlap skipped through a Process record, missed ticks not replayed. Supersedes the type: 'CRON' docs, which are corrected when the implementation lands. Adds ADR-054 cross-references to ADR-015, 018, 019 and 023, the register rows and the roadmap entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
seanspeaks
added
release
✅ Deploy Preview for friggframework-org canceled.
|
|
ADR reconciliation needed after Stack A/B — drafted with Claude Code at Sean's request Severity: must-reconcile. Stack B (#692–#700) builds overlapping pieces outside ADR-054/055. ADR-055
ADR-054
Merge order: this PR (amended) before #694 rebases; ADR-063 (#692) should cite 054/055 for |
…stance calendar triggers, trigger sets, run identity Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGC9SnFuqdjX2xH2qeu8vw
|
Added an "Amendment (ADR-063)" section to ADR-055 ( Generated by Claude Code |
Records what the defineIntegration build (#703) settled: both front doors compile to Definition + this.events on IntegrationBase; the document door's names; one trigger registry; webhook facts declared once by the module; reserved module keys, read-only ctx.integration, fields as a module export, the contract-2 webhook pipeline, and three new open questions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| | 13 | Subscription lifecycle, `instance` scope (register, renew through the scheduler, repair through the daily trigger). Needed for modules with expiring channels. Not needed for HubSpot. | core | ~150 | 4, 9 | | ||
| | — | ADR-049 phases 1-2 (#656, #657), already in flight | core | — | — | | ||
|
|
||
| Subtotal: about 1,250 LOC. |
There was a problem hiding this comment.
The experimental-tier subtotal does not match the table above it. Items 10-13 sum to about 150 + 220 + 180 + 150 = 700 LOC. Adding the per-module descriptors (~60 each) gives roughly 800-900, not "about 1,250". Because this ADR is the sizing basis for the 2.0.1 scope decision, the overstated figure could misinform maintainers.
Fix by either correcting the subtotal or updating the per-item sizes:
Subtotal: about 700 LOC (plus ~60 per module descriptor).| Subtotal: about 1,250 LOC. | |
| Subtotal: about 700 LOC (plus ~60 per module descriptor). |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 326457d. Configure here.
| const target = link | ||
| ? await modules[to].objects.contact.update(link.id, body) | ||
| : await modules[to].objects.contact.upsertBy(config.matchOn, canonical[config.matchOn], body); | ||
| await links.set({ from, id: rec.id }, { to, id: target.id }, canonical); |
There was a problem hiding this comment.
Echo guard hashes timestamps
Medium Severity
Both toCanonical mappers put updatedAt on the object passed to links.isEcho and links.set. A write always changes the other system's modified time, so the echo hash never matches and two-way sync ping-pongs on every webhook and poll.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 326457d. Configure here.
| if (new Date(theirs.properties.lastmodifieddate) > new Date(canonical.updatedAt)) { | ||
| return ctx.skip(rec, 'crm wins'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Default conflict policy unused
Medium Severity
The sample form defaults conflict to newest-wins, but push only special-cases crm-wins and even then only skips an ERP→CRM write when the CRM timestamp is newer. The default policy never compares updatedAt, so simultaneous edits are last-write-wins.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 326457d. Configure here.
…concile Records the maintainer's decision: reconcile is add-only unless prune; it runs after deploy, on installation lifecycle for config-dependent options, and from the module's admin script, with no daily trigger; one provider developer app per stage is the supported setup. Adds HubSpot's Webhooks Journal API (per-portal subscriptions, pull from a 3-day journal) as an alternative under evaluation: subscriptions.scope 'installation' plus a journal-poll trigger. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
|
| - **Add-only by default.** It creates what the union of bindings needs and removes nothing unless it is called | ||
| with `prune: true`. It never prunes a group whose installations it could not read. A dry run reports `create` | ||
| and `remove` (what pruning would do) and writes nothing. | ||
| - **When it runs.** There is no recurring trigger, so the daily reconcile is dropped. It runs at three points: |
There was a problem hiding this comment.
The amendment says "There is no recurring trigger, so the daily reconcile is dropped", but the rest of the ADR still depends on it, and the amendment also says "The rest of this ADR stands".
- §2 (
subscriptionsrow, line 209) says app-scope subscriptions are reconciled by "the daily built-in triggerfrigg.subscriptions.reconcile". - Open question 2 (line 488) keeps "the daily trigger as the fallback".
- Appendix A (a) (lines 694-695 and 710) says HubSpot subscriptions are "reconciled ... by a daily trigger" and that devtools provisions the daily
frigg.subscriptions.reconciletrigger. - ADR-055 (lines 74-75 and 198-199) defines this built-in trigger.
- The item 4 ordering (line 427) says the trigger comes first because it is the subscription safety net.
Both ADR-054 and ADR-055 claim recurring triggers ship stable in 2.0.1, so an implementer cannot tell whether the daily reconcile exists. If it does not, subscriptions missed on deploy or on installation lifecycle are never repaired. Reconcile the two: either remove the daily trigger from the main text, appendix and ADR-055, or drop the "no recurring trigger" claim from the amendment.
| - **When it runs.** There is no recurring trigger, so the daily reconcile is dropped. It runs at three points: | |
| - **When it runs.** Recurring triggers ship stable in 2.0.1 (ADR-055), so the daily `frigg.subscriptions.reconcile` trigger is kept as the safety net. In addition, the reconcile runs at three points: |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Frigg 2.0.1 ADR review (COMMENT only — not an approval)Verdict: changes needed Checklist
Blocking
Should-fix
Docs-only; no new test failures vs |
| url: crm.urls.record('deal', deal.id) }] }, | ||
| ], | ||
| }); | ||
| log.info({ dealId: deal.id, stage }, 'deal alert posted'); |
There was a problem hiding this comment.
[should-fix] Logger argument order is wrong for Frigg’s logger.
log.info({ dealId: deal.id, stage }, 'deal alert posted');Frigg’s logger (packages/core/logs/logger.js) is message-first: info(message, fields). Object-first is pino’s style and is not the Frigg contract.
Failure scenario: a developer copies this example into an integration handler → the message becomes "[object Object]" (or fields are mis-bound) and structured dealId / stage never land as fields.
Fix:
log.info('deal alert posted', { dealId: deal.id, stage });
| - An extension contributes triggers under the name `{moduleKey}.{name}` (for example | ||
| `crm.reconcile`). Framework-owned triggers use the reserved `frigg.` prefix, for example | ||
| the built-in daily `frigg.subscriptions.reconcile` that ADR-054 uses for `app`-scope | ||
| webhook subscriptions. |
There was a problem hiding this comment.
[blocking] Stale cross-reference to daily frigg.subscriptions.reconcile.
ADR-054’s amendment drops the daily reconcile (add-only; post-deploy; install lifecycle; admin script only). This ADR-055 text still presents frigg.subscriptions.reconcile as a built-in daily framework trigger.
Align with the amendment: remove the daily reconcile as a built-in recurring trigger, and point subscription repair at the three remaining call sites.
| | `routes[].receive` + `verify` + `handshake` | core router | Runs the handshake, then verification. Returns 401 with a logged reason when verification fails. Calls `receive`, enqueues one RESOLVE message for each item, and returns 200. No DB connection. This replaces the hand-written RESOLVE hop in the HubSpot extension today. | | ||
| | `resolve` | core worker | `findIntegrationByEntityExternalId(externalId, module)`. Ambiguous matches throw, as they do now. For `scope: 'instance'`, the route carries `{integrationId}` instead. | | ||
| | `normalize` + `emits` | core worker | Turns the raw batch into named events, then dispatches `{moduleKey}.{name}` to the integration's `on` map. An emitted event with no handler is acked and counted, not an error. | | ||
| | `subscriptions` | core lifecycle + ADR-055 | `app` scope (stable): reconciled on deploy (`frigg deploy` post-step), by the module script, and by the daily built-in trigger `frigg.subscriptions.reconcile`. The desired set is the union over every integration that enables the extension. `instance` scope (experimental): `register` on enable, `remove` on disable or delete, `renew` through `createSchedulerCommands` before `expiresAt`, and the daily reconcile trigger repairs anything missed. The state (`id`, `expiresAt`, `secret`) is stored in `IntegrationMapping` under the reserved key `__frigg:sub:{binding}`. That model is already encrypted, so no schema migration is needed. | |
There was a problem hiding this comment.
[blocking] Body still mandates the daily frigg.subscriptions.reconcile; the amendment drops it.
This Decision table says app-scope subscriptions are reconciled “by the daily built-in trigger frigg.subscriptions.reconcile.” The 2026-10-22/23 amendment later in this same ADR says:
- add-only by default (
prune: trueonly when explicitly asked); - no recurring trigger — daily reconcile is dropped;
- runs after
frigg deploy, on installation lifecycle, and from an admin script only.
Same stale daily-reconcile language also appears in the appendix and in ADR-055 (Related / Decision). Leaving both texts in the merged ADR means implementers of the body will provision a daily trigger the amendment removed, while operators reading the amendment will expect no daily safety net.
Failure scenario: core ships frigg.subscriptions.reconcile as a built-in daily tick (per body) or omits it (per amendment) — either way one half of this PR’s own decision is wrong in production.
Summary
Two Proposed ADRs for the one-file integration path in 2.0.1. Under ADR-043 (open PR #646), an ADR does not merge as Proposed, so this PR is opened as a draft. It needs the maintainer's review of the API shape before it is marked Accepted and merged.
defineIntegration,defineAppanduse(), plus extension contract 2. Contract 2 covers verification, tenant resolution, namespaced events, the subscription lifecycle, triggers, config fields, env, scripts and lifecycle hooks.@friggframework/core/experimentaluntil two production apps use them: objects/pages(), the resumable page runner and watermark,ctx.each/ctx.links, and instance-scope subscriptions.ctxonly, with nothis.ctxkeys. A handler is(payload, ctx)withctx = { modules: { <authorKey>: api }, config, log, telemetry, commands, integration }. The integration author chooses the module keys, and they are not part of core's interface. Every code sample is updated to this shape, and the rejected alternative is recorded.encryption: { fieldLevelEncryptionMethod: 'kms' }, and I checked them against ADR-051'sapp-definition.schema.json(PR feat(devtools): frigg validate, and make the app-definition schema match the runtime (ADR-051) #666). Actions and config options are reached through ADR-053/api/v2/integrations/*paths (PR docs(adr): ADR-052 entity proxy security, ADR-053 Management API versioning #668).frigg.prefix (frigg.subscriptions.reconcile).type: 'CRON'docs.CLAUDE.mdanddocs/guides/INTEGRATION-PATTERNS.mdare corrected when the implementation lands, not in this PR.nextyet, so their amendments are listed only in ADR-054.docs/architecture-decisions/README.mdand entries towebsite/roadmap/data/adrs.json.ADR numbers 054 and 055 are not used on
nextor on any open ADR branch.Test plan
website/roadmap/data/adrs.jsonparses, andcountmatches the number of entries.node --check.🤖 Generated with Claude Code
📦 Published PR as canary version:
2.0.0--canary.669.5f3833b.0✨ Test out this PR locally via:
npm install @friggframework/admin-scripts@2.0.0--canary.669.5f3833b.0 npm install @friggframework/core@2.0.0--canary.669.5f3833b.0 npm install @friggframework/devtools@2.0.0--canary.669.5f3833b.0 npm install @friggframework/eslint-config@2.0.0--canary.669.5f3833b.0 npm install @friggframework/prettier-config@2.0.0--canary.669.5f3833b.0 npm install @friggframework/schemas@2.0.0--canary.669.5f3833b.0 npm install @friggframework/serverless-plugin@2.0.0--canary.669.5f3833b.0 npm install @friggframework/test@2.0.0--canary.669.5f3833b.0 npm install @friggframework/ui@2.0.0--canary.669.5f3833b.0 # or yarn add @friggframework/admin-scripts@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/core@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/devtools@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/eslint-config@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/prettier-config@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/schemas@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/serverless-plugin@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/test@2.0.0--canary.669.5f3833b.0 yarn add @friggframework/ui@2.0.0--canary.669.5f3833b.0Note
Low Risk
Adds and updates architecture decision documents only; no runtime, auth, or deployment code changes.
Overview
Documentation-only PR (draft per ADR-043) proposing two architecture decisions for the 2.0.1 one-file integration path.
ADR-054 defines how integrations shrink to a single app file:
defineIntegration/defineApp/use(), extension contract 2 (verify, resolve, namespacedemits, subscriptions, triggers,module.fields, scripts, lifecycle), and(payload, ctx)handlers with module APIs underctx.modules. It records stable vs@friggframework/core/experimentalsync APIs, amends ADRs 015/018/019/023, includes an Oct-2026 amendment from #703, and inlines two targetbackend/index.jssamples in Appendix A.ADR-055 adds
Definition.triggers: one tick Lambda per integration, stateless due-instance fan-out, overlap skip via Process records, and explicit rejection of per-instance EventBridge schedules and undocumentedtype: 'CRON'events.Register updates: cross-reference lines in ADR-015, 018, 019, 023; new rows in
docs/architecture-decisions/README.md;adrs.jsoncount 31 → 33 with summaries for 054 and 055.Reviewed by Cursor Bugbot for commit 050d362. Bugbot is set up for automated code reviews on this repo. Configure here.