garmin_activity_id uniqueness becomes per-user (UNIQUE(user_id,
garmin_activity_id), added in Task 1) so two users' Garmin accounts can
never collide even in the unlikely event their activity ids coincide.
workout_kinds gains a real user_id column (Task 1); workout_type_paces has
none of its own and is scoped via a join to workout_kinds instead, since
it's always accessed through a specific kind.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Part of per-user profile isolation: profile rows are no longer a global
singleton, so every read/write requires the caller's userID. This also
requires scoping ListActivities, ListWorkoutKinds, UpsertActivity,
CreateWorkoutKind, GetSyncState, UpdateSyncState, and ResetAllSyncedData
to userID, plus updating all related tests in the store package.
Binds pre-multi-tenancy singleton rows to one named OIDC subject, given at
startup via an env var (wired in Task 11). No-ops once any user exists or
on a genuinely fresh install.
Lays the groundwork for per-user accounts: CreateUser/GetUserBySub/
ListUsers plus ProvisionUser, which seeds a brand-new user's profile,
default workout-kind taxonomy, and sync state in one transaction. Depends
on later tasks scoping GetProfile/ListWorkoutKinds/GetSyncState to compile
and pass -- expected or committing to a shared branch.
Every existing migration test opens a brand-new DB via store.Open, which
runs migrations 0001-0025 in one uninterrupted pass over an empty
database -- so migrations 0023/0024/0025's table-rebuild (create-new/copy/
drop-old/rename-into-place) never had any real pre-existing rows to carry
across, and no test proved that a genuinely in-use single-tenant install
(real workout_kinds, real synced activities, kind_assignments referencing
workout_kinds by FK) upgrades safely.
TestRebuildMigrationsPreserveForeignKeyReferences manually applies
migrations up to (but not including) the three rebuilds, inserts rows
simulating that pre-existing install, applies the rebuilds, then verifies
the kind_assignments row still resolves to the correct workout_kinds row
by name, and that FK enforcement is genuinely back on afterward (a bogus
workout_kind_id is rejected).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous implementation issued PRAGMA foreign_keys toggles inside
transactions (after tx.Begin()), but SQLite ignores pragmas once a
transaction is open with modernc.org/sqlite, making those calls no-ops.
This fix relocates the toggle to autocommit mode (db.Exec, not tx.Exec)
and limits it to only the three migrations that need it (0023/0024/0025),
which rebuild tables with incoming foreign keys (kind_assignments/
workout_type_paces reference workout_kinds; activities/sync_runs
reference sync_state).
The connection string now correctly restores ?_pragma=foreign_keys(1),
ensuring FK enforcement is ON by default for normal runtime operation
and during migrations that don't need the special handling.
Each affected migration now:
1. Disables FK in autocommit mode before starting its transaction
2. Runs the migration's CREATE/INSERT/DROP/RENAME sequence
3. Re-enables FK in autocommit mode after the transaction commits
All other migrations run normally with FK enforcement active throughout,
protecting against cascading deletions silently failing if a future
connection change causes FK enforcement to inadvertently remain off.
Added TestForeignKeyEnforcementPostMigration to verify FK is correctly
enforced after all migrations complete: valid FK references are accepted,
and invalid ones are rejected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Schema-only step toward per-user profile isolation: profile/workout_kinds/
sync_state are rebuilt to drop their singleton constraints, activities/
sync_runs gain a nullable user_id column. Store methods are scoped in
later tasks.
Four bugs slipped through per-task review since each task only saw its
own diff:
- Logout was a plain <a href> GET against a POST-only backend route, so
it 405'd and never cleared the session cookie or hit Keycloak's
end-session redirect. Now a <form method="post"> with a submit button
styled to match the old link (still a real full-page navigation, not
a fetch, so the Keycloak redirect chain still works).
- Login/logout used origin-relative paths, unreachable from the Vite
dev server (:5173) against the backend (:8080) with no proxy
configured. Both now build their URL from client.ts's now-exported
BASE_URL.
- handleSessionCallback's four failure paths redirected to
/?auth_error=failed with no logging, making a real OIDC failure
undiagnosable in production. Added log.Printf on each failure site.
- handleSessionLogout passed a bare "/" to EndSessionURL; Keycloak
requires post_logout_redirect_uri to be an absolute, registered URL.
Added SessionConfig.PublicBaseURL, wired from cfg.PublicBaseURL in
main.go, and used to build an absolute redirect.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Router() now wraps every route except /health, /session/login, and
/session/callback in a chi group requiring a valid session cookie
(auth.RequireSession). Adds internal/api/session.go with the four
session HTTP handlers (login/callback/logout/me) and SessionConfig.
NewServer takes an auth.Verifier and SessionConfig. Test infra
(doJSON, newTestServer) now mints/attaches a signed session cookie
automatically so the 36 pre-existing tests keep exercising the
already-logged-in path unchanged, plus 8 new tests cover the gating
and session endpoints themselves.
Implements HTTP middleware that enforces session authentication by validating
session cookies and making claims available via ClaimsFromContext. Rejects
requests without valid session cookies with 401 Unauthorized.
Tested via four test cases:
- Missing session cookie rejection
- Valid cookie acceptance with claims extraction
- Expired cookie rejection
- Tampered cookie rejection
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- GarminConnection now flushes Profile's pending debounced autosave before
connecting, avoiding a race where Connect fires with stale credentials.
- client.go reads res.Content before checking IsError so tool error
messages actually include mcp-garmin's response text.
- seedsample: update kind lookups to match current taxonomy names
("Easy Run" -> "Easy", "Interval" -> "Intervals").
- Add backend/start.sh and frontend/start.sh dev launch scripts, and
check in CLAUDE.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Remove duplicated Garmin fields from storage; decode display-only fields
(activity name/type, lap duration/HR, structured workout raw JSON) from
RawJSON at API-response time instead of storing redundant columns
- Add a fully configurable chart color system (pace/HR main-line colors, 4
effort-kind colors, tint/darken/brighten intensity knobs) under Profile >
Chart colors
- Rename training types and fix their display order (Easy, Long, 60'/30'
Threshold, Tempo, Intervals, MAS Test, Race) everywhere they're listed
- Add an Efficiency Factor progression metric; fix Progression chart axes to
use tight non-zero-based domains, m:ss/km pace formatting, and rounded
ticks instead of raw floating-point labels
- Expose the raw get_workout_by_id() payload in the raw-data viewer
alongside activity/lap/detail JSON; enlarge the modal and shrink array
indentation for readability
- Fix "last sync" reporting a meaningless activity count: record one
combined sync run per manual "Sync now" and count genuinely new
activities instead of re-listing whatever Garmin returned for the queried
window
- Let a Review Queue activity be manually cleared back to Unclassified, and
make "Reset all" available even while disconnected from Garmin
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Confirmed via Garmin Connect against a real activity ("Auriol - W3-3-Double
Barrel"): the workout's step count didn't match its 18 recorded laps
because the athlete kept running 6:46 past the prescribed 5-minute
cool-down, logged as an 18th lap the workout never defined. The strict
equality check meant this one extra lap discarded every other lap's real
target too, not just its own.
alignWorkoutTargets now tolerates exactly one extra recorded lap beyond the
step count: the steps that do exist still zip to their laps normally, and
only the trailing extra lap is left without a target. Any larger mismatch
still falls back to nil for every lap, since that can't be trusted at all.
Two new Profile settings -- "minimum representative pace" and "minimum
representative time" -- replace the previous hardcoded 20:00/km cutoff. A
stretch of consecutive samples slower than the configured pace is now
dropped from the chart (and its Y-axis scale) only if it lasts no longer
than the configured time; a longer stretch is kept as a real stop or walk
break rather than noise. Defaults to 12:00/km and 3 seconds.
Caught a boundary bug while verifying against real data: a run lasting
exactly the threshold duration survived filtering because the comparison
used strict "<" instead of "<=", contradicting "not lasting more than N
seconds" (which should include exactly N).
Fetching every needs_review activity's laps and per-second samples up front
gets expensive once there are many -- that work is now deferred to the
current page only. GET /api/review-queue/ takes limit/before and returns
{items, next_cursor, total}; sorting/cursor filtering still needs each
activity's cheap summary row, but only the requested page's items get their
laps/samples fetched.
Frontend loads 10 at a time and fetches the next page when the list's
bottom sentinel scrolls into view. Selecting a specific-kind filter (not
"All") loads the rest of the backlog up front, since filtering only the
items scrolled into view so far would hide matches further down.
A lap can span many minutes, so plotting one flat value per lap hid real
within-lap variation -- a single-lap hill repeat rendered as a dead-flat
line despite the pace/HR swinging throughout it. The actual pace/HR trace
now comes from activity_samples (per-second telemetry) when available,
looking up each sample's enclosing lap only for the target band/phase
color. Falls back to the previous lap-stepped rendering when an activity
has no samples.
SMARTRUN_BACKFILL_HORIZON_DAYS was a server-startup-only env var with no UI,
defaulting to 3 years -- so editing the unrelated "Rolling window" profile
field (for classification, not sync) had no effect on how far back Sync Now
reached. Backfill horizon is now Profile.BackfillHorizonDays, read fresh on
every Backfill call, with its own field on the Profile page.
Sync now now runs Backfill (resumes from the watermark, so a widened history
horizon is picked up automatically) then IncrementalSync then detail-fill, in
one click. "Full backfill" is gone -- in its place, "Reset all" (styled as a
destructive action, gated by a confirm dialog) wipes every synced activity
and its laps/kind assignments and rewinds the backfill watermark, so the next
sync performs a genuinely fresh pull instead of trying to patch up existing
rows with newer schema fields.
Activities recorded from a structured Garmin workout carry a workoutId;
when its steps line up 1:1 with the recorded laps, the per-step target
pace/HR zone is resolved (via a Karvonen lookup for named zones) and stored
on each lap. The Review Queue plots it against the actual per-lap pace/HR
so the user can eyeball whether a run matches its plan while sorting it.
Manual assignments are the user's definitive word and Race assignments come
from a hard Garmin fact (eventType.typeKey), not a retunable rule -- neither
is ever touched by reclassify again, and Race can no longer be set by hand
via the review queue. Adds an Activities page so this locked/unlocked status
is visible per workout, since Review Queue only ever showed unresolved items.
Race is the 8th fixed workout kind, seeded with a real (not placeholder)
rule since Garmin Connect's eventType.typeKey reports "race" for
manually-tagged race activities. Non-running activity types (padel,
cycling, strength training, ...) are now dropped at sync time instead of
being stored. The Review Queue's type filter is now clickable exclusive
pill buttons instead of a dropdown.
The 14 phase-related profile columns (warmup/cooldown per workout type,
plus an unused Interval pair) added more granularity than wanted. Migration
0006 drops all 14 and adds a single warmup_minutes/cooldown_minutes pair
applied uniformly to every fixed-duration workout type; Interval still
detects phases from lap data directly and ignores this setting. Restores
a simplified Phase detection section in the Profile UI with just the two
fields.
Review Queue: show pace alongside distance/duration/HR, sort by activity
date (most recent first) instead of classification timestamp, and add a
filter by workout kind with a special "Unsorted" option for runs where
the rule engine found zero candidates at all (distinct from ambiguous
multi-candidate runs).
Profile: remove the phase-detection warm-up/cool-down section from the
UI -- not used by anything yet (phase segmentation is future work) and
was adding noise. The underlying fields are untouched so no data is lost
and the settings screen still round-trips them on save.
Two Minor findings from the final whole-branch review: the smartrun-dev
skill still told developers to set GARMIN_EMAIL/GARMIN_PASSWORD (now
sourced from the profile row instead), and validateProfile's deliberate
choice not to require 0-100% HR zone coverage had no explanation.
Implements GET /api/profile and PUT /api/profile endpoints with validation
of HR zones (must be contiguous, non-overlapping, 0-100%). Also updates
profile migration to use correct HR zone defaults (0-20, 20-40, etc.)
that match validation requirements.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The migration 0004_workout_taxonomy.sql now seeds a fixed 'Tempo' kind.
The test was also creating a 'Tempo' kind, causing a UNIQUE constraint violation.
Renamed the test kind to 'Test Classification Tempo' to avoid collision.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Add migration 0004_workout_taxonomy.sql to reset workout_kinds table
with exactly 7 fixed named kinds (Easy Run, Long Run, Threshold 30',
Threshold 60', Tempo, Interval, MAS Test), each with a never-matching
placeholder rule
- Add test TestWorkoutTaxonomy_SeededWithSevenFixedTypes to verify all
7 kinds are seeded and active
- Update TestKindAssignment_AppendOnlyHistoryAndCurrentView to use a
unique kind name to avoid conflict with seeded kinds
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add Profile table to store Garmin credentials and engine configuration
parameters. Implements GetProfile() and UpdateProfile() store methods with
comprehensive test coverage. The profile singleton row is automatically
initialized on migration and persists all configuration state.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
