Conversation
When OAuth2Requester.refreshAuth fails and Module.markCredentialsInvalid flips Credential.authIsValid to false, Integration.status was left as ENABLED. The queue worker only short-circuits on DISABLED, so every subsequent webhook kept getting processed, failed again, and re-entered the loop — producing 94k+ error lines/day for Attio integrations with dead tokens. This fix uses Frigg's existing Delegate pattern to propagate the failure upward: 1. Module fires a new CREDENTIAL_INVALIDATED event from markCredentialsInvalid (best-effort, try/catch so it never alters refreshAuth's documented `return false` contract). 2. IntegrationBase.receiveNotification catches the event and calls updateIntegrationStatus.execute(this.id, 'DISABLED'). 3. The delegate wire (module.delegate = this) is installed in IntegrationBase._appendModules, which covers all 7 code paths that construct Integration instances (HTTP reads, queue workers, create/update/delete flows). Recovery is handled by the Gap C fix (PR #574): ProcessAuthorizationCallback.restoreIntegrationsForEntity flips {ERROR, DISABLED} back to ENABLED on successful re-auth. This is Gap B of 3 from the Attio dead-token RCA. Gap A (refresh short-circuit when refresh_token is null) is PR #575. 6 unit tests covering: delegate propagation, null-delegate backward compat, status flip, unknown-delegate no-op, missing-id guard, and module-delegate wiring at construction. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
d-klotz
added
release
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1636cbaa51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| console.log( | ||
| `[Frigg] Module ${notifier?.name || '?'} reported invalid credentials for integration ${this.id} — marking DISABLED` | ||
| ); | ||
| await this.updateIntegrationStatus.execute(this.id, 'DISABLED'); |
There was a problem hiding this comment.
Disabling the integration unconditionally on every CREDENTIAL_INVALIDATED event will also disable it for transient refresh failures, because OAuth2Requester.refreshAuth() emits DLGT_INVALID_AUTH for any caught error (including network/5xx issues), and Module.receiveNotification() maps that directly to markCredentialsInvalid(). With this new status flip, a temporary outage in the token endpoint can now move healthy integrations to DISABLED and stop all webhook processing until manual reauth, which is a functional regression from this change path.
Useful? React with 👍 / 👎.
…ker discard
Per Frigg team lead feedback: DISABLED is reserved for user-intent
("turn off integration, keep settings"). System-driven credential
failures should use ERROR to preserve the semantic distinction.
Changes:
- IntegrationBase.receiveNotification: DISABLED -> ERROR
- backend-utils.js queue worker: extend status discard check at
both lines 157 and 173 from `=== 'DISABLED'` to
`['DISABLED', 'ERROR'].includes(status)` so ERROR integrations
are also short-circuited (no wasted webhook processing)
- Updated log messages to show actual status dynamically
- Added test for ERROR discard in integration-defined-workers
Gap C (PR #574) already resets both {ERROR, DISABLED} on re-auth
via STATUSES_RESET_ON_REAUTH — no change needed there.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…gate-wiring comment Cleans up an internal Quo-specific reference in IntegrationBase._appendModules for upstream clarity. No behavior change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| * Receives notifications from modules (the Delegate pattern) when | ||
| * something integration-level needs attention. Today this catches the | ||
| * `CREDENTIAL_INVALIDATED` event Module fires from `markCredentialsInvalid` | ||
| * and flips this integration's status to DISABLED so the queue worker |
There was a problem hiding this comment.
Documentation inconsistency: Comment states the integration status will be flipped to "DISABLED" but the actual implementation sets it to "ERROR" (line 568). While both statuses are functionally handled the same way in backend-utils.js, this misleading comment could cause confusion during future maintenance.
Correct the comment to:
// and flips this integration's status to ERROR so the queue worker| * and flips this integration's status to DISABLED so the queue worker | |
| * and flips this integration's status to ERROR so the queue worker |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.
|
1 participant
Summary
When
OAuth2Requester.refreshAuthfails andModule.markCredentialsInvalidflipsCredential.authIsValidtofalse,Integration.statuswas left asENABLED. The queue worker only short-circuits onDISABLED, so every subsequent webhook kept getting processed, failed again, and re-entered the error loop -- producing 94k+ error lines/day for Attio integrations with dead tokens.This PR uses Frigg's existing
Delegatepattern to propagate the failure upward:CREDENTIAL_INVALIDATEDevent frommarkCredentialsInvalid(best-effort try/catch so it never altersrefreshAuth's documentedreturn falsecontract)receiveNotificationmethod and callsupdateIntegrationStatus.execute(this.id, 'ERROR')module.delegate = this) is installed inIntegrationBase._appendModules, covering all 7 code paths that construct Integration instances (HTTP reads, queue workers, create/update/delete flows)backend-utils.js) now discards on bothDISABLEDandERRORstatuses (previously onlyDISABLED)Status semantics (per Frigg team lead feedback)
This is Gap B of 3 from the Attio dead-token RCA:
refresh_tokenis null): PR fix(core): narrow OAuth2Requester isRefreshable when refresh is impossible #575Recovery is handled by Gap C:
ProcessAuthorizationCallback.restoreIntegrationsForEntityflips{ERROR, DISABLED}back toENABLEDon successful re-auth, so Gaps B + C are complementary.Files changed
packages/core/modules/module.jsDLGT_CREDENTIAL_INVALIDATEDconstant +this.notify(...)at end ofmarkCredentialsInvalid(), wrapped in try/catchpackages/core/integrations/integration-base.jsmodule.delegate = thiswiring in_appendModulesloop + newreceiveNotification()handler that flips status toERRORpackages/core/handlers/backend-utils.js=== 'DISABLED'to['DISABLED', 'ERROR'].includes(status)at both check points (lines 157, 173)Test plan
7 unit tests across 4 files (all green, 19 suites / 171 tests full regression clean):
CREDENTIAL_INVALIDATEDto its delegate after credential flipdelegate=nullcompletes silently (backward compat)ERRORonCREDENTIAL_INVALIDATEDthis.idis falsy (not hydrated)IntegrationBase._appendModulessetsmodule.delegate = thisfor all modulesERROR🤖 Generated with Claude Code