From cfba3146e24cd91fe19d7dc8886bb2eb269771a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Fri, 8 May 2026 00:10:49 +0200 Subject: [PATCH] Add deepen-connected-services architecture artifacts Reshape the sync-providers seam before Komoot (web-login) and Apple Health (device) adapters land. Captures the decisions in three ADRs, seeds CONTEXT.md with Connected Services vocabulary, and proposes the OpenSpec change covering schema rename + ConnectedServiceManager + capability seams (Importer / RoutePusher / WebhookReceiver). Co-Authored-By: Claude Opus 4.7 (1M context) --- CONTEXT.md | 126 ++++++++++++++ ...ected-services-credential-discriminator.md | 19 ++ .../0002-no-unified-syncprovider-interface.md | 19 ++ docs/adr/0003-routepusher-seam-shape.md | 18 ++ .../deepen-connected-services/.openspec.yaml | 2 + .../deepen-connected-services/design.md | 163 ++++++++++++++++++ .../deepen-connected-services/proposal.md | 36 ++++ .../specs/connected-services/spec.md | 46 +++++ .../specs/wahoo-import/spec.md | 58 +++++++ .../specs/wahoo-route-push/spec.md | 59 +++++++ .../deepen-connected-services/tasks.md | 49 ++++++ 11 files changed, 595 insertions(+) create mode 100644 CONTEXT.md create mode 100644 docs/adr/0001-connected-services-credential-discriminator.md create mode 100644 docs/adr/0002-no-unified-syncprovider-interface.md create mode 100644 docs/adr/0003-routepusher-seam-shape.md create mode 100644 openspec/changes/deepen-connected-services/.openspec.yaml create mode 100644 openspec/changes/deepen-connected-services/design.md create mode 100644 openspec/changes/deepen-connected-services/proposal.md create mode 100644 openspec/changes/deepen-connected-services/specs/connected-services/spec.md create mode 100644 openspec/changes/deepen-connected-services/specs/wahoo-import/spec.md create mode 100644 openspec/changes/deepen-connected-services/specs/wahoo-route-push/spec.md create mode 100644 openspec/changes/deepen-connected-services/tasks.md diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..e41990c --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,126 @@ +# trails.cool domain glossary + +This file names the domain concepts used in the codebase. New terms get added +here as decisions crystallize during architecture work; the goal is that one +concept has one name everywhere — specs, code, conversations. + +If you're naming a new module, a new column, or a new UI surface, look here +first. If the term you need isn't here, propose it (don't invent a synonym). + +--- + +## Connected Services + +The user-facing surface for linking external accounts and devices to a Journal +account. Spec: `openspec/specs/connected-services/`. + +### ConnectedService +A single linked external account or device, owned by one user. Stored in the +`connected_services` table (renamed from `sync_connections`). At most one row +per `(user_id, provider)`. + +### provider +String identifier for the external system: `wahoo`, `komoot`, `apple-health`, +and future `coros`, `garmin`, `strava`. The provider determines the +`credential_kind` and which capabilities (import / push / webhook) the +connection has, via the provider's manifest. + +### credential kind +Discriminator on `connected_services` describing the credential shape stored +in the `credentials` JSONB blob. Three kinds today: + +- **oauth** — OAuth2 access token, refresh token, expiry. Wahoo, and the + expected shape for Coros / Garmin / Strava. +- **web-login** — email + encrypted password + session jar. Komoot. No + official API; we authenticate against the provider's normal web login and + reuse the resulting session cookies. Refresh = re-login. Web-login + breakage (form changes, captchas, password rotation) surfaces at the + import layer, not at the credential layer. +- **device** — no remote credential. Apple Health (and future Health Connect + on Android). Data arrives via authenticated mobile API uploads, not + server-initiated pulls. The `credentials` blob is empty; the connection + exists so the UI can show "Apple Health is paired." + +Credential kind is determined by the provider via its manifest, but stored +explicitly on the row so queries don't need to join the manifest. + +### granted_scopes +Column on `connected_services`, populated only for `credential_kind = oauth`, +NULL otherwise. Lists the OAuth scopes the user actually granted (e.g. +`routes_write`). Feature gates query this column directly; missing a scope +triggers re-authorization. + +### provider_user_id +The external service's identifier for the user. Used to route incoming +webhooks to the right local user. Nullable (Apple Health has none in the +remote-id sense). + +### CredentialAdapter +Per-kind module that knows how to maintain credentials of that kind: + +- `oauth.refresh(creds) → creds | NeedsRelink` +- `web-login.relogin(creds) → creds | InvalidCredentials` +- `device` — no-op + +Adapters do not import data, push routes, or handle webhooks. They only own +the credential lifecycle. + +### ConnectedServiceManager +The deep module callers see. Owns: + +- `link(userId, provider, credentials)` / `unlink(serviceId)` +- `withFreshCredentials(serviceId, fn)` — refreshes via the right + `CredentialAdapter` if expired, calls `fn(creds)`, marks the connection + `needs_relink` if refresh fails. +- `markNeedsRelink(serviceId, reason)` — called by import / push / webhook + layers when they observe a credential failure (e.g. Komoot web-login + fails, Wahoo returns 401 after a successful refresh). + +Per-provider importers / pushers / webhook handlers always go through +`withFreshCredentials` — they never read the `credentials` JSONB directly. + +### provider manifest +Per-provider declaration co-located with the provider's code +(`providers/wahoo/manifest.ts`, etc.). Declares: + +- the provider's `credential_kind` +- which capabilities the provider implements (import? push? webhook?) +- references to the per-capability modules + +A small `providers/registry.ts` imports each manifest. Adding a provider is +one new directory plus one import line. + +## Sync Capabilities + +Three orthogonal capabilities a provider may implement. Each is its own seam +when there are ≥2 adapters; today most are single-adapter and held to a +named shape so the second adapter doesn't reshape the interface. + +### Importer +Pulls workouts / activities from the external service into the Journal. +Wahoo (OAuth pull), Komoot (web-login pull), Apple Health (mobile-pushed) all +implement this, with very different mechanics. Dedup via `sync_imports`. + +### RoutePusher +Pushes a Journal route out to the external service. One adapter today +(Wahoo); the seam exists so Coros / Garmin / Strava push won't reshape it. + +The public seam is `pushRoute(connectedService, route) → {remoteId, version}`. +Provider-specific concerns — FIT Course conversion, the `route:` +`external_id` convention, the PUT→POST-on-404 fallback — are **handled +internally by the adapter**, not exposed on the seam. Idempotency is tracked +via `sync_pushes`. + +### WebhookReceiver +Handles inbound webhooks. Today: Wahoo workout-published. Routes incoming +webhooks to the right user via `provider_user_id`. Unknown +`provider_user_id` returns 200 and is silently dropped (no leak). + +## Storage tables + +- `connected_services` — user ↔ provider links (renamed from + `sync_connections`). +- `sync_imports` — dedup cache for imported workouts, keyed by + `(user_id, provider, workout_id)`. +- `sync_pushes` — push state per `(user_id, route_id, provider)` → + `remote_id`, `last_pushed_version`. diff --git a/docs/adr/0001-connected-services-credential-discriminator.md b/docs/adr/0001-connected-services-credential-discriminator.md new file mode 100644 index 0000000..836f46c --- /dev/null +++ b/docs/adr/0001-connected-services-credential-discriminator.md @@ -0,0 +1,19 @@ +# Connected Services use a credential-kind discriminator + JSONB blob + +trails.cool integrates with external services whose credentials have +fundamentally different shapes: OAuth2 (Wahoo, future Coros/Garmin/Strava), +web-login session (Komoot — no official API, we authenticate against the +provider's web login with email + password and reuse the cookie jar), +and device pairing (Apple Health, future Health Connect — no remote +credential at all). We considered an OAuth-only schema with nullable extras +and per-kind tables, and rejected both: nullable-OAuth would force Komoot +and Apple Health to leave most columns NULL and bake OAuth assumptions into +queries; per-kind tables would fragment `connected_services` and complicate +the user-facing "Connected Services" list. + +We store all linked external accounts in one `connected_services` table +with a `credential_kind ∈ {oauth, web-login, device}` discriminator and a +`credentials` JSONB blob whose schema is determined by kind. OAuth-specific +`granted_scopes` stays a top-level column because feature gates query it +directly. Per-kind `CredentialAdapter` modules own the credential lifecycle +(refresh / relogin / noop); callers never read the JSONB directly. diff --git a/docs/adr/0002-no-unified-syncprovider-interface.md b/docs/adr/0002-no-unified-syncprovider-interface.md new file mode 100644 index 0000000..9feac38 --- /dev/null +++ b/docs/adr/0002-no-unified-syncprovider-interface.md @@ -0,0 +1,19 @@ +# No unified SyncProvider interface; capabilities are separate seams + +The `connected-services` spec calls for a "provider-agnostic framework," and +the obvious shape is a single `SyncProvider` interface with `connect / +refresh / importOne / pushRoute / handleWebhook`. We rejected this because +the providers we actually need to support are heterogeneous: Komoot is +import-only via web-login (no refresh, no webhooks, no push), Apple Health +is mobile-pushed (no server-initiated anything, no remote credential), +Wahoo has all four. A unified interface would be OAuth-shaped by default +and adapters would fill it with NOPs — shallow at the seam, leaky at the +adapters. + +Instead, capabilities are separate seams: `Importer`, `RoutePusher`, +`WebhookReceiver`. Each provider declares which capabilities it implements +in a co-located manifest (`providers//manifest.ts`); a small +`registry.ts` imports each manifest. Credential lifecycle is the one thing +shared across providers, and lives in `ConnectedServiceManager` + +`CredentialAdapter` (per kind). If we ever genuinely need pan-provider +behaviour, we add it to the manager — not to a provider interface. diff --git a/docs/adr/0003-routepusher-seam-shape.md b/docs/adr/0003-routepusher-seam-shape.md new file mode 100644 index 0000000..c6f0b20 --- /dev/null +++ b/docs/adr/0003-routepusher-seam-shape.md @@ -0,0 +1,18 @@ +# RoutePusher seam takes (service, route); workarounds stay inside adapters + +`RoutePusher` has one adapter today (Wahoo), but Coros, Garmin, and Strava +push are all foreseeable. The current Wahoo push code exposes its +workarounds — FIT Course conversion, the deterministic `external_id = +route:` convention, the PUT→POST-on-404 fallback when a user has +deleted the route on the remote side — at the call site, which would force +every future pusher to either inherit those Wahoo-isms or reshape the +seam. + +We commit to the seam shape now, with one adapter: `pushRoute(service, +route) → {remoteId, version}`. Format conversion, idempotency tricks, and +provider-specific HTTP recovery live entirely inside the adapter. The +`sync_pushes` table (`(user_id, route_id, provider) → remote_id, +last_pushed_version`) is the cross-provider contract for idempotency; the +adapter is responsible for honouring it but not for exposing how. When the +second pusher lands, it implements the same shape without changing +callers. diff --git a/openspec/changes/deepen-connected-services/.openspec.yaml b/openspec/changes/deepen-connected-services/.openspec.yaml new file mode 100644 index 0000000..8d87be1 --- /dev/null +++ b/openspec/changes/deepen-connected-services/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-07 diff --git a/openspec/changes/deepen-connected-services/design.md b/openspec/changes/deepen-connected-services/design.md new file mode 100644 index 0000000..4dc3ce4 --- /dev/null +++ b/openspec/changes/deepen-connected-services/design.md @@ -0,0 +1,163 @@ +## Context + +`apps/journal/app/lib/sync/` today uses a unified `SyncProvider` interface (`types.ts:88-118`) with one adapter (Wahoo). The interface bakes OAuth into its shape — `getAuthUrl`, `exchangeCode`, `refreshToken`, `parseWebhook`, plus optional `pushRoute? / updateRoute? / revoke?`. Token-refresh logic lives inline in `pushes.server.ts` (~lines 145-170). The CRUD layer (`connections.server.ts`) is a thin wrapper over the `sync_connections` table. + +Two new providers are imminent and each violates the OAuth assumption: **Komoot** (no official API; we authenticate via the website's login form and reuse session cookies — `credential_kind = web-login`) and **Apple Health** (no remote credential at all; data arrives via authenticated mobile API uploads — `credential_kind = device`). Hand-merging either into the current shape produces NOPs (`refreshToken`, `parseWebhook`) and OAuth-shaped queries. + +Three architectural decisions have already been recorded: +- ADR-0001: credential-kind discriminator + JSONB blob (not OAuth-only schema, not per-kind tables). +- ADR-0002: capability seams (`Importer` / `RoutePusher` / `WebhookReceiver`), not a unified `SyncProvider`. +- ADR-0003: `RoutePusher` interface shape is `(service, route) → {remoteId, version}`; Wahoo workarounds stay inside the adapter. + +Domain vocabulary is in `CONTEXT.md` (Connected Services section). + +## Goals / Non-Goals + +**Goals:** +- Reshape the `sync_connections` storage and the `SyncProvider` interface *before* Komoot or Apple Health code lands, so neither has to distort the architecture. +- Centralize credential lifecycle (refresh / mark-needs-relink) in one module so future providers don't reinvent it. +- Make capability seams (`Importer`, `RoutePusher`, `WebhookReceiver`) testable independently with a fake `ConnectedServiceManager`. +- Commit to a `RoutePusher` shape with one adapter today, so Coros / Garmin / Strava push won't reshape it. +- Carry all three credential kinds (`oauth | web-login | device`) in the schema enum from day one to avoid follow-up migrations. + +**Non-Goals:** +- Adding the Komoot importer or `web-login` `CredentialAdapter` (lands with the amended `komoot-import` change). +- Adding the Apple Health adapter or `device` `CredentialAdapter` (lands with `mobile-app`). +- Changing user-visible behaviour (settings page, OAuth flows, import / push semantics, webhook handling, idempotency rules). +- Touching `sync_imports` or `sync_pushes` schema. Only `sync_connections` is renamed. +- Defining cross-provider behaviour beyond credential lifecycle. `Importer` and `WebhookReceiver` are typed seams but their per-provider mechanics stay heterogeneous. + +## Decisions + +### Decision 1: One `connected_services` table, polymorphic credentials + +`sync_connections` is renamed to `connected_services`. New columns: +- `credential_kind text NOT NULL` — discriminator: `oauth | web-login | device`. +- `credentials jsonb NOT NULL` — shape determined by `credential_kind`. For `oauth`: `{access_token, refresh_token, expires_at}`. For `web-login` (later): `{email, encrypted_password, session_jar}`. For `device` (later): `{}`. + +Top-level columns kept: `user_id`, `provider`, `provider_user_id` (nullable), `granted_scopes` (nullable text[], populated only for `oauth`), `created_at`. Old columns `access_token / refresh_token / expires_at` are dropped after backfill. + +**Why JSONB + discriminator instead of nullable OAuth columns or per-kind tables**: see ADR-0001. Short version: nullable OAuth makes Komoot and Apple Health leave half the columns NULL; per-kind tables fragment the user-facing "Connected Services" list and complicate the registry. + +**`granted_scopes` stays a top-level column** because feature gates (`if (!granted_scopes.includes('routes_write')) re-auth`) read it on every push. Promoting it out of JSONB keeps gate queries cheap and untyped JSONB indexing unnecessary. + +### Decision 2: Capability seams, not a unified provider + +`SyncProvider` is replaced by three interfaces, each implementable independently: + +```ts +interface Importer { + listImportable(service, page): Promise; + importOne(service, id): Promise; +} + +interface RoutePusher { + pushRoute(service, route): Promise<{remoteId, version}>; +} + +interface WebhookReceiver { + parseWebhook(body, headers): WebhookEvent | null; + handle(event): Promise; +} +``` + +A `Provider` is a manifest co-located with its code: + +```ts +// providers/wahoo/manifest.ts +export const wahoo: ProviderManifest = { + id: 'wahoo', + credentialKind: 'oauth', + oauth: { /* getAuthUrl, exchangeCode, scopes */ }, + importer: wahooImporter, + pusher: wahooPusher, + webhookReceiver: wahooWebhook, +}; +``` + +`providers/registry.ts` imports each manifest. Adding a provider is one new directory plus one import line. Per ADR-0002, there is no pan-provider behaviour interface — the manager owns credential lifecycle, capability adapters own everything else. + +**Why this over a unified interface with optional methods**: Wahoo's current interface has three optional methods already (`pushRoute? / updateRoute? / revoke?`); the codebase has been informally drifting toward capability seams. Formalizing it makes "does Komoot push? does Apple Health webhook?" a manifest-level fact rather than a runtime `if (provider.pushRoute)` check. + +### Decision 3: `ConnectedServiceManager` owns credential lifecycle + +```ts +class ConnectedServiceManager { + link(userId, providerId, kind, credentials): Promise; + unlink(serviceId): Promise; + withFreshCredentials(serviceId, fn: (creds) => Promise): Promise; + markNeedsRelink(serviceId, reason): Promise; +} +``` + +`withFreshCredentials` is the chokepoint. It loads the row, checks if the credential is expired (per-kind logic via `CredentialAdapter`), refreshes if needed, calls `fn(credentials)`, and on credential failure flips `status = needs_relink`. Importers, pushers, and webhook handlers always go through this — they never read the `credentials` JSONB directly. + +**Why centralize**: today's refresh logic lives inline in `pushes.server.ts` (~lines 145-170). Komoot's relogin and Apple Health's noop need the same chokepoint. One module = one place where credential bugs and races are caught. + +### Decision 4: `oauth` `CredentialAdapter` ships now; `web-login` and `device` are stubs + +```ts +interface CredentialAdapter { + refresh(credentials): Promise; + revoke?(credentials): Promise; +} +``` + +The `oauth` adapter ships with this change and contains the existing Wahoo-style refresh-token flow plus optional revoke (for providers that support `DELETE /v1/permissions`). The `web-login` and `device` adapters are not implemented in this change — they land with their respective consumer changes. The discriminator enum carries all three values from day one so the consumer changes don't need a migration. + +### Decision 5: `RoutePusher` shape commits to (service, route) → result + +Per ADR-0003, the seam is: + +```ts +interface RoutePusher { + pushRoute(service: ConnectedService, route: Route): Promise<{remoteId: string, version: number}>; +} +``` + +Wahoo's adapter handles internally: +- FIT-Course conversion via `gpxToFitCourse`. +- The `external_id = route:` convention. +- The PUT-vs-POST decision based on `sync_pushes.remote_id`. +- The PUT→POST-on-404 fallback when the user deleted the Wahoo route on their side. +- The `data:application/vnd.fit;base64,...` body encoding. + +Idempotency tracking via `sync_pushes (user_id, route_id, provider) → remote_id, last_pushed_version` is the **shared** contract — every adapter must read/write this table — but how it's used (PUT vs POST, fallback) is adapter-internal. + +### Decision 6: Spec deltas are renames + capability-seam terminology + +Three spec files are touched: +- `connected-services/spec.md`: rename `sync_connections` → `connected_services` everywhere; add a requirement codifying the capability-seam model (replacing the implicit "OAuth tokens stored in" framing). +- `wahoo-import/spec.md`: rename. The "Provider-agnostic sync framework" requirement is rewritten to reference capability seams + manifest registration instead of "implement the `SyncProvider` interface in a single file." +- `wahoo-route-push/spec.md`: rename only. User-visible behaviour and idempotency rules are unchanged. + +## Risks / Trade-offs + +- **[Risk]** Migration runs on production `connected_services` rows (sandbox era — small, but non-zero) — botched backfill could leave Wahoo connections unusable until manual repair. → **Mitigation**: migration is a single transaction; backfill is purely column-shape (no value transformation beyond moving three columns into a JSON object); deploy with a dry-run on staging first; rollback = re-run the inverse migration restoring columns from JSON. + +- **[Risk]** OpenSpec change `komoot-import` already drafts a `journal.integrations` table — any in-progress implementation against that drafted spec becomes wasted work. → **Mitigation**: this change documents the supersession explicitly. The `komoot-import` design.md will be amended in a separate, small follow-up before any Komoot code lands; if Komoot work has already started against the old shape, it is small and isolated (one importer file, no shipped DB). + +- **[Trade-off]** Carrying `web-login` and `device` in the `credential_kind` enum without their adapters means an empty enum branch ships. The alternative (add the values when their adapters land) costs an extra migration each. → **Accepted**: enum-only forward declaration is cheap; migration churn is not. + +- **[Trade-off]** `RoutePusher` is a single-adapter seam today. Per ADR-0003 we commit to its shape now to avoid a reshape when Coros/Garmin/Strava push lands. **Accepted** because the shape is small (`(service, route) → {remoteId, version}`) and Wahoo specifics already live behind it. + +- **[Risk]** Test reorganization (existing `wahoo.test.ts` and `pushes.server.test.ts` split into `importer.test.ts` / `pusher.test.ts` / `webhook.test.ts` / `manager.test.ts`) loses git blame continuity. → **Mitigation**: do the file moves with `git mv` in the same commit as the code split; reviewers can use `--follow` to trace history. + +## Migration Plan + +Single deployable PR ("spec apply") containing: + +1. **DB migration** (Drizzle): rename `sync_connections` → `connected_services`; add `credential_kind text NOT NULL` (default `'oauth'` only for the duration of backfill, then drop default); add `credentials jsonb NOT NULL`; add `status text NOT NULL DEFAULT 'active'`; backfill `credentials = jsonb_build_object('access_token', access_token, 'refresh_token', refresh_token, 'expires_at', expires_at)`; drop `access_token / refresh_token / expires_at`. +2. **New module** `apps/journal/app/lib/connected-services/` (manager + oauth credential adapter + registry + types). +3. **Wahoo split**: `apps/journal/app/lib/sync/providers/wahoo.ts` is moved to `apps/journal/app/lib/connected-services/providers/wahoo/{importer.ts, pusher.ts, webhook.ts, manifest.ts, oauth.ts}`. The old `sync/` directory is deleted. +4. **Caller updates**: routes (`api.sync.connect.*`, `api.sync.disconnect.*`, `api.sync.webhook.wahoo`), background jobs, and the route push action update their imports to go through the manager + registry. +5. **Test reorganization**: `wahoo.test.ts` (285 lines) and `pushes.server.test.ts` (309 lines) split into capability-aligned test files; new `manager.test.ts` covers refresh / needs_relink / link / unlink. +6. **Spec deltas** applied at archive time. + +**Rollback**: revert the PR. The inverse migration restores `access_token / refresh_token / expires_at` from the JSONB blob and renames the table back. Production has small row count, so the inverse runs in seconds. + +## Open Questions + +- **Q1**: Should `connected_services.status` be an enum (`active | needs_relink | revoked`) at the DB level, or a free-form text column with constants in code? → **Lean toward enum** (Postgres `text` with a `CHECK` constraint) so a stray status value can't slip in. Decide during implementation. +- **Q2**: Where does the `oauth` `CredentialAdapter` live — `lib/connected-services/credential-adapters/oauth.ts` or co-located inside `providers/wahoo/`? Wahoo's OAuth-specific config (token endpoint URL, client id) needs to be accessible to the adapter. → **Lean toward shared adapter at `credential-adapters/oauth.ts`** taking the provider-specific config from the manifest. Decide during implementation. +- **Q3**: Does the `oauth` adapter handle revoke universally (call `provider.revokeUrl` if defined), or is revoke a Wahoo-internal concern? → **Lean toward shared with optional config**, since Strava and Garmin also have revoke endpoints. diff --git a/openspec/changes/deepen-connected-services/proposal.md b/openspec/changes/deepen-connected-services/proposal.md new file mode 100644 index 0000000..9c4b9c2 --- /dev/null +++ b/openspec/changes/deepen-connected-services/proposal.md @@ -0,0 +1,36 @@ +## Why + +The current `SyncProvider` interface (`apps/journal/app/lib/sync/types.ts:88-118`) is OAuth-shaped: it bakes `getAuthUrl`, `exchangeCode`, `refreshToken`, `parseWebhook`, and `pushRoute?` into one fat interface. Wahoo is the only adapter today, and the abstraction is a hypothetical seam. With **Komoot** (web-login, no official API) and **Apple Health** (mobile-pushed, no remote credential) both in flight, that seam is about to break: a unified interface fills with NOPs and forces every future provider to inherit OAuth assumptions. + +This change reshapes the seam *before* the second and third adapters land — so Komoot and Apple Health fit cleanly instead of distorting the architecture. Decisions are recorded in `docs/adr/0001-0003`; vocabulary in `CONTEXT.md`. + +## What Changes + +- **BREAKING (DB)**: rename `journal.sync_connections` → `journal.connected_services`. Add `credential_kind` text column (discriminator: `oauth | web-login | device`) and `credentials` JSONB column. Backfill existing Wahoo rows: `credential_kind='oauth'`, move `access_token / refresh_token / expires_at` into the JSONB blob. Drop the old token columns. +- **BREAKING (code)**: replace `SyncProvider` interface with three capability seams: `Importer`, `RoutePusher`, `WebhookReceiver`. Each provider declares which capabilities it implements via a co-located manifest (`providers//manifest.ts`); a small `providers/registry.ts` imports each. +- **New module** `ConnectedServiceManager` at `apps/journal/app/lib/connected-services/manager.ts` owning credential lifecycle: `link / unlink / withFreshCredentials / markNeedsRelink`. Centralizes token-refresh logic currently inlined in `pushes.server.ts` (~lines 145-170). +- **New per-kind module** `CredentialAdapter`. `oauth` adapter ships with this change (`refresh / revoke`). `web-login` adapter lands with `komoot-import`; `device` adapter lands with `mobile-app`. The `credential_kind` enum carries all three from day one to avoid a follow-up migration. +- **`RoutePusher` seam shape**: `pushRoute(service, route) → {remoteId, version}`. Wahoo's FIT-Course conversion, the `route:` `external_id` convention, and the PUT→POST-on-404 fallback stay **inside the Wahoo adapter** — they are not part of the seam (per ADR-0003). +- Wahoo's existing code splits into `providers/wahoo/{importer.ts, pusher.ts, webhook.ts, manifest.ts}`; existing tests reorganize accordingly. +- **Spec deltas**: rename `sync_connections` → `connected_services` in `connected-services/spec.md`, `wahoo-import/spec.md`, `wahoo-route-push/spec.md`. Add capability-seam terminology to `connected-services/spec.md`. + +## Capabilities + +### New Capabilities + +(none — this change reshapes existing capabilities, it does not introduce new user-visible behaviour.) + +### Modified Capabilities + +- `connected-services`: requirement-level changes — credential storage shape (`credential_kind` discriminator + JSONB), provider model (capability seams instead of unified `SyncProvider`), token-refresh policy (centralized via `ConnectedServiceManager.withFreshCredentials`). +- `wahoo-import`: rename of underlying table and refactor of how the importer obtains credentials (via manager, not via direct table read). User-visible behaviour unchanged. +- `wahoo-route-push`: rename of underlying table; `RoutePusher` seam shape codified. User-visible behaviour unchanged. + +## Impact + +- **Code**: `apps/journal/app/lib/sync/` is replaced by `apps/journal/app/lib/connected-services/` (manager + credential adapters + registry) and `apps/journal/app/lib/connected-services/providers/wahoo/` (split capability modules). Routes and jobs that read sync data update their imports. +- **DB**: one migration — rename + add columns + backfill + drop legacy columns. Production `connected_services` row count is small (sandbox era); backfill is online-safe. +- **Tests**: `wahoo.test.ts` (285 lines) and `pushes.server.test.ts` (309 lines) reorganize to test capabilities (`importer.test.ts`, `pusher.test.ts`, `webhook.test.ts`) against a fake `ConnectedServiceManager` yielding stub credentials. New `manager.test.ts` covers refresh / needs_relink transitions. +- **Specs**: 3 spec files updated (table rename + terminology). No removals. +- **Out of scope** (separate follow-on changes): `komoot-import` (will be amended to use this architecture instead of its drafted `journal.integrations` table); `mobile-app` (Apple Health provider lands there). Neither is touched by this change beyond a reference. +- **Decision references**: `docs/adr/0001-connected-services-credential-discriminator.md`, `docs/adr/0002-no-unified-syncprovider-interface.md`, `docs/adr/0003-routepusher-seam-shape.md`, `CONTEXT.md` (Connected Services section). diff --git a/openspec/changes/deepen-connected-services/specs/connected-services/spec.md b/openspec/changes/deepen-connected-services/specs/connected-services/spec.md new file mode 100644 index 0000000..f27fe4a --- /dev/null +++ b/openspec/changes/deepen-connected-services/specs/connected-services/spec.md @@ -0,0 +1,46 @@ +## MODIFIED Requirements + +### Requirement: OAuth token storage in `connected_services` +External-service credentials SHALL be stored in the `journal.connected_services` table (renamed from `sync_connections`) keyed by `(user_id, provider)`. Each row SHALL persist a `credential_kind` discriminator (`oauth | web-login | device`), a `credentials` JSONB blob whose schema is determined by the kind, the provider-side user id (`provider_user_id`, nullable for kinds without one), and OAuth-specific `granted_scopes` as a top-level column (populated only when `credential_kind = 'oauth'`). Disconnecting SHALL delete the row, severing the user's link to the external service without affecting any imported activities. + +For `credential_kind = 'oauth'`, the `credentials` blob SHALL contain `access_token`, `refresh_token`, and `expires_at`. Other kinds (web-login, device) carry their own blob shapes and are introduced by their respective consumer changes; the enum value is reserved from day one so adding a kind does not require a migration. + +#### Scenario: Wahoo connect persists tokens +- **WHEN** a user completes the Wahoo OAuth flow +- **THEN** a `connected_services` row is upserted with `provider = 'wahoo'`, `credential_kind = 'oauth'`, the `credentials` JSONB containing access/refresh tokens and `expires_at`, the provider user id, and the granted scopes + +#### Scenario: Disconnect removes the row but keeps imports +- **WHEN** a user clicks "Disconnect" on a Wahoo connection +- **THEN** the matching `connected_services` row is deleted; previously imported activities are not deleted (they remain owned by the user, just no longer auto-syncing) + +#### Scenario: Each user has at most one row per provider +- **WHEN** a user reconnects an already-connected provider +- **THEN** the existing `connected_services` row is updated in place with the fresh credentials; no duplicate row is created + +## ADDED Requirements + +### Requirement: Capability seams for providers +The system SHALL model each external provider as a manifest declaring its `credential_kind` and which capabilities it implements. Capabilities are modelled as separate interfaces — `Importer`, `RoutePusher`, `WebhookReceiver` — and a provider implements only the subset that applies to it. There SHALL NOT be a unified provider interface that lists every capability with optional methods. + +#### Scenario: Add a new provider +- **WHEN** a developer adds a new provider (e.g. Garmin) +- **THEN** they create `providers//manifest.ts` declaring the provider's `credential_kind` and the capability adapters it implements +- **AND** they implement only the relevant capability interfaces (e.g. an OAuth-pull-only provider implements `Importer` only; not `RoutePusher` or `WebhookReceiver`) +- **AND** they register the manifest by importing it in `providers/registry.ts` +- **AND** existing settings UI, webhook routing, and credential lifecycle work without further changes + +### Requirement: Centralized credential lifecycle via `ConnectedServiceManager` +The system SHALL expose a `ConnectedServiceManager` that owns credential lifecycle for all providers. Importers, pushers, and webhook handlers SHALL obtain credentials exclusively via `withFreshCredentials(serviceId, fn)`, which loads the row, refreshes via the kind-appropriate `CredentialAdapter` if expired, calls `fn(credentials)`, and flips `status = needs_relink` if refresh fails. Capability adapters SHALL NOT read the `credentials` JSONB directly. + +#### Scenario: Expired OAuth token is refreshed transparently +- **WHEN** a capability adapter calls `withFreshCredentials` for a `connected_services` row whose `expires_at` has passed +- **THEN** the manager invokes the OAuth `CredentialAdapter`'s refresh flow +- **AND** updates the `credentials` JSONB with the new tokens and `expires_at` +- **AND** invokes `fn` with the fresh credentials +- **AND** the capability adapter never sees the expired token + +#### Scenario: Refresh failure flips status to needs_relink +- **WHEN** the `CredentialAdapter`'s refresh call returns a permanent failure (e.g. revoked refresh token) +- **THEN** the manager sets `status = 'needs_relink'` on the `connected_services` row +- **AND** raises an error that the caller surfaces as a re-connect prompt +- **AND** subsequent calls for the same service short-circuit until the user re-links diff --git a/openspec/changes/deepen-connected-services/specs/wahoo-import/spec.md b/openspec/changes/deepen-connected-services/specs/wahoo-import/spec.md new file mode 100644 index 0000000..2993552 --- /dev/null +++ b/openspec/changes/deepen-connected-services/specs/wahoo-import/spec.md @@ -0,0 +1,58 @@ +## MODIFIED Requirements + +### Requirement: Provider-agnostic sync framework +The system SHALL provide capability-shaped seams for external activity sync providers. Each provider declares a manifest at `providers//manifest.ts` listing its `credential_kind` (`oauth | web-login | device`) and the capability adapters it implements (`Importer`, `RoutePusher`, `WebhookReceiver`). There SHALL NOT be a unified `SyncProvider` interface containing every capability with optional methods. + +#### Scenario: Add new provider +- **WHEN** a developer wants to add a new sync provider (e.g., Garmin) +- **THEN** they create `providers/garmin/` containing a `manifest.ts` declaring `credential_kind` and the implemented capabilities +- **AND** they implement only the capability interfaces that apply (e.g. `Importer` for an import-only provider) +- **AND** they register the manifest in `providers/registry.ts` +- **AND** OAuth flows, webhook routing, and settings UI work without further changes + +### Requirement: Connect Wahoo account +Users SHALL be able to connect their Wahoo account via OAuth2. + +#### Scenario: Connect Wahoo +- **WHEN** a user clicks "Connect Wahoo" in journal settings +- **THEN** they are redirected to Wahoo's OAuth authorization page with scopes `workouts_read`, `user_read`, `offline_data`, `routes_write` +- **AND** after granting permission, redirected back to the journal +- **AND** access and refresh tokens are stored in `connected_services` (in the `credentials` JSONB blob, with `credential_kind = 'oauth'`) +- **AND** the granted scopes are recorded in `connected_services.granted_scopes` so feature gates can detect missing scopes without round-tripping to Wahoo + +#### Scenario: Disconnect Wahoo +- **WHEN** a user clicks "Disconnect" next to their Wahoo connection +- **THEN** the stored credentials are deleted from `connected_services` + +#### Scenario: Token refresh +- **WHEN** a Wahoo API call fails with an expired token +- **THEN** the OAuth `CredentialAdapter` is invoked via `ConnectedServiceManager.withFreshCredentials` to obtain a new access token automatically +- **AND** the new tokens are written back to the `credentials` JSONB blob + +#### Scenario: Existing connection without routes_write +- **WHEN** a user connected before the `routes_write` scope was added attempts an action that requires it (such as pushing a route) +- **THEN** the system detects the missing scope from `connected_services.granted_scopes` and routes the user through OAuth re-authorization to grant the new scope +- **AND** the existing tokens remain valid until re-auth completes; ongoing read flows (workout import, webhook ingestion) continue to work + +### Requirement: Webhook-based automatic sync +New Wahoo workouts SHALL be automatically imported when they complete. + +#### Scenario: Webhook receives new workout +- **WHEN** Wahoo sends a `workout_summary` webhook to `/api/sync/webhook/wahoo` +- **THEN** the system identifies the user via `provider_user_id` +- **AND** downloads the FIT file from Wahoo's CDN (without auth headers, as CDN URLs are pre-signed) +- **AND** converts it to GPX +- **AND** creates a journal activity with the GPX, stats, and PostGIS geometry +- **AND** records the import in `sync_imports` to prevent duplicates + +#### Scenario: Webhook for workout without file +- **WHEN** a webhook arrives for a workout with no FIT file URL +- **THEN** the activity is created without GPX or geometry + +#### Scenario: Duplicate webhook +- **WHEN** a webhook arrives for a workout already imported +- **THEN** the import is skipped silently (idempotent) + +#### Scenario: Unknown user webhook +- **WHEN** a webhook arrives with a `provider_user_id` not matching any connection +- **THEN** the request is ignored with a 200 response (don't reveal user existence) diff --git a/openspec/changes/deepen-connected-services/specs/wahoo-route-push/spec.md b/openspec/changes/deepen-connected-services/specs/wahoo-route-push/spec.md new file mode 100644 index 0000000..d687ef0 --- /dev/null +++ b/openspec/changes/deepen-connected-services/specs/wahoo-route-push/spec.md @@ -0,0 +1,59 @@ +## MODIFIED Requirements + +### Requirement: Send to Wahoo action on route detail page +The Journal SHALL show a "Send to Wahoo" button on the route detail page when all of the following hold: the viewer owns the route, the viewer has a connected Wahoo account, and the route has stored geometry (`routes.geom` is non-null and `routes.gpx` is non-empty). The button SHALL trigger a server action that pushes the current route version to Wahoo. + +#### Scenario: Owner with connected Wahoo and a route with geometry +- **WHEN** the route owner loads a route detail page for a route that has geometry +- **AND** the owner has a `connected_services` row with `provider = 'wahoo'` +- **THEN** a "Send to Wahoo" button is visible alongside the existing "Export GPX" action + +#### Scenario: Owner without a connected Wahoo account +- **WHEN** the route owner loads a route detail page +- **AND** the owner has no Wahoo `connected_services` row +- **THEN** the "Send to Wahoo" button is not rendered + +#### Scenario: Non-owner viewing the route +- **WHEN** any visitor who is not the route owner loads the route detail page +- **THEN** the "Send to Wahoo" button is not rendered, regardless of the viewer's own Wahoo connection + +#### Scenario: Route without geometry +- **WHEN** the route owner loads a route detail page for a route whose `geom` is null or whose `gpx` is empty +- **THEN** the "Send to Wahoo" button is not rendered + +### Requirement: Re-auth flow when `routes_write` scope is missing +The Journal SHALL detect when a connected Wahoo account lacks the `routes_write` scope before calling Wahoo, redirect the user through OAuth to grant it, and resume the push automatically after the user returns. + +#### Scenario: Existing connection lacks routes_write +- **WHEN** the route owner clicks "Send to Wahoo" +- **AND** the user's `connected_services.granted_scopes` does not include `routes_write` +- **THEN** the server redirects the user to Wahoo's authorization URL with the full updated scope list and a `state` that encodes `{ return_to: , push_after: true }` +- **AND** no Wahoo `/v1/routes` call is attempted + +#### Scenario: Push completes after re-auth +- **WHEN** the user returns from Wahoo's OAuth callback with `push_after = true` in the state +- **THEN** the connection is updated with the new scopes +- **AND** the push action runs automatically against the route encoded in `return_to` +- **AND** the user lands back on the route detail page with the "Sent to Wahoo" confirmation visible + +#### Scenario: User declines the new scope +- **WHEN** the user reaches Wahoo's authorization page and clicks "Deny" +- **THEN** the user is redirected back to the route detail page with an inline notice "Sending to Wahoo needs route permission — please reconnect your account in Settings" +- **AND** no `sync_pushes` row is created + +## ADDED Requirements + +### Requirement: `RoutePusher` capability seam +The system SHALL expose `RoutePusher` as the capability seam through which any provider pushes routes. The seam shape is `pushRoute(service, route) → {remoteId, version}`. Provider-specific concerns — file format conversion (e.g. FIT Course), `external_id` conventions, HTTP fallback strategies, idempotency-tracking semantics — SHALL be handled inside the adapter and SHALL NOT appear on the `RoutePusher` interface. + +#### Scenario: Wahoo pusher implements the seam without leaking workarounds +- **WHEN** the route push action invokes the Wahoo `RoutePusher` +- **THEN** the seam is called as `pushRoute(connectedService, route)` and returns `{remoteId, version}` +- **AND** the FIT-Course conversion, the `external_id = route:` convention, the PUT-vs-POST decision, and the PUT→POST-on-404 fallback are all internal to the Wahoo adapter +- **AND** callers do not pass FIT data or Wahoo-specific fields across the seam + +#### Scenario: Future pusher reuses the same seam +- **WHEN** a developer adds a second `RoutePusher` (e.g. Garmin) +- **THEN** they implement `pushRoute(service, route) → {remoteId, version}` with the same shape +- **AND** the route push action invokes it identically to the Wahoo pusher +- **AND** their format conversion and HTTP recovery live inside the adapter diff --git a/openspec/changes/deepen-connected-services/tasks.md b/openspec/changes/deepen-connected-services/tasks.md new file mode 100644 index 0000000..d5a4d38 --- /dev/null +++ b/openspec/changes/deepen-connected-services/tasks.md @@ -0,0 +1,49 @@ +## 1. Database migration + +- [ ] 1.1 Update Drizzle schema in `packages/db/src/schema/journal.ts`: rename `syncConnections` table to `connectedServices`, add `credentialKind` (text, NOT NULL) and `credentials` (jsonb, NOT NULL) and `status` (text, NOT NULL DEFAULT `'active'` with CHECK constraint for `active | needs_relink | revoked`); drop `accessToken`, `refreshToken`, `expiresAt`. +- [ ] 1.2 Generate migration via `pnpm db:generate`; hand-edit if needed so backfill runs in the same transaction as the column adds (`UPDATE connected_services SET credential_kind = 'oauth', credentials = jsonb_build_object('access_token', access_token, 'refresh_token', refresh_token, 'expires_at', expires_at)`), then drop the old columns. +- [ ] 1.3 Verify the migration on a local database with seeded Wahoo connection rows; verify the inverse rollback restores the columns from JSONB. +- [ ] 1.4 Update FK references in `sync_imports` and `sync_pushes` if any reference the table name (likely none — both reference `users` / `routes`, not connections). + +## 2. ConnectedServiceManager + OAuth CredentialAdapter + +- [ ] 2.1 Create `apps/journal/app/lib/connected-services/types.ts` with `ConnectedService` record type, `CredentialKind`, `Credentials` (discriminated union per kind), `NeedsRelink` error class, `Status` enum. +- [ ] 2.2 Create `apps/journal/app/lib/connected-services/credential-adapters/oauth.ts` implementing `CredentialAdapter` with `refresh(creds, providerConfig) → Credentials | NeedsRelink` and optional `revoke`. +- [ ] 2.3 Create `apps/journal/app/lib/connected-services/manager.ts` exposing `link / unlink / withFreshCredentials / markNeedsRelink`. Persistence via Drizzle on the `connectedServices` table. +- [ ] 2.4 Write `manager.test.ts`: in-memory tests with stub `CredentialAdapter` covering refresh-on-expired, refresh-fails-→-needs_relink, link/unlink, markNeedsRelink, status short-circuit. +- [ ] 2.5 Write `credential-adapters/oauth.test.ts`: contract tests against a mocked token endpoint (refresh success, refresh 4xx → NeedsRelink). + +## 3. Provider manifest + registry + +- [ ] 3.1 Create `apps/journal/app/lib/connected-services/registry.ts` and `types.ts` capability interfaces (`Importer`, `RoutePusher`, `WebhookReceiver`, `ProviderManifest`). +- [ ] 3.2 Create `apps/journal/app/lib/connected-services/providers/wahoo/manifest.ts` declaring `credentialKind: 'oauth'`, OAuth config (auth URL, token URL, scopes), and references to capability adapters. +- [ ] 3.3 Wire `providers/registry.ts` to import the Wahoo manifest. Expose `getManifest(providerId)` and `getAllManifests()`. + +## 4. Wahoo capability adapters + +- [ ] 4.1 Move and reshape Wahoo importer logic into `providers/wahoo/importer.ts` implementing the `Importer` seam (`listImportable`, `importOne`). Internally calls `withFreshCredentials` for any auth'd Wahoo HTTP. Keep FIT→GPX conversion isolated. +- [ ] 4.2 Move and reshape Wahoo route push logic into `providers/wahoo/pusher.ts` implementing `RoutePusher` (`pushRoute(service, route) → {remoteId, version}`). Keep FIT-Course conversion, `external_id = route:`, the PUT-vs-POST decision, and the PUT→POST-on-404 fallback **inside** the adapter. Read/write `sync_pushes` per the existing idempotency rules. +- [ ] 4.3 Move and reshape Wahoo webhook handling into `providers/wahoo/webhook.ts` implementing `WebhookReceiver` (`parseWebhook`, `handle`). +- [ ] 4.4 Reorganize tests: split existing `wahoo.test.ts` (285 lines) and `pushes.server.test.ts` (309 lines) into `providers/wahoo/{importer.test.ts, pusher.test.ts, webhook.test.ts}` against a fake `ConnectedServiceManager` that yields stub credentials. Use `git mv` first, then split, so blame survives. + +## 5. Caller migration + +- [ ] 5.1 Update OAuth connect routes (`/api/sync/connect/wahoo`, callback) to call `ConnectedServiceManager.link` with `credentialKind: 'oauth'` and the JSONB credentials shape. Remove direct writes to the old token columns. +- [ ] 5.2 Update disconnect route (`/api/sync/disconnect/wahoo`) to call `ConnectedServiceManager.unlink`. +- [ ] 5.3 Update webhook route (`/api/sync/webhook/wahoo`) to look up the manifest via the registry and dispatch to its `WebhookReceiver`. Unknown `provider_user_id` still returns 200. +- [ ] 5.4 Update the route push action and any background jobs (`apps/journal/app/jobs/`) that previously called into `apps/journal/app/lib/sync/`. Replace direct `connections.server.ts` reads with `withFreshCredentials`. +- [ ] 5.5 Update the connections settings page (`/settings/connections`) and any UI surfaces reading `granted_scopes` to read from the `connected_services` row directly. +- [ ] 5.6 Delete the old `apps/journal/app/lib/sync/` directory once all imports are gone. Delete the old `SyncProvider` interface from `types.ts`. + +## 6. Verification + +- [ ] 6.1 Run `pnpm typecheck && pnpm lint && pnpm test` — all green. +- [ ] 6.2 Run `pnpm test:e2e` with the Wahoo flow specs — all green. +- [ ] 6.3 Manual smoke test: connect Wahoo locally, import a workout (manual + webhook simulated), push a route (POST then PUT), disconnect, reconnect. +- [ ] 6.4 Check that `connected_services` migration applies cleanly to a copy of staging data (or local seed reflecting prod). + +## 7. Documentation + follow-up + +- [ ] 7.1 Update `CONTEXT.md` if any term was sharpened during implementation. +- [ ] 7.2 File a follow-up issue/note to amend `openspec/changes/komoot-import/design.md` to use `connected_services` + `web-login` credential kind instead of the drafted `journal.integrations` table. +- [ ] 7.3 At archive time, apply the spec deltas in `specs/connected-services/`, `specs/wahoo-import/`, `specs/wahoo-route-push/` to `openspec/specs/`.