Enhance workout recovery and auto-save logic to prevent saving comple…

[wulfland]

…ted workouts

[Copilot]

Pull request overview

This PR tightens workout session recovery and auto-save behavior to avoid persisting/recovering completed workouts, aiming to prevent “resuming” a finished session.

Changes:

• Prevent auto-save from running when a workout is marked completed.
• Clear activeWorkout in localStorage earlier during workout completion flow.
• Update recoverActiveWorkout() to ignore (and delete) completed workouts found in localStorage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/hooks/useWorkoutSession.ts	Adds guards around auto-save for completed workouts and changes when auto-saved state is cleared during endWorkout.
src/db/service.ts	Ensures recovery ignores completed workouts and clears localStorage when they are encountered.

[Copilot]

In endWorkout, clearAutoSavedWorkout() is called before the DB write. If createWorkout/updateWorkout throws, the user loses the only recovery copy in localStorage (data loss). Also, clearing localStorage here doesn’t stop the existing auto-save interval, which can immediately re-write activeWorkout while the async DB write is in progress. Consider stopping the interval first (clear autoSaveTimerRef), performing the DB save in a try/catch, and only clearing activeWorkout after a successful save (or leaving it intact on failure).

[Copilot]

The “double-check” inside save() doesn’t actually validate current state because workout/isActive are captured by the effect closure; if isActive is set to false, there’s a window before the effect cleanup runs where the interval can still fire and this check will still see the old isActive === true. If the goal is to prevent saves after ending/canceling, clear the interval synchronously in endWorkout/cancelWorkout and/or read workout/isActive from refs updated on render.

[Copilot]

recoverActiveWorkout() assumes the parsed JSON is a non-null object. If localStorage contains 'null' (or another non-object JSON value), workout.completed will throw. Add a guard after JSON.parse to validate the shape (e.g., if (!workout || typeof workout !== 'object') { ... }) before reading properties, and consider clearing the bad activeWorkout entry.

Suggested change

	// Don't recover completed workouts - they should be cleared
	if (workout.completed) {
	localStorage.removeItem('activeWorkout');
	return null;
	}
	return workout;
	// If the parsed value is not a non-null object, treat as corrupted data
	if (!workout || typeof workout !== 'object') {
	localStorage.removeItem('activeWorkout');
	return null;
	}
	// Don't recover completed workouts - they should be cleared
	if ((workout as import('../types/models').Workout).completed) {
	localStorage.removeItem('activeWorkout');
	return null;
	}
	return workout as import('../types/models').Workout;

[Copilot]

This new recovery behavior (auto-clearing activeWorkout when completed is true) isn’t covered by tests. Since src/db/service.test.ts exists, add unit tests for recoverActiveWorkout() covering: missing key, malformed JSON/non-object JSON, completed workout (clears + returns null), and incomplete workout (returns revived Dates).

[wulfland]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@wulfland I've opened a new pull request, #77, to work on those changes. Once the pull request is ready, I'll request review from you.