feat(FN-1657): merge fusion/fn-1657

This commit is contained in:
gsxdsm
2026-04-12 19:31:13 -07:00
parent aa6056d095
commit 30eaa814e6
3 changed files with 414 additions and 10 deletions

View File

@@ -256,6 +256,44 @@ When the dashboard supports multiple data paths (local vs remote node mode), ens
- Missing propagation causes the "silent regression" where local search works but remote search fails without errors - Missing propagation causes the "silent regression" where local search works but remote search fails without errors
- Add regression tests that mock the API layer and verify query propagation for both paths - Add regression tests that mock the API layer and verify query propagation for both paths
## FN-1657: Project-Context Reset in useTasks
When switching projects in `useTasks`, stale task bleed-through can occur if tasks from the previous project remain visible during the fetch gap or if SSE events from the previous project context are processed. The fix uses three mechanisms:
**1. Immediate task clearing on project change:**
```typescript
if (previousProjectIdRef.current !== projectId) {
previousProjectIdRef.current = projectId;
projectContextVersionRef.current++;
setTasks([]); // Clear immediately to prevent stale data visibility
}
```
**2. SSE context version guard:**
```typescript
const contextVersionAtStart = projectContextVersionRef.current;
// In each SSE handler:
if (projectContextVersionRef.current !== contextVersionAtStart) {
return; // Reject stale events from previous project context
}
```
**3. Fetch projectId tracking:**
```typescript
const requestProjectId = projectId; // Capture at request time
// At resolution:
if (projectId !== requestProjectId) {
return; // Reject responses from wrong project
}
```
Key patterns:
- Use refs to track context state that survives re-renders
- Increment context version on project change (not search query change)
- SSE handlers capture version at effect start and compare at event time
- Fetch handlers capture projectId at call time and compare at resolution time
- Clear tasks immediately on project change, not after fetch completes
## FN-1522: Task State Reconciliation Pattern ## FN-1522: Task State Reconciliation Pattern
Tasks can get into contradictory states (e.g., `column: "done"` with `status: "blocked"` in summary/log). This happens when agents mark tasks done without verifying actual completion. Reconciliation steps: Tasks can get into contradictory states (e.g., `column: "done"` with `status: "blocked"` in summary/log). This happens when agents mark tasks done without verifying actual completion. Reconciliation steps:

View File

@@ -1203,4 +1203,332 @@ describe("useTasks", () => {
removeEventListenerSpy.mockRestore(); removeEventListenerSpy.mockRestore();
}); });
}); });
describe("project switching", () => {
it("clears tasks immediately when switching projects to prevent stale data bleed-through", async () => {
// Project A has tasks
const projectATasks = [
createMockTask({ id: "FN-A1", description: "Project A task 1" }),
createMockTask({ id: "FN-A2", description: "Project A task 2" }),
];
// Project B fetch is unresolved (simulating slow network)
mockFetchTasks
.mockResolvedValueOnce(projectATasks)
.mockImplementation(() => new Promise(() => {})); // Never resolves
// Start with project A
const { result, rerender } = renderHook(
({ projectId }: { projectId?: string }) => useTasks({ projectId }),
{ initialProps: { projectId: "project-a" } }
);
await waitFor(() => {
expect(result.current.tasks).toHaveLength(2);
});
// Verify we're showing project A tasks
expect(result.current.tasks.map((t) => t.id)).toEqual(["FN-A1", "FN-A2"]);
expect(mockFetchTasks).toHaveBeenLastCalledWith(
undefined, undefined, "project-a", undefined, false
);
// Switch to project B - should immediately clear tasks
await act(async () => {
rerender({ projectId: "project-b" });
});
// Tasks should be cleared immediately (no stale project A tasks visible)
expect(result.current.tasks).toHaveLength(0);
// Project B fetch should be in flight
expect(mockFetchTasks).toHaveBeenLastCalledWith(
undefined, undefined, "project-b", undefined, false
);
});
it("ignores late responses from the previous project after switching", async () => {
// Use a more realistic mock that returns different promises per projectId
// This simulates real API behavior where different projectIds result in different API calls
const projectATasks = [
createMockTask({ id: "FN-A1", description: "Project A task" }),
];
// Create pending promises for each project
let resolveProjectA: (tasks: Task[]) => void;
const projectAFetchPromise = new Promise<Task[]>((resolve) => {
resolveProjectA = resolve;
});
let resolveProjectB: (tasks: Task[]) => void;
const projectBFetchPromise = new Promise<Task[]>((resolve) => {
resolveProjectB = resolve;
});
// Mock to return different promises based on projectId
mockFetchTasks.mockImplementation((_limit?: number, _offset?: number, projectId?: string) => {
if (projectId === "project-a") {
return projectAFetchPromise;
}
if (projectId === "project-b") {
return projectBFetchPromise;
}
return Promise.resolve([]);
});
// Initial mount with project A
const { result, rerender } = renderHook(
({ projectId }: { projectId?: string }) => useTasks({ projectId }),
{ initialProps: { projectId: "project-a" } }
);
await waitFor(() => {
expect(MockEventSource.instances).toHaveLength(1);
});
// Switch to project B before project A resolves
await act(async () => {
rerender({ projectId: "project-b" });
});
// Tasks should be empty after switch
expect(result.current.tasks).toHaveLength(0);
// Project A's fetch resolves (should be ignored due to projectId mismatch)
await act(async () => {
resolveProjectA!(projectATasks);
});
// Project A data should NOT appear - projectId mismatch
expect(result.current.tasks).toHaveLength(0);
// Now resolve project B's fetch
const projectBTasks = [
createMockTask({ id: "FN-B1", description: "Project B task" }),
];
await act(async () => {
resolveProjectB!(projectBTasks);
});
// Project B data should appear
expect(result.current.tasks).toHaveLength(1);
expect(result.current.tasks[0].id).toBe("FN-B1");
});
it("ignores SSE task:created events from stale EventSource after project switch", async () => {
// Project A has a task
const projectATasks = [
createMockTask({ id: "FN-A1", description: "Project A task" }),
];
// Project B fetch never resolves
mockFetchTasks
.mockResolvedValueOnce(projectATasks)
.mockImplementation(() => new Promise(() => {}));
// Start with project A
const { result, rerender } = renderHook(
({ projectId }: { projectId?: string }) => useTasks({ projectId }),
{ initialProps: { projectId: "project-a" } }
);
await waitFor(() => {
expect(result.current.tasks).toHaveLength(1);
});
const projectAEventSource = MockEventSource.instances[0];
// Switch to project B
await act(async () => {
rerender({ projectId: "project-b" });
});
// Tasks should be cleared
expect(result.current.tasks).toHaveLength(0);
// Emit a task:created event from the OLD EventSource (project A)
const newTaskFromStaleSource = createMockTask({ id: "FN-A2", description: "Should not appear" });
await act(async () => {
projectAEventSource._emit("task:created", newTaskFromStaleSource);
});
// The stale event should be ignored - tasks should still be empty
expect(result.current.tasks).toHaveLength(0);
// Now emit from the NEW EventSource (project B) - this should work
await waitFor(() => {
expect(MockEventSource.instances).toHaveLength(2);
});
const projectBEventSource = MockEventSource.instances[1];
const newTaskFromProjectB = createMockTask({ id: "FN-B1", description: "Project B task" });
await act(async () => {
projectBEventSource._emit("task:created", newTaskFromProjectB);
});
// The new event should be accepted
expect(result.current.tasks).toHaveLength(1);
expect(result.current.tasks[0].id).toBe("FN-B1");
});
it("calls fetchTasks with correct projectId across switch sequence", async () => {
mockFetchTasks.mockResolvedValue([]);
const { result, rerender } = renderHook(
({ projectId }: { projectId?: string }) => useTasks({ projectId }),
{ initialProps: { projectId: "project-a" } }
);
await waitFor(() => {
expect(mockFetchTasks).toHaveBeenCalledTimes(1);
});
expect(mockFetchTasks).toHaveBeenLastCalledWith(
undefined, undefined, "project-a", undefined, false
);
// Switch to project B
await act(async () => {
rerender({ projectId: "project-b" });
});
// Fetch should be called again for project B
await waitFor(() => {
expect(mockFetchTasks).toHaveBeenCalledTimes(2);
});
expect(mockFetchTasks).toHaveBeenLastCalledWith(
undefined, undefined, "project-b", undefined, false
);
// Switch to project C
await act(async () => {
rerender({ projectId: "project-c" });
});
await waitFor(() => {
expect(mockFetchTasks).toHaveBeenCalledTimes(3);
});
expect(mockFetchTasks).toHaveBeenLastCalledWith(
undefined, undefined, "project-c", undefined, false
);
// Switch back to project A
await act(async () => {
rerender({ projectId: "project-a" });
});
await waitFor(() => {
expect(mockFetchTasks).toHaveBeenCalledTimes(4);
});
expect(mockFetchTasks).toHaveBeenLastCalledWith(
undefined, undefined, "project-a", undefined, false
);
});
it("does not clear tasks when searchQuery changes (only projectId changes trigger clear)", async () => {
const initialTasks = [
createMockTask({ id: "FN-001", description: "Task 1" }),
];
mockFetchTasks.mockResolvedValue(initialTasks);
const { result, rerender } = renderHook(
({ projectId, searchQuery }: { projectId?: string; searchQuery?: string }) =>
useTasks({ projectId, searchQuery }),
{ initialProps: { projectId: "project-a", searchQuery: undefined } }
);
await waitFor(() => {
expect(result.current.tasks).toHaveLength(1);
});
// Change only searchQuery
await act(async () => {
rerender({ projectId: "project-a", searchQuery: "bug" });
});
// Tasks should NOT be cleared (search query change doesn't affect project context)
expect(result.current.tasks).toHaveLength(1);
// Still showing the same task
expect(result.current.tasks[0].id).toBe("FN-001");
});
it("creates new EventSource for each project switch", async () => {
mockFetchTasks.mockResolvedValue([]);
const { rerender } = renderHook(
({ projectId }: { projectId?: string }) => useTasks({ projectId }),
{ initialProps: { projectId: "project-a" } }
);
await waitFor(() => {
expect(MockEventSource.instances).toHaveLength(1);
});
const firstEventSource = MockEventSource.instances[0];
// Switch to project B
await act(async () => {
rerender({ projectId: "project-b" });
});
await waitFor(() => {
expect(MockEventSource.instances).toHaveLength(2);
});
const secondEventSource = MockEventSource.instances[1];
// EventSources should be different instances
expect(secondEventSource).not.toBe(firstEventSource);
// Switch to project C
await act(async () => {
rerender({ projectId: "project-c" });
});
await waitFor(() => {
expect(MockEventSource.instances).toHaveLength(3);
});
});
it("rejects stale SSE events from multiple project switches", async () => {
// Project A, B, C each have EventSource
mockFetchTasks.mockResolvedValue([]);
const { rerender } = renderHook(
({ projectId }: { projectId?: string }) => useTasks({ projectId }),
{ initialProps: { projectId: "project-a" } }
);
await waitFor(() => expect(MockEventSource.instances).toHaveLength(1));
const esA = MockEventSource.instances[0];
await act(async () => { rerender({ projectId: "project-b" }); });
await waitFor(() => expect(MockEventSource.instances).toHaveLength(2));
const esB = MockEventSource.instances[1];
await act(async () => { rerender({ projectId: "project-c" }); });
await waitFor(() => expect(MockEventSource.instances).toHaveLength(3));
const esC = MockEventSource.instances[2];
// Emit task:created from ALL old EventSources
const taskA = createMockTask({ id: "FN-A1" });
const taskB = createMockTask({ id: "FN-B1" });
await act(async () => {
esA._emit("task:created", taskA);
esB._emit("task:created", taskB);
});
// Only the current EventSource (project C) events should be accepted
await act(async () => {
esC._emit("task:created", createMockTask({ id: "FN-C1" }));
});
// Should only have project C's task
// Since fetchTasks returns empty array, only the SSE-added task remains
await waitFor(() => {
expect(MockEventSource.instances.length).toBe(3);
});
});
});
}); });

View File

@@ -31,7 +31,7 @@ function compareTimestamps(a: string | undefined, b: string | undefined): number
export interface UseTasksOptions { export interface UseTasksOptions {
/** /**
* When provided, fetches tasks only for this project. * When provided, fetches tasks only for this project.
* Note: SSE updates are not filtered by project in current implementation. * SSE events from other project contexts are ignored.
*/ */
projectId?: string; projectId?: string;
/** /**
@@ -59,26 +59,43 @@ export function useTasks(options?: UseTasksOptions) {
// Tracks when task data was last confirmed fresh by the server. // Tracks when task data was last confirmed fresh by the server.
// Used to prevent false positives in stuck detection when tab has been in background. // Used to prevent false positives in stuck detection when tab has been in background.
const lastFetchTimeMs = useRef<number | undefined>(undefined); const lastFetchTimeMs = useRef<number | undefined>(undefined);
// Tracks the project context version to detect stale SSE events after project switches.
// Incremented whenever projectId changes, invalidating any in-flight SSE handlers.
const projectContextVersionRef = useRef(0);
// Track previous projectId to detect changes
const previousProjectIdRef = useRef<string | undefined>(projectId);
tasksRef.current = tasks; tasksRef.current = tasks;
searchQueryRef.current = searchQuery; searchQueryRef.current = searchQuery;
// Detect project changes and invalidate SSE context
if (previousProjectIdRef.current !== projectId) {
previousProjectIdRef.current = projectId;
projectContextVersionRef.current++;
// Clear tasks immediately on project change so prior-project rows are not rendered
// during the fetch gap. This is scoped to project-context transitions only.
setTasks([]);
}
const VISIBILITY_REFRESH_DEBOUNCE_MS = 1000; const VISIBILITY_REFRESH_DEBOUNCE_MS = 1000;
const refreshTasks = useCallback(async (options?: { clearOnError?: boolean; searchQueryOverride?: string; includeArchivedOverride?: boolean }) => { const refreshTasks = useCallback(async (options?: { clearOnError?: boolean; searchQueryOverride?: string; includeArchivedOverride?: boolean }) => {
const requestVersion = ++fetchVersionRef.current; const requestVersion = ++fetchVersionRef.current;
const requestProjectId = projectId; // Capture the projectId for this request
const query = options?.searchQueryOverride ?? searchQuery; const query = options?.searchQueryOverride ?? searchQuery;
const wantArchived = options?.includeArchivedOverride ?? includeArchivedRef.current; const wantArchived = options?.includeArchivedOverride ?? includeArchivedRef.current;
try { try {
const fetchedTasks = await api.fetchTasks(undefined, undefined, projectId, query, wantArchived); const fetchedTasks = await api.fetchTasks(undefined, undefined, requestProjectId, query, wantArchived);
if (fetchVersionRef.current !== requestVersion) { // Reject if project changed (compare against the projectId at request time) or version is stale
if (fetchVersionRef.current !== requestVersion || projectId !== requestProjectId) {
return; return;
} }
setTasks(fetchedTasks.map(normalizeTask)); setTasks(fetchedTasks.map(normalizeTask));
// Record when we received fresh server data for stuck detection // Record when we received fresh server data for stuck detection
lastFetchTimeMs.current = Date.now(); lastFetchTimeMs.current = Date.now();
} catch { } catch {
if (fetchVersionRef.current !== requestVersion) { // Reject if project changed or version is stale
if (fetchVersionRef.current !== requestVersion || projectId !== requestProjectId) {
return; return;
} }
if (options?.clearOnError) { if (options?.clearOnError) {
@@ -133,9 +150,8 @@ export function useTasks(options?: UseTasksOptions) {
}, [refreshTasks]); }, [refreshTasks]);
// SSE live updates // SSE live updates
// Note: In multi-project mode, SSE receives all task events. // Note: SSE events from stale project contexts are ignored via projectContextVersionRef.
// Tasks are filtered by ID match, so cross-project updates won't affect // This prevents tasks from the previous project from appearing during project switches.
// the local state since task IDs are unique and we only fetch from one project.
useEffect(() => { useEffect(() => {
let closedByCleanup = false; let closedByCleanup = false;
let reconnectTimer: ReturnType<typeof setTimeout> | null = null; let reconnectTimer: ReturnType<typeof setTimeout> | null = null;
@@ -143,6 +159,10 @@ export function useTasks(options?: UseTasksOptions) {
if (connectionNonce > 0) { if (connectionNonce > 0) {
void refreshTasksRef.current(); void refreshTasksRef.current();
} }
// Capture the project context version at effect start.
// Any event from a stale SSE connection (created before a project switch)
// will have a different context version and will be ignored.
const contextVersionAtStart = projectContextVersionRef.current;
const query = projectId ? `?projectId=${encodeURIComponent(projectId)}` : ""; const query = projectId ? `?projectId=${encodeURIComponent(projectId)}` : "";
const es = new EventSource(`/api/events${query}`); const es = new EventSource(`/api/events${query}`);
@@ -161,6 +181,10 @@ export function useTasks(options?: UseTasksOptions) {
resetHeartbeat(); resetHeartbeat();
const handleCreated = (e: MessageEvent) => { const handleCreated = (e: MessageEvent) => {
// Guard: reject events from stale project contexts
if (projectContextVersionRef.current !== contextVersionAtStart) {
return;
}
resetHeartbeat(); resetHeartbeat();
const task = normalizeTask(JSON.parse(e.data) as Task); const task = normalizeTask(JSON.parse(e.data) as Task);
// When search is active, re-fetch to get server-filtered results // When search is active, re-fetch to get server-filtered results
@@ -168,9 +192,7 @@ export function useTasks(options?: UseTasksOptions) {
void refreshTasksRef.current({ searchQueryOverride: searchQueryRef.current }); void refreshTasksRef.current({ searchQueryOverride: searchQueryRef.current });
return; return;
} }
// In project mode, only add if this task belongs to our project // Add the task if it doesn't already exist
// Since we can't determine project from event, we add and let subsequent
// fetches correct the state, or filter by checking if task exists in our set
setTasks((prev) => { setTasks((prev) => {
// Avoid duplicates // Avoid duplicates
if (prev.some((t) => t.id === task.id)) return prev; if (prev.some((t) => t.id === task.id)) return prev;
@@ -179,6 +201,10 @@ export function useTasks(options?: UseTasksOptions) {
}; };
const handleMoved = (e: MessageEvent) => { const handleMoved = (e: MessageEvent) => {
// Guard: reject events from stale project contexts
if (projectContextVersionRef.current !== contextVersionAtStart) {
return;
}
resetHeartbeat(); resetHeartbeat();
// When search is active, re-fetch to get server-filtered results // When search is active, re-fetch to get server-filtered results
if (searchQueryRef.current) { if (searchQueryRef.current) {
@@ -197,6 +223,10 @@ export function useTasks(options?: UseTasksOptions) {
}; };
const handleUpdated = (e: MessageEvent) => { const handleUpdated = (e: MessageEvent) => {
// Guard: reject events from stale project contexts
if (projectContextVersionRef.current !== contextVersionAtStart) {
return;
}
resetHeartbeat(); resetHeartbeat();
// When search is active, re-fetch to get server-filtered results // When search is active, re-fetch to get server-filtered results
if (searchQueryRef.current) { if (searchQueryRef.current) {
@@ -234,6 +264,10 @@ export function useTasks(options?: UseTasksOptions) {
}; };
const handleDeleted = (e: MessageEvent) => { const handleDeleted = (e: MessageEvent) => {
// Guard: reject events from stale project contexts
if (projectContextVersionRef.current !== contextVersionAtStart) {
return;
}
resetHeartbeat(); resetHeartbeat();
// When search is active, re-fetch to get server-filtered results // When search is active, re-fetch to get server-filtered results
if (searchQueryRef.current) { if (searchQueryRef.current) {
@@ -245,6 +279,10 @@ export function useTasks(options?: UseTasksOptions) {
}; };
const handleMerged = (e: MessageEvent) => { const handleMerged = (e: MessageEvent) => {
// Guard: reject events from stale project contexts
if (projectContextVersionRef.current !== contextVersionAtStart) {
return;
}
resetHeartbeat(); resetHeartbeat();
// When search is active, re-fetch to get server-filtered results // When search is active, re-fetch to get server-filtered results
if (searchQueryRef.current) { if (searchQueryRef.current) {