fix: address code review findings across sync, tasks, and UI (#6339)

- fix(tasks): preserve selectAllTasks memoization by only filtering when
  undefined entities exist, avoiding new array allocation on every call
- fix(sync): replace mutable ARGON2_PARAMS export with getter/setter to
  prevent test pollution across spec files
- fix(sync): convert tag-task-page from async pipe to toSignal pattern
  to fix OnPush change detection after bulk sync state updates
- fix(sync): remove E2E navigation workarounds that masked the rendering
  bug now fixed by the toSignal conversion
- fix(sync): add clarifying comments for archive-wins sibling conflict
  resolution and waitForSyncWindow switchMap concurrency semantics
- fix(ios): add hex preview of first 16 bytes to WebDAV UTF-8 decode
  error for easier debugging
- docs: add JSDoc to getDiffInWeeks noting negative diff behavior

https://claude.ai/code/session_01Y51QDFEdvrJ9VVgp9XfWLJ

Co-authored-by: Claude <noreply@anthropic.com>

e2e/tests/sync/webdav-provider-switch.spec.ts

@@ -414,13 +414,6 @@ test.describe('@webdav WebDAV Provider Switch', () => {
     await syncPageC.triggerSync();
     await waitForSyncComplete(pageC, syncPageC);
 
-    // TODO: Navigation is a workaround for Angular not re-rendering task list
-    // after remote sync ops are applied to NgRx. Investigate whether the root
-    // cause is in HydrationStateService cooldown or change detection.
-    await pageC.goto('/#/tag/TODAY/tasks');
-    await pageC.waitForLoadState('networkidle');
-    await workViewPageC.waitForTaskList();
-
     // C should have 2 tasks (from A and B)
     await expect(pageC.locator('task')).toHaveCount(2, { timeout: 30000 });
     console.log('[Three Client Test] C: Joined and received 2 tasks');
@@ -455,11 +448,6 @@ test.describe('@webdav WebDAV Provider Switch', () => {
     await syncPageC.triggerSync();
     await waitForSyncComplete(pageC, syncPageC);
 
-    // TODO: Navigation workaround — see comment above for pageC's first sync
-    await pageC.goto('/#/tag/TODAY/tasks');
-    await pageC.waitForLoadState('networkidle');
-    await workViewPageC.waitForTaskList();
-
     // CRITICAL: C should have B's task (this was the bug!)
     await expect(pageC.locator('task')).toHaveCount(4, { timeout: 30000 });
     await expect(pageC.locator('task', { hasText: taskB2 })).toBeVisible({
@@ -471,19 +459,6 @@ test.describe('@webdav WebDAV Provider Switch', () => {
     await syncPageA.triggerSync();
     await waitForSyncComplete(pageA, syncPageA);
 
-    // TODO: Navigation workaround — see comment above for pageC's first sync
-    await pageA.goto('/#/tag/TODAY/tasks');
-    await pageA.waitForLoadState('networkidle');
-    await workViewPageA.waitForTaskList();
-
-    await pageB.goto('/#/tag/TODAY/tasks');
-    await pageB.waitForLoadState('networkidle');
-    await workViewPageB.waitForTaskList();
-
-    await pageC.goto('/#/tag/TODAY/tasks');
-    await pageC.waitForLoadState('networkidle');
-    await workViewPageC.waitForTaskList();
-
     // All clients should have 4 tasks
     await expect(pageA.locator('task')).toHaveCount(4, { timeout: 30000 });
     await expect(pageB.locator('task')).toHaveCount(4, { timeout: 30000 });

e2e/tests/sync/webdav-sync-archive.spec.ts

@@ -455,17 +455,6 @@ test.describe('@webdav WebDAV Archive Sync', () => {
     // With archive-wins rule, archive always wins regardless of timestamps.
     // Archive is an explicit user intent ("I'm done with these tasks"),
     // so it takes priority over concurrent edits.
-    // TODO: Navigation is a workaround for Angular not re-rendering task list
-    // after remote sync ops are applied to NgRx. Investigate whether the root
-    // cause is in HydrationStateService cooldown or change detection.
-    await pageA.goto('/#/tag/TODAY/tasks');
-    await pageA.waitForLoadState('networkidle');
-    await workViewPageA.waitForTaskList();
-
-    await pageB.goto('/#/tag/TODAY/tasks');
-    await pageB.waitForLoadState('networkidle');
-    await workViewPageB.waitForTaskList();
-
     // With archive-wins rule, archive always wins regardless of timestamps
     await expect(pageA.locator('task')).toHaveCount(0, { timeout: 30000 });
     await expect(pageB.locator('task')).toHaveCount(0, { timeout: 30000 });

ios/App/App/WebDavHttpPlugin.swift

@@ -81,7 +81,8 @@ public class WebDavHttpPlugin: CAPPlugin, CAPBridgedPlugin {
             let bodyString: String
             if let responseData = responseData, !responseData.isEmpty {
                 guard let decoded = String(data: responseData, encoding: .utf8) else {
-                    call.reject("Failed to decode response as UTF-8 (\(responseData.count) bytes)", "DECODE_ERROR")
+                    let preview = responseData.prefix(16).map { String(format: "%02x", $0) }.joined(separator: " ")
+                    call.reject("Failed to decode response as UTF-8 (\(responseData.count) bytes, first bytes: \(preview))", "DECODE_ERROR")
                     return
                 }
                 bodyString = decoded

src/app/features/tasks/store/task.selectors.ts

@@ -278,11 +278,13 @@ export const selectAllTasks = createSelector(
   selectTaskFeatureState,
   (state: TaskState): Task[] => {
     const all = selectAll(state);
-    const filtered = all.filter((task): task is Task => !!task);
-    if (filtered.length !== all.length) {
+    // Only filter when undefined entities exist — otherwise return the original
+    // memoized array from selectAll to avoid breaking NgRx selector memoization.
+    if (all.some((task) => !task)) {
       devError('selectAllTasks: found undefined entities in task state');
+      return all.filter((task): task is Task => !!task);
     }
-    return filtered;
+    return all;
   },
 );
 

src/app/op-log/encryption/encryption.spec.ts

@@ -9,21 +9,19 @@ import {
   decryptWithDerivedKey,
   clearSessionKeyCache,
   getSessionKeyCacheStats,
-  ARGON2_PARAMS,
+  setArgon2ParamsForTesting,
 } from './encryption';
 
 describe('Encryption', () => {
   const PASSWORD = 'super_secret_password';
   const DATA = 'some very secret data';
 
-  const ORIGINAL_ARGON2_PARAMS = { ...ARGON2_PARAMS };
-
   beforeAll(() => {
-    Object.assign(ARGON2_PARAMS, { memorySize: 8, iterations: 1 });
+    setArgon2ParamsForTesting({ parallelism: 1, memorySize: 8, iterations: 1 });
   });
 
   afterAll(() => {
-    Object.assign(ARGON2_PARAMS, ORIGINAL_ARGON2_PARAMS);
+    setArgon2ParamsForTesting();
   });
 
   it('should encrypt and decrypt data correctly', async () => {

src/app/op-log/encryption/encryption.ts

@@ -8,12 +8,32 @@ const SALT_LENGTH = 16;
 const IV_LENGTH = 12;
 const KEY_LENGTH = 32;
 
-export const ARGON2_PARAMS = {
+const DEFAULT_ARGON2_PARAMS = {
   parallelism: 1,
   iterations: 3,
   memorySize: 65536, // 64 MB - memorySize is in KiB
 };
 
+let _argon2Params = { ...DEFAULT_ARGON2_PARAMS };
+
+/**
+ * Returns the current Argon2 parameters.
+ * Tests can override these via `setArgon2ParamsForTesting()`.
+ */
+export const getArgon2Params = (): typeof DEFAULT_ARGON2_PARAMS => _argon2Params;
+
+/**
+ * Override Argon2 parameters for testing (use weak params to speed up tests).
+ * Pass `undefined` to restore defaults.
+ */
+export const setArgon2ParamsForTesting = (
+  params?: Partial<typeof DEFAULT_ARGON2_PARAMS>,
+): void => {
+  _argon2Params = params
+    ? { ...DEFAULT_ARGON2_PARAMS, ...params }
+    : { ...DEFAULT_ARGON2_PARAMS };
+};
+
 // ============================================================================
 // WEBCRYPTO AVAILABILITY CHECK
 // ============================================================================
@@ -68,13 +88,14 @@ const deriveKeyBytesArgon = async (
   password: string,
   salt: Uint8Array,
 ): Promise<Uint8Array> => {
+  const params = getArgon2Params();
   return await argon2id({
     password,
     salt,
     hashLength: KEY_LENGTH,
-    parallelism: ARGON2_PARAMS.parallelism,
-    iterations: ARGON2_PARAMS.iterations,
-    memorySize: ARGON2_PARAMS.memorySize,
+    parallelism: params.parallelism,
+    iterations: params.iterations,
+    memorySize: params.memorySize,
     outputType: 'binary',
   });
 };

src/app/op-log/sync/conflict-resolution.service.ts

@@ -372,7 +372,12 @@ export class ConflictResolutionService {
         localWinsRemoteOps.push(...resolution.conflict.remoteOps);
         remoteOpsToReject.push(...resolution.conflict.remoteOps.map((op) => op.id));
 
-        // Store the new update op (will be uploaded on next sync)
+        // Store the new update op (will be uploaded on next sync).
+        // Note: localWinOp is undefined for archive-wins sibling conflicts
+        // (non-archive conflicts for an entity being archived). These resolve
+        // as local-wins to prevent remote ops from resurrecting the entity,
+        // but no new op is needed — the archive-win op from the sibling
+        // conflict already covers the entity.
         if (resolution.localWinOp) {
           newLocalWinOps.push(resolution.localWinOp);
           OpLog.warn(

src/app/pages/tag-task-page/tag-task-page.component.html

@@ -1,6 +1,6 @@
 <work-view
-  [backlogTasks]="(workContextService.backlogTasks$ | async) || []"
-  [doneTasks]="(workContextService.doneTasks$ | async) || []"
+  [backlogTasks]="backlogTasks()"
+  [doneTasks]="doneTasks()"
   [isShowBacklog]="false"
-  [undoneTasks]="(workContextService.undoneTasks$ | async) || []"
+  [undoneTasks]="undoneTasks()"
 ></work-view>

src/app/pages/tag-task-page/tag-task-page.component.ts

@@ -1,6 +1,6 @@
 import { ChangeDetectionStrategy, Component, inject } from '@angular/core';
 import { WorkContextService } from '../../features/work-context/work-context.service';
-import { AsyncPipe } from '@angular/common';
+import { toSignal } from '@angular/core/rxjs-interop';
 import { WorkViewComponent } from '../../features/work-view/work-view.component';
 
 @Component({
@@ -8,8 +8,12 @@ import { WorkViewComponent } from '../../features/work-view/work-view.component'
   templateUrl: './tag-task-page.component.html',
   styleUrls: ['./tag-task-page.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
-  imports: [AsyncPipe, WorkViewComponent],
+  imports: [WorkViewComponent],
 })
 export class TagTaskPageComponent {
-  workContextService = inject(WorkContextService);
+  private _workContextService = inject(WorkContextService);
+
+  backlogTasks = toSignal(this._workContextService.backlogTasks$, { initialValue: [] });
+  doneTasks = toSignal(this._workContextService.doneTasks$, { initialValue: [] });
+  undoneTasks = toSignal(this._workContextService.undoneTasks$, { initialValue: [] });
 }

src/app/util/get-diff-in-weeks.ts

@@ -1,3 +1,9 @@
+/**
+ * Returns the number of complete weeks between d1 and d2 (d2 - d1).
+ * Uses Math.floor so partial weeks round toward zero for positive diffs.
+ * NOTE: Callers always pass d1 <= d2 (start date, then later check date).
+ * For negative diffs, Math.floor rounds away from zero (e.g. -0.5 → -1).
+ */
 export const getDiffInWeeks = (d1: Date, d2: Date): number => {
   const d1Copy = new Date(d1);
   const d2Copy = new Date(d2);

src/app/util/wait-for-sync-window.operator.ts

@@ -23,6 +23,15 @@ const SYNC_WINDOW_TIMEOUT_MS = 30000;
  * derived from Angular signals) and proceeds once the window closes.
  * A 30-second timeout ensures the pipeline never stalls permanently.
  *
+ * ## Concurrency Note
+ *
+ * This operator uses `switchMap` internally, so if a **new** value arrives
+ * while an earlier value is still waiting for the sync window to close, the
+ * earlier wait is cancelled and only the latest value proceeds. This is
+ * intentional for the current use case (day-change strings), where only the
+ * most recent date matters. Do **not** reuse this operator for sources where
+ * every emission must be preserved — use `concatMap` semantics instead.
+ *
  * ## When to Use
  *
  * Use this operator instead of `skipDuringSyncWindow()` for stream-based effects