refactor: address all code review findings for roadmap XState refactor

Fix all CodeRabbit review comments and Biome warnings:
- Replace non-null assertions with explicit null checks in roadmap-store.ts (lines 548, 553, 559, 565)
- Replace non-null assertion with proper type guards in test (line 396)
- Use @shared/state-machines path alias instead of relative imports
- Prune stale feature actors in setRoadmap when roadmap changes
- Add lastActivityAt timestamp tracking to generation machine context
- Update deriveGenerationStatus to use persisted lastActivityAt from context
- Fix misleading test title "ignore MARK_DONE" → "self-transition"
- Add test verifying progress preservation on GENERATION_ERROR
- Remove progress reset in setError action to preserve progress on errors
- Add compile-time assertions to ensure state name arrays stay in sync with XState machines

All tests pass (77/77). TypeScript compilation successful. Zero Biome warnings in modified files.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

apps/frontend/src/renderer/stores/roadmap-store.ts

@@ -17,7 +17,7 @@ import {
   mapFeatureStateToStatus,
   type RoadmapGenerationEvent,
   type RoadmapFeatureEvent
-} from '../../shared/state-machines';
+} from '@shared/state-machines';
 
 // ---------------------------------------------------------------------------
 // Module-level XState actor singletons
@@ -176,9 +176,7 @@ function deriveGenerationStatus(actor: Actor<typeof roadmapGenerationMachine>):
     message: ctx.message ?? '',
     error: ctx.error,
     startedAt: ctx.startedAt ? new Date(ctx.startedAt) : undefined,
-    lastActivityAt: phase !== 'idle' && phase !== 'complete' && phase !== 'error'
-      ? new Date()
-      : undefined
+    lastActivityAt: ctx.lastActivityAt ? new Date(ctx.lastActivityAt) : undefined
   };
 }
 
@@ -191,8 +189,20 @@ export const useRoadmapStore = create<RoadmapState>((set) => ({
 
   // Actions
   setRoadmap: (roadmap) => {
-    featureActors.forEach((actor) => actor.stop());
-    featureActors.clear();
+    // Prune stale actors: stop and remove actors for features not in the new roadmap
+    if (roadmap) {
+      const newFeatureIds = new Set(roadmap.features.map((f) => f.id));
+      for (const [featureId, actor] of featureActors.entries()) {
+        if (!newFeatureIds.has(featureId)) {
+          actor.stop();
+          featureActors.delete(featureId);
+        }
+      }
+    } else {
+      // No roadmap → cleanup all actors
+      featureActors.forEach((actor) => actor.stop());
+      featureActors.clear();
+    }
     return set({ roadmap });
   },
 
@@ -545,24 +555,28 @@ async function reconcileLinkedFeatures(projectId: string, roadmap: Roadmap): Pro
   let hasChanges = false;
 
   for (const feature of featuresNeedingReconciliation) {
-    const task = taskMap.get(feature.linkedSpecId!);
+    // Safe: linkedSpecId is guaranteed to exist by the filter on line 531
+    const linkedSpecId = feature.linkedSpecId;
+    if (!linkedSpecId) continue;
+
+    const task = taskMap.get(linkedSpecId);
 
     if (!task) {
       // Task no longer exists → mark as done with deleted outcome
       if (feature.status !== 'done' || feature.taskOutcome !== 'deleted') {
-        store.markFeatureDoneBySpecId(feature.linkedSpecId!, 'deleted');
+        store.markFeatureDoneBySpecId(linkedSpecId, 'deleted');
         hasChanges = true;
       }
     } else if (task.status === 'done' || task.status === 'pr_created') {
       // Task is completed → mark feature as done
       if (feature.status !== 'done' || !feature.taskOutcome) {
-        store.markFeatureDoneBySpecId(feature.linkedSpecId!, 'completed');
+        store.markFeatureDoneBySpecId(linkedSpecId, 'completed');
         hasChanges = true;
       }
     } else if (task.metadata?.archivedAt) {
       // Task is archived → mark feature as done with archived outcome
       if (feature.status !== 'done' || feature.taskOutcome !== 'archived') {
-        store.markFeatureDoneBySpecId(feature.linkedSpecId!, 'archived');
+        store.markFeatureDoneBySpecId(linkedSpecId, 'archived');
         hasChanges = true;
       }
     }

apps/frontend/src/shared/state-machines/__tests__/roadmap-feature-machine.test.ts

@@ -283,7 +283,7 @@ describe('roadmapFeatureMachine', () => {
       expect(snapshot.value).toBe('in_progress');
     });
 
-    it('should ignore MARK_DONE when already in done', () => {
+    it('should remain in done on MARK_DONE self-transition', () => {
       const snapshot = runEvents([{ type: 'MARK_DONE' }, { type: 'MARK_DONE' }]);
       expect(snapshot.value).toBe('done');
     });

apps/frontend/src/shared/state-machines/__tests__/roadmap-generation-machine.test.ts

@@ -168,6 +168,17 @@ describe('roadmapGenerationMachine', () => {
       expect(snapshot.value).toBe('error');
       expect(snapshot.context.error).toBe('Generation failed');
     });
+
+    it('should preserve progress when transitioning to error', () => {
+      const snapshot = runEvents([
+        { type: 'START_GENERATION' },
+        { type: 'PROGRESS_UPDATE', progress: 60, message: 'Processing...' },
+        { type: 'GENERATION_ERROR', error: 'Something went wrong' },
+      ]);
+      expect(snapshot.value).toBe('error');
+      expect(snapshot.context.progress).toBe(60);
+      expect(snapshot.context.error).toBe('Something went wrong');
+    });
   });
 
   describe('stop flow: STOP from analyzing/discovering/generating → idle', () => {
@@ -393,7 +404,11 @@ describe('roadmapGenerationMachine', () => {
       actor.send({ type: 'START_GENERATION' });
 
       const secondStartedAt = actor.getSnapshot().context.startedAt;
-      expect(secondStartedAt).toBeGreaterThan(firstStartedAt!);
+      expect(firstStartedAt).toBeDefined();
+      expect(secondStartedAt).toBeDefined();
+      if (firstStartedAt && secondStartedAt) {
+        expect(secondStartedAt).toBeGreaterThan(firstStartedAt);
+      }
       actor.stop();
     });
   });

apps/frontend/src/shared/state-machines/roadmap-generation-machine.ts

@@ -6,6 +6,7 @@ export interface RoadmapGenerationContext {
   error?: string;
   startedAt?: number;
   completedAt?: number;
+  lastActivityAt?: number;
 }
 
 export type RoadmapGenerationEvent =
@@ -32,6 +33,7 @@ export const roadmapGenerationMachine = createMachine(
       error: undefined,
       startedAt: undefined,
       completedAt: undefined,
+      lastActivityAt: undefined,
     },
     states: {
       idle: {
@@ -83,21 +85,24 @@ export const roadmapGenerationMachine = createMachine(
         error: () => undefined,
         startedAt: () => Date.now(),
         completedAt: () => undefined,
+        lastActivityAt: () => Date.now(),
       }),
       updateProgress: assign({
         progress: ({ event }) =>
           event.type === 'PROGRESS_UPDATE' ? Math.min(100, Math.max(0, event.progress)) : 0,
         message: ({ event }) =>
           event.type === 'PROGRESS_UPDATE' ? event.message : undefined,
+        lastActivityAt: () => Date.now(),
       }),
       setCompleted: assign({
         progress: () => 100,
         completedAt: () => Date.now(),
+        lastActivityAt: () => Date.now(),
       }),
       setError: assign({
         error: ({ event }) =>
           event.type === 'GENERATION_ERROR' ? event.error : undefined,
-        progress: () => 0,
+        lastActivityAt: () => Date.now(),
       }),
       resetContext: assign({
         progress: () => 0,
@@ -105,6 +110,7 @@ export const roadmapGenerationMachine = createMachine(
         error: () => undefined,
         startedAt: () => undefined,
         completedAt: () => undefined,
+        lastActivityAt: () => undefined,
       }),
     },
   }

apps/frontend/src/shared/state-machines/roadmap-state-utils.ts

@@ -5,7 +5,10 @@
  * derived from the roadmap machine definitions. Used by roadmap-store
  * and roadmap hooks to avoid duplicate constants.
  */
+import type { StateValueFrom } from 'xstate';
 import type { RoadmapGenerationStatus, RoadmapFeatureStatus } from '../types/roadmap';
+import { roadmapGenerationMachine } from './roadmap-generation-machine';
+import { roadmapFeatureMachine } from './roadmap-feature-machine';
 
 /**
  * All XState generation state names.
@@ -19,6 +22,9 @@ export const GENERATION_STATE_NAMES = [
 
 export type GenerationStateName = typeof GENERATION_STATE_NAMES[number];
 
+// Compile-time assertion: ensure GENERATION_STATE_NAMES stays in sync with roadmapGenerationMachine
+const _genCheck: readonly StateValueFrom<typeof roadmapGenerationMachine>[] = GENERATION_STATE_NAMES;
+
 /**
  * All XState feature state names.
  *
@@ -31,6 +37,9 @@ export const FEATURE_STATE_NAMES = [
 
 export type FeatureStateName = typeof FEATURE_STATE_NAMES[number];
 
+// Compile-time assertion: ensure FEATURE_STATE_NAMES stays in sync with roadmapFeatureMachine
+const _featCheck: readonly StateValueFrom<typeof roadmapFeatureMachine>[] = FEATURE_STATE_NAMES;
+
 /**
  * Generation states where the machine has settled — the generation is
  * complete or has errored. Stale progress events should NOT overwrite