fix(providers): restore self-service integrations - #2045
Merged
Merged
Conversation
Use current public or personal-token flows for BodySpec, Cycling Analytics, Ultrahuman, and Wger. Stop loading providers that still require unavailable vendor onboarding or obsolete API migration. Refs #1996
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe change introduces personal-token authentication across providers, server routes, web and mobile clients, and transactional token persistence. It also migrates selected providers away from deployment credentials, updates BodySpec to PKCE OAuth, and limits production registration to providers with supported onboarding. ChangesToken authentication and provider contracts
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProviderPanel
participant tokenAuthRouter
participant Provider
participant Database
User->>ProviderPanel: Enter personal token
ProviderPanel->>tokenAuthRouter: connect(providerId, token)
tokenAuthRouter->>Provider: exchangeToken(token)
Provider-->>tokenAuthRouter: TokenSet
tokenAuthRouter->>Database: Persist connection and tokens
Database-->>ProviderPanel: success
ProviderPanel->>ProviderPanel: Refresh provider status
Assessment against linked issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
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 |
PR Summary by QodoRestore self-service provider integrations with per-user personal-token auth
AI Description
Diagram
High-Level Assessment
Files changed (46)
|
Contributor
Mobile PreviewScan to open on device:
To test on device:
|
Remove workers for unregistered providers and use the canonical Cycling Analytics ID so token-connected sync jobs are consumed. Refs #1996
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Code Review by Qodo
Context used✅ Compliance rules (platform):
227 rules✅ Skills:
fix-provider, write-tests, cloudflare 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
|
Storybook previews for This comment updates automatically on each PR push. |
Make connection persistence atomic and classify provider outages and revoked personal tokens so clients reach retry or reconnect states.
Align Cycling Analytics queue and onboarding IDs with provider.id, and surface restored BodySpec in the onboarding guide.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Assert token rejection branches, JWT causes, expiry boundaries, provider selection, and atomic persistence so authentication regressions cannot silently pass mutation CI. Share the HTTPS token metadata schema across provider and router boundaries.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/providers/provider-auth-policy.ts (1)
29-35: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winThird copy of auth-type derivation, and it has already drifted.
getAuthTypeFromSetup()requiresoauthConfig && exchangeCodefor"oauth", whilegetProviderAuthType()in src/providers/types.ts:316-329 only checksoauthConfig, andProviderModel(src/providers/provider-model.ts:48-66) implements a third variant usingReflect.get. A provider withoauthConfigbut noexchangeCodeplus amanualTokenis classified"token"here and"oauth"there. Collapse these onto the sharedgetProviderAuthType()/setup-derivation helper so policy checks and client-facing metadata cannot disagree.As per coding guidelines: "Use domain-driven modeling, focused modules, SOLID principles" and DRY — duplicate logic across modules is a code smell.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/provider-auth-policy.ts` around lines 29 - 35, Replace the local getAuthTypeFromSetup derivation with the shared getProviderAuthType/setup-derivation helper used by ProviderModel, preserving the canonical precedence and oauthConfig-only behavior. Update policy callers to use that shared result so auth policy and provider metadata cannot classify the same setup differently.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adding-a-provider.md`:
- Line 18: Update the manualToken provider example around the Personal token
entry and its token-handling flow to avoid presenting refresh tokens as access
tokens. Narrow the example wording and supported input to personal API/access
tokens, or implement a provider-specific refresh-token exchange that preserves
the returned TokenSet fields instead of discarding refreshToken and assigning a
fixed expiry.
In `@packages/mobile/app/providers/use-provider-detail-actions.ts`:
- Around line 242-251: The token-provider action silently does nothing when
tokenAuth metadata is missing. In
packages/mobile/app/providers/use-provider-detail-actions.ts lines 242-251,
update the token case to add an else branch that sets syncMessage (or the
established equivalent error state) and calls captureException; in
packages/mobile/app/providers/index.tsx lines 601-610, add the corresponding
failure branch that surfaces an error consistently with the OAuth handling and
calls captureException.
In `@packages/mobile/components/ProviderGuide.test.tsx`:
- Around line 8-22: Remove the inline react-native and expo-router vi.mock
declarations from ProviderGuide.test.tsx, and rely on the shared mocks
configured by packages/mobile/test-setup.ts. Keep the test’s existing behavior
and imports unchanged.
In `@packages/server/src/routers/sync-registration.test.ts`:
- Around line 176-178: Update the expected provider ID in the registeredIds
assertion within the sync registration test to use CyclingAnalyticsProvider’s
canonical value, cycling_analytics, while preserving the other expected IDs.
In `@packages/server/src/routers/token-auth.ts`:
- Around line 81-101: Wrap the queryCache.invalidateByPrefix call after
connectProviderWithTokens in a try/catch; log the cache-invalidation error and
report it to Sentry via captureException(error), then continue returning {
success: true } because persistence already succeeded. Keep the existing
connection-save error handling unchanged.
In `@packages/web/src/components/DataSourcesAuthModals.stories.tsx`:
- Around line 85-97: Add Storybook coverage for TokenAuthModal's error and
loading states alongside PersonalToken. Configure the existing tRPC mock-link
pattern in DataSourcesAuthModals.stories.tsx so tokenAuth.connect returns an
error for PersonalTokenError and remains pending for a separate loading story,
while preserving the current provider props and callbacks.
In `@packages/web/src/components/DataSourcesAuthModals.tsx`:
- Around line 210-212: Update the conditional error message rendered by
DataSourcesAuthModals to include role="alert" on its existing div, preserving
the current styling and error text so screen readers announce token-connection
failures.
In `@packages/web/src/components/DataSourcesPanel.tsx`:
- Around line 341-350: Update the "token" branch in the provider handling switch
to fail fast with an explicit error when p.tokenAuth is missing, instead of
silently falling through to break. Preserve the existing setTokenAuthProvider
behavior when tokenAuth is present.
In `@src/providers/automated-login.test.ts`:
- Around line 341-351: Remove the duplicated Ultrahuman client token-rejection
and manualToken label assertions from automated-login.test.ts, and place them in
ultrahuman.test.ts alongside the existing 404 coverage. Keep the 401/403 status
cases and expected authentication_failed message intact, while leaving
automated-login.test.ts focused only on automated-login responsibilities.
In `@src/providers/cycling-analytics.ts`:
- Around line 146-192: The token resolution flow in `#resolveTokens` must support
manual Cycling Analytics tokens when OAuth configuration is unavailable. Follow
the loadTokens() and manual-token refresh approach used by wger.ts, and stop
routing these tokens through resolveOAuthTokens with
cyclingAnalyticsOAuthConfig(); preserve direct use of unexpired stored tokens
and ensure non-expiring manual tokens remain usable without client credentials.
In `@src/providers/types.ts`:
- Line 328: Update the auth classification logic in src/providers/types.ts at
line 328 to define and document explicit precedence for providers exposing both
oauthConfig and manualToken, ensuring the intended personal-token path remains
available. In src/providers/provider-model.ts at lines 57-64, always parse and
populate tokenAuth from manualToken independently of the winning authType so
clients still receive the token label and instructions URL.
In `@src/providers/ultrahuman.test.ts`:
- Around line 147-169: Add an explicit call-count assertion after each
getDailyMetrics await in the delegated-email and token-owner tests, verifying
mockFetch was called exactly once; apply the same correction to the analogous
test around the line 185-191 case. Keep the existing URL assertions unchanged.
In `@src/providers/ultrahuman.ts`:
- Around line 205-210: Update the token object returned by the non-expiring
personal-token flow to set expiresAt to null instead of a far-future sentinel
date. Apply the same change to the corresponding pattern in
cycling-analytics.ts, preserving the existing accessToken, refreshToken, and
scopes values.
- Around line 228-234: Update the Ultrahuman sync token-loading call in the
provider’s sync flow to pass options.userId as the third argument to loadTokens,
and fail fast when options.userId is missing before loading credentials;
preserve the existing missing-token error and client construction behavior.
- Around line 140-145: Update the response-status check in the Ultrahuman
request flow to throw ProviderTokenRejectedError only for 401 and 403; remove
404 from this token-rejection branch so a no-data daily_metrics response does
not mark the connection for reconnect.
In `@src/providers/wger-sync.integration.test.ts`:
- Around line 57-65: Extract the duplicated fakeJwt helper into
src/providers/test-helpers.ts and export it. In
src/providers/wger-sync.integration.test.ts lines 57-65 and
src/providers/wger.test.ts lines 84-90, remove the local fakeJwt definitions and
import the shared helper from ./test-helpers.ts.
---
Outside diff comments:
In `@src/providers/provider-auth-policy.ts`:
- Around line 29-35: Replace the local getAuthTypeFromSetup derivation with the
shared getProviderAuthType/setup-derivation helper used by ProviderModel,
preserving the canonical precedence and oauthConfig-only behavior. Update policy
callers to use that shared result so auth policy and provider metadata cannot
classify the same setup differently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c786b05f-c5fb-44d3-a49a-9cedb60a24c9
📒 Files selected for processing (57)
README.mddocs/adding-a-provider.mddocs/bodyspec.mdpackages/mobile/app/providers/[id].test.tsxpackages/mobile/app/providers/[id].tsxpackages/mobile/app/providers/auth-modals.test.tsxpackages/mobile/app/providers/auth-modals.tsxpackages/mobile/app/providers/index.test.tsxpackages/mobile/app/providers/index.tsxpackages/mobile/app/providers/provider-card.tsxpackages/mobile/app/providers/styles.tspackages/mobile/app/providers/use-provider-detail-actions.tspackages/mobile/components/ProviderGuide.test.tsxpackages/onboarding/src/provider-guide.test.tspackages/onboarding/src/provider-guide.tspackages/server/src/router.test.tspackages/server/src/router.tspackages/server/src/routers/sync-helpers.tspackages/server/src/routers/sync-registration.test.tspackages/server/src/routers/sync.test.tspackages/server/src/routers/sync.tspackages/server/src/routers/token-auth.test.tspackages/server/src/routers/token-auth.tspackages/web/src/components/DataSourcesAuthModals.stories.tsxpackages/web/src/components/DataSourcesAuthModals.test.tsxpackages/web/src/components/DataSourcesAuthModals.tsxpackages/web/src/components/DataSourcesPanel.test.tsxpackages/web/src/components/DataSourcesPanel.tsxpackages/web/src/components/DataSourcesSyncTypes.tspackages/web/src/components/ProviderGuide.test.tsxsrc/db/tokens.integration.test.tssrc/db/tokens.test.tssrc/db/tokens.tssrc/jobs/provider-queue-config.test.tssrc/jobs/provider-queue-config.tssrc/jobs/provider-registration.test.tssrc/jobs/provider-registration.tssrc/providers/README.mdsrc/providers/auth-errors.tssrc/providers/automated-login.test.tssrc/providers/bodyspec.test.tssrc/providers/bodyspec.tssrc/providers/cycling-analytics.test.tssrc/providers/cycling-analytics.tssrc/providers/oauth-providers.test.tssrc/providers/provider-auth-policy-unit.test.tssrc/providers/provider-auth-policy.tssrc/providers/provider-model.test.tssrc/providers/provider-model.tssrc/providers/types.test.tssrc/providers/types.tssrc/providers/ultrahuman-sync.integration.test.tssrc/providers/ultrahuman.test.tssrc/providers/ultrahuman.tssrc/providers/wger-sync.integration.test.tssrc/providers/wger.test.tssrc/providers/wger.ts
💤 Files with no reviewable changes (4)
- src/jobs/provider-queue-config.ts
- packages/server/src/routers/sync-helpers.ts
- src/providers/oauth-providers.test.ts
- src/jobs/provider-registration.ts
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Owner
Author
|
Follow-up to the CodeRabbit review: #2045 (review) All 16 inline findings now have explicit replies and resolved threads. The outside-diff finding is also fixed in 5e5504c: provider-auth-policy now uses the shared setup classifier, while ProviderModel and public auth metadata use the same canonical precedence and complete-OAuth contract. The two suggestions intentionally not applied are documented in their threads with contract/runtime evidence:
Local validation at this exact head is green: full unit/mobile suite (13,804 passed), root/server/web/mobile typechecks, full lint, Storybook build, real-Compose Wger/Ultrahuman integration tests (18 passed), and targeted mutation testing (51/51 killed, 100%). |
7 tasks
This was referenced Jul 27, 2026
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.
Summary
Validation
Validated on Node 26.5.0 at exact head
5e5504c5a22c36765e885386221589a03af87400:pnpm typecheckpnpm --filter dofek-server typecheckpnpm --filter dofek-web typecheckpnpm --filter dofek-mobile typecheckpnpm lintgit diff --checkExact-head CI: https://github.com/Asherlc/dofek/actions/runs/30216187551
Fixes #1996
Summary by CodeRabbit
New Features
Bug Fixes
Documentation