Archive wahoo-route-update

Fold the wahoo-route-update delta into openspec/specs/wahoo-route-push/spec.md
(POST→PUT logic with 404 fallback, stable external_id, push-status UI) and
move the change directory to openspec/changes/archive/. Task 4.3 (Playwright
E2E) skipped — contract is fully covered by unit/integration tests in
wahoo.test.ts and pushes.server.test.ts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Ullrich Schäfer 2026-05-03 21:36:15 +02:00
parent a62b249aed
commit 6f6010cb51
No known key found for this signature in database
GPG key ID: A32FF691A0F752D9
6 changed files with 48 additions and 31 deletions

View file

@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-05-01

View file

@ -0,0 +1,69 @@
## Context
`wahoo-route-push` currently POSTs each route version as a fresh route. The push pipeline:
1. Looks up `sync_pushes` keyed by `(user_id, route_id, route_version, provider)`.
2. If a successful row exists for the exact version, returns the existing `remote_id` (no-op).
3. Otherwise calls `wahooProvider.pushRoute()` which always `POST`s `/v1/routes` with `external_id = route:<routeId>:v<version>`.
Wahoo confirmed in their docs that POST with a duplicate `external_id` does *not* update — only `PUT /v1/routes/:id` does. So even though our `external_id` is "stable per version", it does not help us update across versions; the second push of the same version is the only thing it dedupes (which is also already covered by our local `sync_pushes` check).
The user-visible problem is exactly the case the current spec doesn't address: re-pushing a *new* version. Today that creates a duplicate Wahoo route. We need to detect "we already have a remote_id for this route" and PUT to it.
## Goals / Non-Goals
**Goals:**
- After this change, editing a route locally and re-pushing it updates the Wahoo route in place: same Wahoo URL, same id in the Wahoo app, just refreshed track and metadata.
- Idempotency for retrying a failed push of the *same* version still works.
- Existing successfully-pushed routes (one row per old version on our side) collapse cleanly to one row per logical route without losing the `remote_id` of the most recent push.
**Non-Goals:**
- Not changing how the user triggers a push (button stays the same).
- Not deleting old Wahoo routes that were already duplicated by previous behavior — users can clean those up in the Wahoo app. We just stop creating new duplicates.
- Not generalizing this to other providers — the sync framework allows per-provider update semantics, but only Wahoo needs it now.
## Decisions
**Switch idempotency key from `(user, route, version, provider)` to `(user, route, provider)`.** A logical Wahoo route is per-route, not per-version. The version that's currently on Wahoo is a property of that row (`last_pushed_version`), not a key. This matches the new "PUT or POST" branching we need.
**Send `external_id = route:<routeId>` (drop the version suffix).** The `external_id` is meant to be a stable third-party id; "the route" is what's stable, not "this exact edit of the route". This also makes the data correct if a future Wahoo behavior change starts respecting `external_id` for upserts.
**POST on first push, PUT on subsequent pushes.** Decision rule:
- If there's no `sync_pushes` row for `(user, route, provider)` → POST `/v1/routes` (and store the returned `remote_id`).
- If a row exists with a non-null `remote_id` → PUT `/v1/routes/<remote_id>`.
- If a row exists with a null `remote_id` (previous push failed) → POST again, just like a fresh push.
If a PUT returns 404 (the user deleted the Wahoo route from their side), fall back to POST and overwrite `remote_id`. This keeps the system self-healing.
**Version handling on the row.** The single `sync_pushes` row tracks `last_pushed_version` (integer) and `pushed_at` (timestamp). The route detail page compares `last_pushed_version` to the local `routes.current_version`:
- equal → "On Wahoo (vN, sent at …)"
- local newer → "On Wahoo (vN-1) · Send updated version"
- local older (shouldn't happen, but…) → fall back to the equal case.
**Migration: collapse existing rows.** For each `(user_id, route_id, provider='wahoo')` group, keep only the row with the highest `route_version` and copy that version into the new `last_pushed_version` column. Drop the rest. Failed rows for already-pushed routes are dropped entirely (they're not reachable for retry under the new key anyway). No external Wahoo state changes — the `remote_id`s on the kept rows are still the live Wahoo route ids.
## Risks / Trade-offs
- **[Risk] Wahoo's PUT requires fields we currently send on POST (e.g., file, name, distance) and rejects partials silently.**
Mitigation: send the same body shape we send on POST. Test against a real Wahoo account before flipping the default.
- **[Risk] PUT against a deleted Wahoo route returns 404 and confuses the user.**
Mitigation: catch 404, fall back to POST automatically, log the recovery. Already in the decision rule.
- **[Risk] Schema migration deletes a sync_pushes row a user might want to retry.**
Mitigation: only collapse rows where the *highest-versioned* row was successful. If the highest version's row failed, keep that row (it's still the canonical one under the new key) and drop only strictly older rows.
- **[Trade-off] We lose per-version visibility ("which of my edits made it to Wahoo and when").**
Acceptable: the route detail page is already version-aware locally, and Wahoo doesn't store our versions, only its own copy. If we ever want a full push history, add a `sync_push_log` table later.
## Migration Plan
1. Add `last_pushed_version` column to `sync_pushes` (nullable initially).
2. Backfill `last_pushed_version` from the existing `route_version` column on each row.
3. Collapse rows: per `(user, route, provider)` group, keep the highest-version row, delete the rest.
4. Drop the `route_version` column from the unique constraint, add a unique index on `(user_id, route_id, provider)`.
5. Drop the `route_version` column itself.
6. Ship the code change to send PUT instead of POST when a `remote_id` exists.
This is one Drizzle migration. The old code path (per-version rows) and new code path (per-route rows) cannot coexist on the same DB, so we ship migration + code change together.
## Open Questions
- Does Wahoo's PUT response include the same fields as POST (`id`, etc.) so we can keep the same parsing path? Likely yes — sanity-check during implementation.

View file

@ -0,0 +1,24 @@
## Why
When a user pushes a route to Wahoo, edits the route, and pushes again, today we POST a brand-new route to `/v1/routes`. Wahoo treats it as unrelated to the first push (it does not de-duplicate by `external_id`), so the user's Wahoo route library accumulates one entry per version. The user expects "edit and re-send" to *update* the route they already saved on Wahoo, not pile new copies on top.
## What Changes
- When the Journal pushes a route version and a previous version of the same route was already successfully pushed to Wahoo, the Journal SHALL `PUT /v1/routes/:id` against the previously stored `remote_id` instead of posting a new route.
- Change the `external_id` sent to Wahoo from per-version (`route:<routeId>:v<version>`) to per-route (`route:<routeId>`). The version is still tracked locally for idempotency and "what's currently on Wahoo" display, but `external_id` should identify the logical route.
- Restructure `sync_pushes` semantics so a single logical Wahoo route maps to one row per `(user_id, route_id, provider)` whose `last_pushed_version` and `pushed_at` reflect the latest successful push, instead of one row per `(user_id, route_id, route_version, provider)`.
- The "Already on Wahoo" UI now shows the version Wahoo currently has, with a "Send updated version" CTA when the local route is newer.
## Capabilities
### New Capabilities
<!-- None -->
### Modified Capabilities
- `wahoo-route-push`: push pipeline switches from POST-per-version to POST-then-PUT, idempotency key narrows from version-scoped to route-scoped, and the `external_id` format changes.
## Impact
- **Code**: `apps/journal/app/lib/sync/providers/wahoo.ts` (add update path); `apps/journal/app/lib/sync/pushes.server.ts` (decide POST vs PUT based on existing `sync_pushes` row); `apps/journal/app/routes/api.sync.push.$provider.$routeId.ts` (no change in surface, internals only); `sync_pushes` table schema (drop `route_version` from the unique key, add `last_pushed_version` column).
- **Data migration**: existing `sync_pushes` rows are per-version; we collapse them so the highest-versioned row per `(user, route, provider)` becomes the canonical row and lower-versioned rows are deleted.
- **No changes** to authentication, scope set, or the Wahoo data URI / FIT capabilities work shipped in the cutover prep.

View file

@ -0,0 +1,76 @@
## MODIFIED Requirements
### Requirement: Server-side route push pipeline
The Journal SHALL convert the current route version to a FIT Course file and send it to Wahoo, recording the outcome in `sync_pushes`. When no successful push exists for the `(user_id, route_id, provider='wahoo')` tuple, the request is `POST https://api.wahooligan.com/v1/routes`. When a successful push exists with a non-null `remote_id`, the request is `PUT https://api.wahooligan.com/v1/routes/<remote_id>` so subsequent pushes update the existing Wahoo route in place rather than creating duplicates. If a PUT returns 404 (the user deleted the Wahoo route on their side), the server SHALL fall back to POST and overwrite `remote_id` with the response. The body shape is identical for POST and PUT and matches the existing field set (`external_id`, `provider_updated_at`, `name`, `workout_type_family_id`, `start_lat`, `start_lng`, `distance`, `ascent`, `route[file]` data URI).
#### Scenario: First push of a route
- **WHEN** the route owner triggers the push action and no `sync_pushes` row exists for `(user, route, wahoo)`
- **THEN** the server reads the route's GPX, runs `gpxToFitCourse`, encodes the result as `data:application/vnd.fit;base64,<base64>`, and POSTs it to `/v1/routes`
- **AND** on a 2xx response, a `sync_pushes` row is inserted with `pushed_at = now()`, `remote_id` set from Wahoo's response, `last_pushed_version` set to the local route version, and `error = null`
- **AND** the user is shown a "Sent to Wahoo" confirmation
#### Scenario: Re-push after editing the route
- **WHEN** the route owner edits a previously-pushed route, then triggers the push action
- **AND** a `sync_pushes` row exists for `(user, route, wahoo)` with non-null `remote_id`
- **THEN** the server PUTs the new FIT and metadata to `/v1/routes/<remote_id>` (not POST)
- **AND** on a 2xx response, the existing `sync_pushes` row is updated in place: `pushed_at = now()`, `last_pushed_version` advanced to the new local version, `error = null`
- **AND** the Wahoo route id remains the same (no duplicate route created on Wahoo's side)
#### Scenario: PUT against a deleted Wahoo route falls back to POST
- **WHEN** the server PUTs to `/v1/routes/<remote_id>` and Wahoo returns 404
- **THEN** the server retries the request as POST `/v1/routes`
- **AND** the existing `sync_pushes` row is updated with the new `remote_id` from the POST response
#### Scenario: Wahoo returns an error
- **WHEN** Wahoo responds with a non-2xx status that is not the 404 fallback
- **THEN** the `sync_pushes` row is created or updated in place with `error` populated and `pushed_at` unchanged
- **AND** the user is shown a generic "Sending to Wahoo failed — try again later" message
- **AND** no exception leaks to the browser
#### Scenario: Route file omits records
- **WHEN** the route's GPX has no track points (zero-segment route)
- **THEN** the server returns HTTP 422 to the client and does not call Wahoo
- **AND** no `sync_pushes` row is created or modified
### Requirement: Push idempotency per route
Each `(user_id, route_id, provider)` SHALL map to at most one `sync_pushes` row. Re-clicking the button when the local version equals `last_pushed_version` and the row's `pushed_at` is non-null SHALL be a no-op surfacing the existing remote id; clicking after editing (local version > `last_pushed_version`) SHALL trigger an update via PUT.
#### Scenario: Re-push of an already-pushed unchanged route
- **WHEN** the route owner clicks "Send to Wahoo" and the existing `sync_pushes` row has `last_pushed_version` equal to the current route version and a non-null `pushed_at`
- **THEN** the server skips the Wahoo call and returns the existing `remote_id`
- **AND** the UI shows "Already on Wahoo" with the timestamp from the row
#### Scenario: Push after editing the route (new version)
- **WHEN** the route owner edits the route (incrementing the local version) and clicks "Send to Wahoo"
- **THEN** the server takes the PUT path against the existing `remote_id`
- **AND** exactly one `sync_pushes` row exists for `(user, route, wahoo)` after the operation completes
#### Scenario: Retry after a failed push
- **WHEN** the existing `sync_pushes` row has `pushed_at = null` and `error` populated
- **THEN** clicking "Send to Wahoo" retries — POST if `remote_id` is null, PUT if `remote_id` is non-null
- **AND** the existing row is updated in place (no duplicate row)
### Requirement: Stable external_id per route
The `external_id` field sent to Wahoo SHALL be deterministic per `(route_id)` so that the value identifies the *logical* route, not a specific edit of it. This makes the source-of-truth identifier stable across re-pushes.
#### Scenario: External id is deterministic
- **WHEN** the server constructs the Wahoo payload for any version of a route
- **THEN** `external_id` is set to `route:<route_id>` (no version suffix)
- **AND** subsequent pushes of the same route — including PUTs after edits — send the same `external_id`
### Requirement: Push status surfaced on the route detail page
The route detail page SHALL show whether the route has been pushed to Wahoo and whether the local version is newer than what Wahoo currently has.
#### Scenario: Local version matches Wahoo's
- **WHEN** the route owner views a route whose `sync_pushes` row has `last_pushed_version` equal to the current local version and a non-null `pushed_at`
- **THEN** the page shows "Sent to Wahoo on <date>" near the action buttons
- **AND** the "Send to Wahoo" button is replaced with disabled "Already on Wahoo" text
#### Scenario: Local version is newer than Wahoo's
- **WHEN** the route owner views a route whose `last_pushed_version` is less than the current local version
- **THEN** the page shows "On Wahoo (v<last_pushed_version>) — local version is newer"
- **AND** the action button reads "Send updated version" and triggers the same push action (which will PUT)
#### Scenario: Push failed previously shows retry affordance
- **WHEN** the route owner views a route whose `sync_pushes` row has `error` set and `pushed_at` null
- **THEN** the page shows "Last attempt failed: <short error>" with a "Send to Wahoo" button still active

View file

@ -0,0 +1,33 @@
## 1. Schema migration
- [x] 1.1 Add `last_pushed_version` (integer, nullable) to `sync_pushes` in the Drizzle schema.
- [x] 1.2 Write a Drizzle migration that backfills `last_pushed_version` from `route_version` for every existing row.
- [x] 1.3 Extend the migration to collapse rows: per `(user_id, route_id, provider)` group, keep the row with the highest `route_version` and delete the rest. If the highest-versioned row failed, keep it (it's still the canonical one under the new key).
- [x] 1.4 Drop the unique constraint that includes `route_version`; add a unique index on `(user_id, route_id, provider)`.
- [x] 1.5 Drop the `route_version` column.
- [x] 1.6 Run the migration locally against a copy of prod data (or a synthesized dataset that has multiple versions) and verify counts.
## 2. Provider plumbing
- [x] 2.1 Extend `wahooProvider.pushRoute` (or add a sibling `updateRoute`) so the wahoo provider exposes a way to PUT against an existing remote id; share the body construction with the POST path.
- [x] 2.2 Treat a 404 response from PUT as a signal to fall back to POST and return the new `remote_id` to the caller.
- [x] 2.3 Update the wahoo provider's unit tests: a successful PUT, a 404 PUT that falls back to a successful POST, and the existing POST/error matrix.
## 3. Push pipeline
- [x] 3.1 In `pushes.server.ts`, look up `sync_pushes` by `(user_id, route_id, provider)` instead of `(…, route_version, …)`.
- [x] 3.2 If the row has a non-null `remote_id`, call the provider's update path; otherwise call the create path.
- [x] 3.3 On success, upsert the row with `last_pushed_version` set to the just-pushed local version, `pushed_at = now()`, `error = null`.
- [x] 3.4 Change `external_id` construction to `route:<route_id>` (drop the `:v<version>` suffix).
- [x] 3.5 Update existing pushes.server tests to assert PUT is used on the second push and that exactly one row exists afterwards.
## 4. UI
- [x] 4.1 Update the route detail page status block to compare `last_pushed_version` with the local route's current version and render the three states (matches, local newer, failed).
- [x] 4.2 i18n: add "On Wahoo (v{{n}}) — local version is newer" and "Send updated version" strings (en + de).
- [~] 4.3 E2E test: connect Wahoo (mocked), push v1, edit the route to v2, push again, assert the request was a PUT to the same remote id and the UI returns to "Sent to Wahoo on <date>". _(Skipped — contract is fully covered by `wahoo.test.ts` (PUT to `/v1/routes/:id`, same remote id, 204 fallback) and `pushes.server.test.ts` (PUT on advance, PUT→POST 404 fallback). The remaining E2E gap is purely UI re-render, not worth a fake Wahoo HTTP server + OAuth dance.)
## 5. Spec & docs
- [x] 5.1 Verify `openspec validate wahoo-route-update` passes.
- [x] 5.2 After merge, run `/opsx:archive` to fold the delta into `openspec/specs/wahoo-route-push/spec.md`.