Removes the unused GET /api/activities list/detail endpoints, moves the
review-queue list and resolve/unlock/unassign actions under
/api/activities, renames resolve to assign end-to-end, and drops the
now-unused store.ReviewQueue helper.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SetActivityWorkoutNotFound is a per-user-scoped store method but never got
the adversarial cross-user coverage CLAUDE.md's testing conventions require
for such features. Extend the existing isolation test with a mismatched-user
call (must no-op) and a correct-user call (must exclude the activity from
that user's own ActivitiesMissingWorkout/count without touching the other
user's pending activity).
fillPendingWorkouts previously treated every get_workout_by_id
failure identically -- log it, leave workout_raw_json null, retry
next sync -- which is correct for a transient failure but means a
definitive 404 (the workout deleted on Garmin's side after being
linked to an activity) got silently retried forever, with the only
symptom being an unexplained, permanent "1 more workout pending"
nudge. Now checks errors.Is(err, garmin.ErrNotFound) and, only for
that case, calls SetActivityWorkoutNotFound instead of retrying.
A confirmed-404 workout must stop being retried forever, but
workout_raw_json should never be fabricated -- it stays null exactly
as it does for "not yet fetched." workout_not_found_at is the
separate marker ActivitiesMissingWorkout/CountActivitiesMissingWorkout
now check for, mirroring the existing pattern of the other _fetched_at
columns. UpsertActivity's ON CONFLICT clause already never touches
these columns, so the marker persists across every later re-sync.
roundTrip now wraps the returned error with the new ErrNotFound
sentinel whenever the wrapper's response set not_found (Task 1),
letting any Client method's caller distinguish a definitive 404 from
a transient failure via errors.Is, regardless of which garminconnect
method was called.
garminconnect's own GarminConnectNotFoundError already exists
specifically for this (its docstring: "so callers can now catch a
missing resource specifically, e.g. deleting an already-deleted
workout"), raised by connectapi() for any real HTTP 404. dispatch()
now surfaces that distinction as an extra not_found: true field
alongside the existing error string, so internal/garmin/client.go
(next commit) can tell a definitive 404 apart from a transient
failure.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks: mark a wrapper-level 404 as not_found, expose it as
garmin.ErrNotFound in the Go client, add a workout_not_found_at
column + store query updates, then have fillPendingWorkouts stop
retrying a confirmed-404 workout instead of retrying forever.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A workout deleted on Garmin's side after being linked to an activity
404s forever on get_workout_by_id, and today's fillPendingWorkouts
retries it every sync indefinitely with no user-visible feedback --
just a perpetual "1 more workout pending" nudge. Distinguishes a
definitive 404 (via garminconnect's own GarminConnectNotFoundError)
from a transient failure, and stops retrying the former.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
LoginGate's initial session check swallowed every failure into the
same "unauthenticated" branch, so a genuinely unreachable backend
looked identical to a normal logged-out state -- the one screen in
the app where "backend not here" showed no feedback at all, since
every banner-migrated component only renders after authentication.
api/client.ts's request() now throws a dedicated NetworkError (a
distinct type, not just a distinguishable message) for a fetch()
failure specifically, so LoginGate.tsx's catch can tell that apart
from a real 401 via instanceof and show a banner before falling
through to "unauthenticated" either way.
Verified with a real headless-browser run against the dev server
with no backend: the banner renders correctly and coexists with the
existing ?auth_error= banner without conflict.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Corrupting the true last character of a JWT's base64url-encoded
HMAC-SHA256 signature is unreliable: that character encodes only 4
real bits plus 2 unused padding bits, and Go's encoding/base64
ignores those padding bits by default -- about 1 in 4 replacement
characters decode to byte-identical signature bytes, so the
"tampered" cookie still verifies and the test spuriously passes.
Corrupting the second-to-last character instead is deterministic,
since HMAC-SHA256's fixed 32-byte digest length means that position
is always fully significant.
Confirmed via 20 repeated runs (previously ~1/3 failure rate).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Multiple independent requests that fail with the same message (e.g.
Activities.tsx's mount effect firing loadFirstPage/listWorkoutKinds/getProfile
separately) each call showError() independently, previously stacking
2-3 textually-identical banners for a single root cause (backend down,
brief network blip). show() now skips adding a banner if one with the
same severity+message is already visible, and skips scheduling a
redundant auto-dismiss timer for it -- the original banner's timer
keeps running untouched.
Field-validation messages ("please fill this in") stay local and
inline. Real failures (Garmin login rejected, MFA rejected, complete()
failing) now call showError. The complete()-failure Retry button's
visibility switches from the error text's presence to a new
completeFailed boolean, since the error text itself now lives only in
the (transient) banner.
Load/save/delete failures now call showError instead of setting local
error state. The profile-not-loaded-yet fallback gains a
profileLoadFailed boolean so it can still distinguish "loading" from
"failed to load" without the error text itself, which now lives only
in the (transient) banner.
The modal is now purely the progress bar/spinner. The instant a poll
reports the sync finished, it builds one banner message (the
success/error line plus any pending-activities/pending-workouts
nudge as extra lines) and closes itself -- no more manual Close click
to dismiss a result the user already saw. The Close button stays as a
manual escape hatch for a still-running sync.
request()'s fetch() call throwing (network down, connection refused,
offline) previously propagated the raw browser error text (e.g.
"TypeError: Failed to fetch"). Centralizing the friendly message here,
rather than in each call site, means every existing
catch((e) => showError(String(e))) pattern gets it for free.
A plain module-level singleton store (banner.ts) plus a single renderer
(BannerStack, mounted once above LoginGate in main.tsx) -- no React
Context, since this codebase has none today and a plain exported
function is callable from anywhere, including api/client.ts (a plain
module, not a component) in a later task. Nothing calls it yet; that's
each of this plan's remaining tasks.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
11 tasks: the shared banner store + renderer, a friendlier
network-unreachable message in client.ts, then migrating every
existing inline error site (GarminConnection, SyncModal, Profile,
Activities, Analysis, TrainingTypesCard, LoginGate, OnboardingWizard)
to it, finishing with dead-CSS cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces every page/component's ad-hoc inline error paragraph with a
shared, module-level banner store (red/orange/blue/green, stacking,
auto-dismiss + manual close) -- from docs/IDEAS.md's backlog item.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
It polled /api/sync/status every 1.5s indefinitely even after
in_progress became false and the final result was already shown,
only stopping on unmount (clicking Close). Skip scheduling the next
poll once a fetched status reports the sync is no longer running.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
TestFillPendingDetails_ReportsPhaseAwareProgressThroughActivitiesAndWorkoutsPhases
only ever called FillPendingDetails directly, never exercising
FullSync's own setProgress(PhaseDiscovering, ...) / deferred
setProgress(PhaseIdle, ...) wiring around backfillCore/
incrementalSyncCore -- what handleSyncRun actually runs in production
and what SyncModal's "Discovering activities..." spinner depends on.
Add a mock.Client.Delay field (slept, ctx-cancellable, at the start of
GetActivities) so a test can give the discovering phase real
wall-clock duration, then add
TestFullSync_ReportsPhaseTransitionsThroughDiscoveringToIdle, which
runs FullSync in a goroutine and polls Progress() to confirm it
observes PhaseDiscovering mid-flight and PhaseIdle after completion.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every run recorded today is SyncKindFull (the single-pass "Sync now"
action) -- "backfill"/"incremental" only ever appear in historical
rows predating that consolidation.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow this file's existing pattern (two real provisioned users,
identical-shaped rows, assert on empty-result/unaffected-by rather
than just two independently-created rows not colliding) for the two
methods touched by the details_fetched_at gating fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fillPendingDetails and fillPendingWorkouts are each independently
LIMIT-bounded over different candidate sets, so on a large first
backfill a structured-workout activity could fall inside the workouts
pass's window while still outside the details pass's window.
fillActivityWorkout would then align workout targets against zero laps
(details/laps never written yet) and unconditionally mark
workout_raw_json non-NULL, permanently losing that activity's target
pace/HR bands once its real laps arrived later -- silently, with no
error.
Add details_fetched_at IS NOT NULL to ActivitiesMissingWorkout's and
CountActivitiesMissingWorkout's WHERE clause: an activity is only
eligible for a workout fetch once its laps actually exist to align
against. This still covers the intended retry case (an activity whose
workout fetch previously failed always has details_fetched_at already
set) while excluding never-yet-processed activities.
Rename/rewrite the store test to assert the corrected exclusion
(count=1, not 2) as an explicit regression test, and fix the
TestSyncStatus_ReportsPhaseAwareProgressAndWorkoutsPending API fixture,
which exercised the same buggy shape.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GarminConnection.tsx no longer polls sync status itself or renders any
inline "syncing.../last sync..." text -- SyncModal (previous commit) is
now the only place sync progress and results are shown. Auth-status
polling is simplified to a fixed interval now that it no longer needs
to speed up while a sync is running.
Blocking overlay polling /api/sync/status, rendering by progress.Phase
(indeterminate spinner while discovering, a real progress bar for the
activities/workouts phases), then the final sync result (success count,
or the error message that was already fetched but never rendered
before) plus the existing pending-details/pending-workouts nudge. Never
auto-closes -- not yet wired into GarminConnection.tsx (next commit).
Replaces detail_fill_progress ({Done, Total}) with progress ({Phase,
Done, Total}), and adds workouts_pending (mirroring
activities_pending_details) from the new CountActivitiesMissingWorkout.
Frontend types updated to match -- GarminConnection.tsx is intentionally
left broken by this commit alone; it's fixed in the next commit that
adds SyncModal.
Progress becomes phase-aware ({Phase, Done, Total} instead of a flat
{Done, Total}), with FullSync reporting an indeterminate "discovering"
phase during backfill/incremental sync (there's no meaningful total
before those calls have already happened), then FillPendingDetails
reporting a real Done/Total for its activities pass, then its new,
independent workouts pass.
alignWorkoutTargets now takes a lap count instead of a []garmin.Lap
slice, since it never used lap content, only length -- this lets the
new workouts pass call it against laps read back from the DB rather
than needing the original Garmin lap data again.
Splitting the passes also fixes a latent bug: an activity whose
details were fetched successfully but whose workout fetch failed in
that same run previously had no way to ever retry the workout fetch,
since ActivitiesMissingDetails stops returning it once
details_fetched_at/splits_fetched_at are set. ActivitiesMissingWorkout
queries workout_id/workout_raw_json independently, so it keeps
surfacing that activity until its workout is actually fetched.
Mirrors ActivitiesMissingDetails/CountActivitiesMissingDetails, but
queries workout_id IS NOT NULL AND workout_raw_json IS NULL --
independent of whether details/splits were ever fetched, since
workout_id is known at initial upsert time. This also fixes a latent
gap: an activity whose details were fetched successfully but whose
workout fetch failed in the same run previously had no way to ever be
retried, since ActivitiesMissingDetails stops returning it the moment
details_fetched_at/splits_fetched_at are set.
Both were only reachable via the periodic background sync loop removed
in 4d2cbe4 -- nothing in production calls them anymore, only tests did.
FullSync already calls backfillCore/incrementalSyncCore directly.
Rewrite the affected tests to call the *Core functions (still exported
within the package) instead, dropping the now-redundant standalone
SyncRun-recording assertion covered by TestFullSync_RecordsOneCombinedSyncRun.
Six tasks: remove dead Backfill/IncrementalSync wrappers, add
ActivitiesMissingWorkout/CountActivitiesMissingWorkout, split
FillPendingDetails into phase-aware activities/workouts passes,
reshape GET /api/sync/status, add SyncModal, wire it into
GarminConnection.tsx.
Discovered while verifying the sync design with the user: these two
exported methods are dead in production (only FullSync's *Core calls
are actually used), kept alive only by tests, and their doc comments
still reference the background sync loop removed in 4d2cbe4.
Phase-aware sync progress (discovering/activities/workouts), a new
/api/sync/status shape, and a blocking SyncModal replacing the Profile
page's inline sync text -- implementing the "improve synchronization"
idea from docs/IDEAS.md.
daysAgoToISODate built the date at local midnight but formatted it with
toISOString(), which converts to UTC first -- in any timezone ahead of
UTC (e.g. Europe/Paris), local midnight is still the previous day in
UTC, so the picker displayed a date one day earlier than the one that
was actually selected/stored. Format from the Date's local
year/month/day components instead of going through UTC.
There's no reason the Garmin token-store root and the SQLite database
file need to live near each other -- they're independent env vars
(GARMIN_TOKENSTORE, GENIUSRUN_DB_PATH) and should stay independently
configurable, including in their defaults. Revert the "next to DBPath"
default introduced in 77f9588 back to a plain ".garmin" relative to
the working directory, so pointing GENIUSRUN_DB_PATH elsewhere never
silently drags the token-store default along with it.
