mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
21523 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Johannes Millan
|
238b91aa59
|
test(e2e): accept beforeunload prompt in shared dialog fallback (#9122)
The USE_REMOTE crash-resume spec reloads Client B mid-sync. While isSyncInProgress is set, startup.service's beforeunload handler calls preventDefault(), so Chrome raises a beforeunload prompt (the preceding button click supplies the user activation it requires). Playwright auto-accepts such prompts only when no 'dialog' listener is registered — but installDevErrorDialogHandler registers one on every page and early- returned on beforeunload, leaving the prompt unanswered. The navigation then never starts and reload() times out (observed on SuperSync shard 6/6; the earlier load→domcontentloaded de-flake could not help because navigation is blocked before any lifecycle event). Accept beforeunload in the shared fallback, restoring Playwright's default; other dialog types keep flowing to spec-specific handlers. Corrects the two crash-resume comments that encoded the wrong theory. |
||
|
Johannes Millan
|
c18055a341
|
fix(android): stop the widget labelling a stale list as Today (#9098) (#9118)
The widget renders a pre-computed snapshot Angular pushes into `widget_data`; the blob carried no expiry, so a process that stayed dead across midnight kept rendering yesterday's tasks under a hardcoded "Today" header, indefinitely. Native cannot recompute the list. TODAY_TAG membership is virtual, today's repeat instances do not exist as entities until TaskDueEffects materializes them on a day change, and overdue carry-over runs there too — so there is no persisted field a native filter could read that would give the right answer even in principle. Only running the app can produce today's list. So make the widget honest instead of wrong: Angular stamps the snapshot with `validUntil`, the instant it stops being today (start-of-next-day offset included), and `dayStr` for the label. Native's whole verdict is `WidgetData.headerFor` → `now >= validUntil`; when stale it renders the snapshot's own date instead of "Today". The list stays visible and useful, it just stops claiming to be today's. A stale snapshot whose day cannot be read says "Outdated" rather than falling back to the very lie this fixes. Shipping the boundary rather than its inputs keeps the app's calendar rules in one language. The iOS port (#8950) can consume `validUntil` unchanged instead of mirroring getDbDateStr semantics into Swift, where a non-Gregorian default locale would silently misread the day. The verdict is a pure function so it is unit-testable: this project has no Robolectric, and a decision left inside the provider could ship inverted and green. Both refresh paths rebuild the header. A push can change the day the blob describes; a tap cannot, but it re-renders at a later `now` than the last verdict was computed at — and it is the one interaction that reaches our code while the app is dead, so a tap on a new day must not redraw rows under a "Today" header. Also fixed: onUpdate re-registered the adapter with an unchanged intent, which does not re-invoke the factory's onDataSetChanged(), so the periodic update rendered whatever rows the adapter last built. `v` stays 1: the fields are additive, and parse() returns empty for any other version, so a bump would blank the widget of every install until next opened. A pre-#9098 blob has no `validUntil` and is never reported stale — unknown must not read as expired. Known bounds, documented in the plan rather than papered over: the label flips only on a push, a tap, or the inexact Doze-deferred 30-min periodic update, and the launcher paints cached views on unlock — so the lie is bounded, not eliminated. Force-stop is not a gap: the system masks a stopped package's widget entirely, so the reported symptom can only occur in the Doze band. An exact alarm at `validUntil` would close the rest; deferred on cost/scope, not because it would not work. Verified: 25/25 Kotlin across 6 timezones incl. midnight-gap zones (Santiago, Apia); 203/203 Angular in both timezones CI runs. Boundary math asserted against the real getDbDateStr rollover. Sabotage-measured, not assumed: inverting the verdict fails 5 tests, >=→> fails 4, dropping the validUntil guard fails 3, Locale.US→getDefault fails 1, dropping the day round-trip fails 1. |
||
Jon Kilroy
|
d5afe62016
|
feat(schedule): add a single-day view to the Schedule tab (#9058)
* feat(schedule): allow 'day' as a persisted time-view mode * feat(schedule): compute single-day range and header for day view * feat(schedule): add day-view toggle and single-day labels Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JzGvAaeT2TrZcEUA6fYPSZ * docs(schedule): note the day view mode in the Schedule wiki * fix(schedule): make the day-view header compact and locale-aware Address review feedback on the single-day view: - Build the day title with a locale-aware Intl skeleton instead of a hardcoded 'EEE, MMM d, yyyy' pattern, which pinned en-US field ordering. - Shrink the title responsively so it never gets ellipsis-clipped: show the full weekday + date + year on roomy widths, and fall back to a compact month + day below TABLET, where the toggle group and nav controls leave too little room. The weekday still shows in the day-column header. - Rename the .day-view-btn test hook to .e2e-day-view-btn per convention. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(schedule): rename week-month-* classes to time-view-* The toggle group now has three buttons (Day, Week, Month), so the week-month-selector / week-month-btn names no longer describe it. Rename to time-view-selector / time-view-btn. Pure CSS class rename, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(schedule): complete time-view-* rename in zen theme The class rename left zen.css targeting .week-month-*, so its overrides matched nothing. The :not(.active) rule is the load-bearing one: its !important suppresses the toggle's hover highlight, which zen exists to strip. Co-authored-by: Jon Kilroy <jkusa@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Johannes Millan <johannes.millan@gmail.com> Co-authored-by: Jon Kilroy <jkusa@users.noreply.github.com> |
||
coyaSONG
|
5097155e25
|
feat(work-view): show custom section task counts #9068 (#9109) | ||
|
Johannes Millan
|
99f923ad6c
|
docs(sync): fix stale schema-compat docs, add bump policy, rescope Task 6 (#9119)
* docs(sync): fix stale schema docs, add bump policy, rescope Task 6 * docs(sync): add severity-triage and schema-bump rules to AGENTS.md * docs(sync): fix review findings in schema compat docs |
||
Johannes Millan
|
6756e9998c |
docs(sync): record fast-track simplification audit
Capture the read-only audit evidence and its verified and provisional risk/reward guides in one baseline-preserving changeset. |
||
|
Johannes Millan
|
4519c5f4d3 |
docs: correct simplification audit findings
Replace stale placements for revised scopes, separate line-level LOC categories, and neutralize superseded implementation guidance. |
||
|
Johannes Millan
|
0204210c5b |
docs: document non-sync simplification audit
Record the complete non-sync audit, two-pass candidate review, conservative reconciliation, global risk/reward order, and verified stashed implementation state. |
||
|
Johannes Millan
|
fdbc2c1a86
|
refactor(sync): centralize clock pruning in store, make merge atomic (#9107)
Follow-up to
|
||
|
Johannes Millan
|
fcbb789746
|
docs(sync): record #9105 staleness-eviction plan, improve prune snack (#9106) | ||
|
Johannes Millan
|
a224eb5fa3
|
fix(sync): keep import author in client-side clock pruning (#9096) (#9102)
The client pruned its durable vector clock with uploader-only protection, so once 21+ client ids accumulated after a full-state import, the import author's low-counter entry was evicted. Every subsequent local op then permanently failed the sync-import filter's knows-import-counter rescue and was dropped as CONCURRENT on every peer — the client-side ceiling of the server fix in #9089. - client limitVectorClockSize wrapper now takes a preserve list, matching the shared implementation - calculateRemoteClockMerge (remote merge + reducer checkpoint) preserves the latest full-state author; an in-batch full-state op supersedes the stored one, and the checkpoint resolves the author inside its transaction so a just-rejected import cannot name the protected author - snapshot save, compaction, hydrator restore, and the sync-hydration file-snapshot bootstrap protect the author on their durable-clock paths - docs: add the missing calculateRemoteClockMerge prune site, correct the stale RepairOperationService rows (repair ships the full clock), and reword the sync-core pruning comment to the real invariant |
||
|
Johannes Millan
|
b1f074705a
|
test(project): guard project navigation route detection (#9103) | ||
ghostkeni
|
67cd017bc7
|
Add critical tests for task and sync helper flows (#9079)
* test: add critical coverage for task and sync helpers Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test(tasks): fix vacuous alert assertion and zero-delta time test - Reset devError latch and alert spy call history before orphan-subtask assertion so the test is not vacuous in a full Karma suite run - Change DAY_3 timeSpent from 60 to 90 in updateTimeSpentForTask test so totalDelta is non-zero (+30), making the accumulator mutation detectable by mutation testing - Add isLww=false short-circuit case to bulk-archive-filter spec Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
Johannes Millan
|
e5a9ff7866
|
fix(sync): unfreeze the disjoint-field merge to stop data loss (#9095) (#9101)
Drop disableDisjointMerge from the production conflict-resolution entry point. With the merge frozen (#9061), concurrent edits to different fields of one entity resolve by whole-entity LWW: the later side wins a full 'replace' snapshot with a dominating clock and the other side's edit is silently and permanently lost on every client (a rename dies when another device marks the task done). The journal half of the freeze stays. Wire-safe: v18.14.0 predates lwwUpdateMode entirely and applies LWW Update payloads via updateOne, so the merge's 'patch' union-delta merges cleanly on released clients (#8874's back-compat envelope design). Restores the strengthened 3.1 disjoint-merge E2E parked out of #9089 (it fails deterministically while the merge is frozen) and adds a unit regression for the exact rename-vs-done pair; the caller guard spec now pins that the merge stays enabled on the production resolve path. |
||
|
Johannes Millan
|
b50d5f6d96
|
fix(sync): dedupe surgical-sync retries and keep the import author through pruning (#9089)
* fix(sync): deduplicate surgical sync retries Persist split-file configuration and acknowledge operation IDs already committed remotely after a lost upload response. * fix(sync): preserve import author during clock pruning Keep the active causal full-state author in oversized stored clocks and reuse the stored protected IDs when classifying response-loss retries. * test(sync): harden supersync failure coverage Exercise real upload endpoints and exact operation IDs across response loss, validation failures, schema blockers, full-state boundaries, vector pruning, and concurrent edits. * fix(sync): heal a corrupt primary on duplicate-only uploads The .bak recovery path caches the CORRUPT primary's rev precisely so this cycle's conditional overwrite repairs sync-ops.json. The duplicate-retry short-circuit read that cache and returned before the write, leaving the primary corrupt whenever the recovered buffer already held every pending op. Flag the recovered entry and let those uploads fall through. Also stop synthesising a serverSeq for ops already in the buffer: the field is optional, the upload and download paths number ops differently, and a mixed batch could hand two ops the same value. * perf(sync): look the full-state author up once per upload Batch upload is off by default, so the guarded batch path was not the one serving production: the serial path queries the causal full-state author per op inside a single transaction, and a clock of 21-50 entries passes validation and trips the guard on every one of them. Memoize the author per transaction and resolve it lazily, so only an op whose clock actually overflows pays, and only once. This also retires the batch pre-scan and its loop-carried author, leaving both paths on one mechanism. Report the lookup through ProcessOperationResult so the upload summary stops under-reporting round-trips, and record why reconstructing the stored protected set loosens id-collision detection. * test(sync): wait for the committed title in renameTask renameTask blurred the textarea, slept 300ms and returned without ever checking the rename landed. Blur -> dispatch -> re-render outruns that delay on a loaded machine, so a following sync uploads without the rename op and the caller asserts against a task that was never renamed — which is what supersync 3.1 hits on CI but never locally. Wait for the new title instead, mirroring markTaskDone's done-state wait and the e2e no-waitForTimeout rule. * test(sync): dispatch focus so renameTask actually commits renameTask relied on el.focus() to emit a focus event, but these tests drive two clients as separate pages and only one page can hold focus, so on CI the event often never fires. TaskTitleComponent then keeps _isFocused=false, and resetToLastExternalValueTrigger resets tmpValue to the stored title on the next task-object emission. Blur therefore computes wasChanged=false, task.component skips update(), and the rename is silently dropped without ever becoming an op. That is supersync 3.1: client A's rename lives only in tmpValue, A syncs and uploads nothing, B uploads its done op, A downloads it, the task ref changes and the title reverts to the original — exactly the state the CI artifact captured. A real user always has real focus, so the app itself is unaffected. Dispatch focus explicitly, mirroring the synthetic input/blur already used here. Also correct the previous commit's claim: the toBeVisible wait matches tmpValue, a component-local signal rendered in both template branches, so it never observed the committed title and could not have fixed this. * docs: revert incidental prettier reformat of unrelated docs The master merge ran prettier across files it pulled in, reformatting three documents this branch has no business touching: markdown table cell padding plus *emphasis* -> _emphasis_, with no content change. handover.md documents two unrelated branches entirely. Restores them to master. vector-clocks.md keeps its edits — those are this branch's own and describe the pruning protection. * refactor(sync): drop the full-state author lookup's roundtrip accounting resolveFullStateAuthor memoizes per transaction, so the lookup it counts fires at most once per upload — the plumbing existed to report a number that is always 0 or 1, on a log line already counting dozens. The memo and its own accounting cancelled out. Removing didQuery lets resolveFullStateAuthor return string | undefined and getPruneProtectedIds return string[], instead of both carrying a tuple purely to feed the counter. uploadDbRoundtrips and the batch path's own counter are untouched. * test(sync): assert the committed store title in renameTask The focus dispatch did not fix supersync 3.1 — the shard failed again with an identical snapshot (original title, done, rename gone), so that diagnosis was wrong. Stop guessing at the trigger and make the helper able to observe the thing in question. task-title renders tmpValue, a component-local signal, in BOTH its editing and idle branches. Every DOM assertion here therefore matches as soon as the synthetic input event fires, whether or not an op was ever captured — which is why two rounds of "wait for the title" changed nothing. Read the store instead, via the __e2eTestHelpers.store hook the timeSpent helper already uses. This is a diagnostic as much as a fix: it splits the two remaining explanations. If renameTask now fails, the rename never becomes an op and the bug is in how the test drives the edit. If it passes and 3.1 still fails at the merge assertion, the op is captured and lost during sync — a real defect, and the test is right to fail. * test(sync): move the 3.1 disjoint-merge rewrite out to #9095 3.1 was the last red shard, and it turned out to be right: the store-backed renameTask passes, so the rename IS captured as an op, and the test still fails at the merge assertion — the op is committed and then lost during sync. Filed as #9095. That bug is pre-existing and cannot be reached by anything in this PR: the file-based adapter is not used by SuperSync, and the server-side author memo only engages for clocks over 20 entries where this test carries about three. 3.1 is also the only test here that exercises neither of this PR's fixes — it races a title change against a move-to-done, which is conflict resolution, not retry dedup or clock pruning. So it moves to #9095 rather than holding verified sync fixes red. The rest of the hardening stays: the fault injections whose globs never matched a real endpoint, the schema-mismatch test that asserted nothing, and the compaction suite that called an endpoint which never existed are what actually cover the fixes here. Restoring the old 3.1 puts a misleading test back, so it now carries a comment saying why it proves little and where the real one lives. The strengthened version is kept on test/issue-9095-disjoint-merge-repro. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
Johannes Millan
|
0238b5a729
|
fix(sync): fence in-flight sync cycles across destructive config changes (#9088)
* fix(sync): fence in-flight sync cycles across config changes (#9074) Destructive config changes (encryption enable/disable/password change, provider/account switch) blocked NEW sync cycles but neither drained nor cancelled a cycle already mid-await, which could then apply remote ops, upload, acknowledge, or advance the cursor against the new epoch/target (cross-epoch/cross-provider contamination, mixed-key server history). Fix, per the issue's KISS sketch: - Monotonic sync epoch on SyncProviderManager, bumped AFTER each provider switch / target-moving config write / bypass ingress, and at runWithSyncBlocked entry. Content-only saves do not bump. - Every cycle entry point (main sync, immediate upload, WS download, force upload) captures the epoch with its cycle-guard claim and threads it as fenceEpoch. - Provider I/O is fenced in one choke point: getOperationSyncCapable() returns a per-cycle delegate that re-asserts the epoch before every provider call (uploads, downloads, all setLastServerSeq cursor writes). Local writes (apply-lock closures incl. the full-state path, deferred acks, hydration, migration appends, rejected-ops handling, raw-rebuild resume) re-assert at the call site. - Stale completions throw SyncEpochChangedError, handled everywhere as a benign abort (no error snack; UNKNOWN_OR_CHANGED) — each abort point lands in a crash-equivalent state by design. - runWithSyncBlocked is serialized, sets the block flag FIRST, bumps the epoch, then drains the main sync AND the cycle-guard side channels (bounded, throws on timeout) — the fence cannot recall request bytes already on the wire, so destructive remote writes wait for the stale cycle to settle. ImmediateUploadService now routes through WrappedProviderService instead of a raw cast. Closes the audit findings C1-1 (ORCH-1) and C1-2 (SWITCH-2). * fix(sync): don't bump the sync epoch on first-time setup (#9074) Every conflict-dialog E2E timed out on `SyncEpochChangedError (1 → 2)`: the first-ever provider activation and the first-ever config save (no previous privateCfg, so isSyncTargetChanged reports a target change) both bumped the epoch, racing the fresh config's first sync into a spurious abort — the dialog-producing cycle itself was fenced. First-time setup has no OLD target an in-flight cycle could be running against (a pre-activation cycle sees getActiveProvider() === null and exits; a pre-first-save cycle is not ready), so these bumps fence nothing and only cause false positives. Gate them: bump on a config save only when a previous config existed, and on activation only when a previous provider was active. Real switches (X→Y, X→null) and real target moves still bump; the cache-invalidation emission keeps its true-on-first-save semantics untouched. * fix(sync): read the (provider, epoch) fence pair in one sync block (#9074) The SuperSync provider-switch E2E still aborted its first post-switch sync: the cycle captured the fence epoch at its guard claim but fetched the provider object several awaits later, so a switch completing in between handed the cycle the NEW provider with a STALE epoch — the fence then aborted a cycle that was actually running against the new target. The consistency rule is pair atomicity, not claim atomicity: a switch swaps the provider object and bumps the epoch in one synchronous block, so reading getActiveProvider() and syncEpoch in one synchronous block always yields a consistent pair (old+old aborts on the later bump; new+new proceeds). Move the capture to the provider read in the main sync and WS-download cycles (immediate upload aligned for uniformity — it was same-block already only by accident of having no await between). |
||
|
Johannes Millan
|
57cee868bf
|
fix(locale): consolidate textLocale, fix planner month label, enforce via lint (#8987) (#9065)
* refactor(locale): collapse inlined textLocale copies into the helper No behavior change: textLocale() is defined as isoTextLocale() ?? currentLocale(), which is exactly what these five sites had inlined. #9056 was based on master and so could not use the helper #9055 adds — it re-inlined the expression at focus-session, habit-tracker, worklog, scheduled-list and scheduled-date-group. Now that both have landed, collapse them so there is one canonical spelling instead of six. Caller specs mock textLocale() directly rather than the isoTextLocale/ currentLocale pair, mirroring what each SUT actually calls. * fix(locale): planner month label followed the browser locale, not the app's monthLabel passed no locale to toLocaleDateString, so the spelled-out month followed the *browser's* locale and ignored both the configured date locale and the UI language: a German browser rendered 'Juli 2026' in an English app. Same family as #8987 but reachable without the ISO option at all. Route it through textLocale(), which is the UI language under the ISO option and currentLocale() otherwise. The two existing specs computed their expected value with the same undefined locale, so they mirrored the bug and could never have caught it. They now pin a fixed app locale, deliberately not the runner's browser locale — otherwise they would pass either way. * test(lint): add require-text-locale rule and enforce it over src/app Guards the #8987 invariant that kept recurring: spelled-out weekday/month names must be formatted with textLocale(), never currentLocale() (the ISO option's 'sv' sentinel) and never the implicit browser locale. Three PRs chased this bug class site-by-site because currentLocale() is the obvious-looking default at every new call site. - Resolves `const locale = ...currentLocale()` through the scope chain: that is the shape the original bug had in plannedStartDateStr, so a rule matching only direct calls would have missed the very bug it exists to prevent. - Covers new Intl.DateTimeFormat() too — the same trap in constructor form, used at ~9 sites. Stays silent on clock times (hour/minute/dayPeriod), which must keep currentLocale() so the ISO 24h format survives. - Specs excluded: computing an expected string against an explicit locale is a legitimate test technique; the invariant is about what the product renders. - Documents its own blind spots (locale threaded through a parameter, reassigned variables, non-literal options) and pins them as valid cases, per the no-multi-entity-effect convention. Error severity is safe: textLocale() equals currentLocale() for every non-ISO option, so for spelled-out names it is never worse. Zero violations remain. * fix(lint): keep require-text-locale silent on clock-time formats The rule documented itself as staying silent on clock times, but `dayPeriod` sat in ALWAYS_SPELLED_OUT and only `.toLocaleTimeString()` was excluded — so `{ hour, minute, dayPeriod }` via `Intl.DateTimeFormat`/`toLocaleString` did fire, and its message told the reader to switch to textLocale(). Following that advice flips the ISO 24h clock to 12h: "13:05" -> "1:05 in the afternoon". The same holds for any mixed date+time options object: `{ weekday, hour }` goes from "onsdag 13:05" to "Wednesday 1:05 PM" — a Swedish name traded for a broken clock, the exact ISO regression this rule family exists to prevent. A format that mixes a spelled-out name with a clock has no single correct locale; it has to be split (names on textLocale(), clock on currentLocale(), as plannedStartDateStr does), which is more than a one-locale message can advise. So skip any options object containing `hour`, and say so. This costs a blind spot on `{ weekday, hour }` — cheaper than confidently wrong advice at `error` severity. No call site changes: `dayPeriod`/`era` have zero uses in src/, so this was latent. All four real bug shapes are still caught; src/app stays at zero violations. The two clock-time `valid` cases named dayPeriod in their comments but only ever tested `{ hour, minute }` in their code, which is how this slipped through — pin the actual shapes instead. * fix(lint): catch dateStyle in require-text-locale The rule missed `dateStyle` entirely, so the canonical #8987 shape walked straight past it: `toLocaleDateString(currentLocale(), { dateStyle: 'full' })` renders "onsdag 15 juli 2026" under the sentinel — a spelled-out weekday and month — without naming weekday or month at all. Zero call sites today, so this was latent, but guarding call sites that do not exist yet is the rule's whole job. `dateStyle` needs its own value set rather than month's: the two invert. `month: 'short'` is "Jul" (spelled out) but `dateStyle: 'short'` is "2026-07-15" (numeric), so reusing SPELLED_OUT_VALUES would have flagged dateStyle:'short' and pushed the reader to route ISO's YYYY-MM-DD through textLocale() — the mirror of the clock-time trap. Modelled as a per-field map so the inversion is stated where it can't be conflated, and pinned from both sides: 'short' as valid, 'full'/'medium'/'long' as invalid. Sabotage-verified — swapping in month's value set fails the spec. `timeStyle` joins `hour` as a clock-time field: `{ dateStyle, timeStyle }` is the mixed date+time case again ("onsdag 15 juli 2026 kl. 13:05" -> "Wednesday, July 15, 2026 at 1:05 PM"), and it carries no `hour` key for the existing guard to catch. Verified: 6/6 real bug shapes flagged, 0/6 false positives on correct usage, src/app still at zero violations. |
||
|
Johannes Millan
|
d11815a4ab
|
fix(sync): guard snapshots against unpersisted state changes (#8751) (#9081)
* fix(sync): guard snapshots against unpersisted state changes (#8751) An unrecovered op-log persist failure used to leave the optimistic NgRx change in live state with no durable op behind it; once compaction (or the hydrator snapshot save) wrote that live state into state_cache, the phantom change became durable locally with no op to sync — permanent, silent cross-device divergence. - OperationCaptureService: sticky per-tab divergence flag, set on every keep-state failure path in OperationLogEffects (write-failure catch, invalid-entityId return, validation rejection — whose snack is now sticky) - phantom-change-guard.util: snapshotting is unsafe while the flag is set, captured writes are pending, or deferred sync-window actions are buffered; checked synchronously immediately before every live-state snapshot read - enforced at both state_cache writers (compaction incl. emergencyCompact, saveCurrentStateAsSnapshot), plus a sticky-flag fast-path before lock acquisition - meta-reducer-ordering spec: clear the module-level deferred buffer it leaked across spec files * test(sync): clear the deferred-action buffer globally in test setup The module-level deferred buffer is shared across the whole Karma context, so actions left in it leak into later spec FILES, where the phantom-change guard (#8751) reads it and silently skips compaction and snapshot saves. Clearing it per-file only fixes the one file that happens to buffer today; any future spec that buffers reintroduces the order-dependent failure. Move the clear next to the per-spec IDBFactory swap in src/test.ts, which already owns this kind of cross-spec hygiene. * docs(sync): correct quota-error reachability note in op-log effects The #8751 reasoning (and #9082) assume isQuotaExceededError never matches in production because _handleAppendError wraps quota errors into StorageQuotaExceededError extends Error. That holds only for the standard QuotaExceededError name: _handleAppendError matches that name alone, so Firefox's NS_ERROR_DOM_QUOTA_REACHED and legacy Safari's code 22 still arrive as raw DOMExceptions and do reach handleQuotaExceeded. The outcome is unchanged (emergencyCompact now always returns false there, since the failing write is still counted pending), but record the real reachability so nobody deletes the branch as dead code. * test(sync): pin phantom-guard position against compaction starvation The guard's liveness depends on WHERE it sits, and only the lower bound was documented (no awaits before the snapshot read). The upper bound was not: triggerCompaction() fires from inside the write path, so the triggering action is still counted pending when compact() runs and is decremented on a microtask chain once that write releases the op-log lock. The guard observes a settled counter only because it runs after the getPendingRemoteOps() await — a real IndexedDB round-trip that lets those microtasks drain first. Hoisting it above that await (a natural 'check the cheap guard first' refactor) makes it observe the still-pending write and skip on every attempt, starving compaction permanently. Verified no existing spec catches this: with the guard hoisted, the new spec is the only one of 67 that fails, since the others pin getPendingCount() to a constant. Add the regression spec and state the constraint at the guard. * test(sync): align snapshot guard coverage with write flush * test(sync): make the phantom-change guard tests fail without the guard Three of the four #8751 guard tests could not fail. The snapshot spec's describe stubbed getLastSeq and getStateSnapshot but not getCurrentVectorClock, so with the guard removed the save path threw (limitVectorClockSize on undefined) into saveCurrentStateAsSnapshot's outer catch and skipped the write anyway — the "should skip" assertions passed for the wrong reason. Stub the clock for the whole describe so the save actually reaches saveStateCache when unguarded. Also assert the flag test bails before the state read, not merely before the write: that pins the guard's position, mirroring the compaction fast-path test. Mutation-verified: with both flag paths disabled (the util's check and _doCompact's fast-path), exactly these 4 tests fail and nothing else — previously only 1 did. The quota-recovery fixture now uses Firefox's NS_ERROR_DOM_QUOTA_REACHED: per isQuotaExceededError's own docblock, the standard 'QuotaExceededError' name is wrapped into StorageQuotaExceededError (a plain Error) and can never reach that path, so the test was covering an unreachable spelling. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
Johannes Millan
|
aa94abd890
|
feat(sync): add one background full-sync scheduler (#9078)
* feat(sync): add one observable busy definition for sync work
Task 3 needs a single busy/idle definition so callers stop polling three
signals independently and disagreeing about what "busy" means. Two of the
three were only readable point-in-time: SyncCycleGuard.isActive is a plain
field and isEncryptionOperationInProgress is a getter, so a consumer could
ask "busy now?" but never be told when that stopped being true. No
behaviour change; nothing consumes SyncBusyService yet.
SyncCycleGuard now backs its state with a subject and exposes isActive$,
emitting on BOTH transitions. Claim-visibility is the load-bearing part:
the side channels (immediate upload, WS download) claim a cycle without
touching any other sync signal, so a release-only edge would report those
cycles as idle for their entire duration — exactly the coverage the guard
is in the union for. A spec pins that case.
released$ remains as sugar for the release edge, derived via pairwise() so
an idle guard cannot fire a phantom release at subscribe time. Both it and
isActive$ are re-check HINTS, not lock hand-offs: the guard's no-deadlock
property rests on callers never waiting on it, so the docblock states the
emission carries no claim and subscribers must still win tryBegin(). A
spec asserts two subscribers racing one edge get [true, false].
SyncBusyService merges the guard's activity with the wrapper's sync and
encryption signals and recomputes the whole union on every edge, so no
interleaving can desynchronise it. That ordering is load-bearing: sync()'s
finally clears isSyncInProgress$ while the guard is still held, and only
the guard's release resolves the union to idle.
Provider SYNCING is deliberately excluded — it is presentation state set
only by sync(), strictly narrower than the guard, and would be a fourth
lock disagreeing with the other three.
* feat(sync): add a config epoch for revalidating deferred sync work
Task 3 defers background sync work while other work is in flight, so a
request can outlive the target it was made against: a queued sync must not
run after the user switches provider, moves the folder, or signs out.
Add the monotonic in-tab counter that deferred work will capture and
revalidate before I/O. No behaviour change; nothing reads it yet.
Bumped on all four authoritative transitions: any provider-config write,
a target move asserted by a bypass ingress, an active provider switch, and
a credential revoke.
The last two are the reason this is a counter on the manager rather than a
projection of providerConfigChanged$: NEITHER emits on that stream. A
provider switch deliberately does not (switches are handled via
getLastSyncedProviderId -> forceFromSeq0) and clearAuthCredentials never
did, so an epoch derived from that stream alone would silently miss both —
including provider switch, which Task 3's acceptance criteria require to
invalidate a queued request. Specs pin both, asserting the switch case
bumps while providerConfigChanged$ stays silent.
Machine-only OAuth token refresh for an unchanged account still does NOT
bump: it goes through the credential store and moves no target, so
invalidating healthy queued work there would be a regression.
Follows the existing _activeProviderSetupId shape (in-tab, not persisted,
not cross-tab). Documented as a staleness heuristic, never a security
input — callers compare epochs, never configuration.
* feat(sync): add the background full-sync scheduler
Background triggers (interval, resume, visibility, settle) call sync()
behind an exhaustMap, which DROPS any trigger arriving while a sync runs —
the work it asked for is lost until something else happens to trigger
again. Add the single owner of generic pending background work: a burst
collapses into at most one pending rerun, which drains once the current
work settles. Dormant; nothing routes through it yet (that is the next
tranche, where the behaviour actually changes).
request() is fire-and-forget with no result and no failure taxonomy.
sync() resolves with the truthy string 'HANDLED_ERROR' on handled failure,
so its result cannot be truth-tested; the scheduler reads it not at all —
settled success and settled failure release identical state, and
source-specific retry stays with the source.
Deferral is the whole point, and a deferral is exactly the window in which
the user switches provider, moves the folder, or signs out. So a request
captures {configEpoch, providerId} and revalidates before EVERY leading or
trailing run, not only at request() time. A stale request is DROPPED, not
retargeted: the trigger that wanted target A has no opinion about target B,
and a live trigger will ask again.
It never calls sync() while anything else is active — that call would only
bounce off the cycle guard and return HANDLED_ERROR, silently burning the
request — and never before the awaited initial path opens the gate, so it
cannot start a shadow initial sync.
TWO wake-ups are required, and this is the subtle part. sync()'s finally
releases the busy signals BEFORE SyncEffects flips the gate in its .then().
A scheduler waking only on the busy edge therefore finds the gate still
shut, returns, and strands the request forever — the session's first
background sync would silently never run. Verified by removing the gate
wake-up: exactly that scenario fails.
Also exposes SyncTriggerService.initialSyncGateOpen$, the observable mirror
of isInitialSyncDoneSync() (setInitialSyncDone is the sole writer of both).
It deliberately excludes the MAX_WAIT_FOR_INITIAL_SYNC failsafe that the
existing public observables merge in: that timer keeps the UI from hanging
on a sync that never lands, which is right for rendering and wrong here —
it would open the gate on a schedule rather than on the initial sync having
actually completed.
* feat(sync): route background sync triggers through the scheduler
Split the dynamic background branch out of triggerSync$ into
scheduleBackgroundSync$, which calls scheduler.request() instead of
sync(). Interval, resume, visibility, idle/activity, online-regained and
the trailing settle timer now go through the scheduler; initial,
after-enable and before-close stay directly awaited, as do the ~14
explicit user/manual sync callers.
This is the behaviour change the previous three commits were seams for: a
background trigger arriving while a sync is in flight used to be DROPPED by
the shared exhaustMap, losing the work it asked for until something else
happened to trigger again. It is now deferred and drained.
exhaustMap's cross-source protection is not lost but upgraded. It stays on
triggerSync$, where it now guards only initial vs after-enable. Background
exclusion becomes the scheduler's busy check plus SyncCycleGuard.tryBegin()
— which was always the real authority; exhaustMap was a coarse
approximation that could only drop, never defer.
Kept deliberately unchanged so the split does not quietly alter behaviour
as a side effect: the 2s throttle (the scheduler collapses bursts anyway,
but this preserves the pre-existing rate ceiling), the E2E auto-sync kill
switch, and the offline skip.
Incidental correctness win: the ungated setInitialSyncDone(true) in the
catch used to mean ANY failing background sync — hours into a session —
flipped the initial gate. Only initial/after-enable reach that catch now,
so the gate is opened solely by the paths that own it.
Also drops SyncCycleGuard.released$, which had zero consumers: it was built
for Task 4 and is speculative until something needs it. isActive$ is what
the busy union actually consumes, and its edge tests move with it.
* test(sync): make SyncEffects testable and cover the trigger routing
The routing split was the whole behaviour change of this branch and had
NO test coverage: sync.effects.spec.ts is 61 lines testing constants, so
the split broke no test because no test was watching. Two things blocked
covering it, both fixed here.
1. createEffect stamps a non-configurable marker onto whatever object the
factory returns, and syncBeforeQuit$ returned the shared EMPTY singleton
when not on Electron. The first construction branded EMPTY process-wide,
so every later one died with "Cannot redefine property
__@ngrx/effects_create__" — the class could not be built twice in one
suite. This, not the Dropbox SDK the spec header blames, is why it never
had behavioural tests. defer(() => EMPTY) hands over a fresh instance per
construction; subscribed behaviour is unchanged.
2. isOnline$ is a module-level shareReplay(1) whose startWith(navigator.onLine)
captures its initial value at import time, and headless Chrome reports
navigator.onLine === false. Anything gated on it is permanently offline
under test with no seam to override, so an online-gated assertion would
pass while proving nothing. Add an IS_ONLINE$ token whose default factory
returns that same observable — no runtime change, just somewhere for a
spec to provide a fake.
The 8 new tests pin the acceptance criterion: a background trigger reaches
the scheduler and does NOT call sync(); the settle branch routes even though
it emits null rather than a trigger name (so routing cannot depend on the
emitted value); offline and the E2E kill switch suppress it; and the initial
sync still calls sync() directly, bypassing the scheduler, with only that
path flipping the initial gate.
* fix(sync): stop the scheduler running syncs back-to-back
Two real bugs, both found after the branch looked finished, neither by
reasoning: the first by wiring the real guard and busy service to the real
scheduler, the second by adversarial review. The full unit suite was green
throughout — it was never evidence for either.
1. Re-entrancy. The scheduler drained SYNCHRONOUSLY on the busy edge, and
that edge fires from SyncCycleGuard.end(), which sync() calls inside its
own finally. So the next sync started part-way through the previous
one's teardown, and the wrapper's SYNCING safeguard — which runs two
lines later — then saw the NEW sync's status and reset it to
UNKNOWN_OR_CHANGED. Provider status is still a live exclusion gate for
the immediate-upload side channel, so that is not cosmetic. Wake-ups now
drain on a microtask, letting the finishing cycle unwind first.
2. A permanent back-to-back sync loop. exhaustMap was the ONLY thing
bounding the sync RATE; throttleTime(2000) bounds the TRIGGER rate and
was never the binding constraint, so the previous commit's claim that
keeping it "preserves the pre-existing rate ceiling" was simply wrong.
I_INTERVAL_TIMER is self-sustaining and independent of sync activity and
is not clamped by SYNC_MIN_INTERVAL, so whenever a sync outlasts
syncInterval — a 90s WebDAV sync on a 60s interval is ordinary, since
getFileRev is a full GET rather than a cheap ETag — every tick lands
mid-sync and, once triggers are deferred instead of dropped, drains the
instant the previous run settles. Idle time goes from ~25% to zero,
permanently.
The wasted I/O is the lesser harm. sync() opens the hydration window and
closes it in its finally, so with no gap between runs isInSyncWindow is
effectively always true and skipDuringSyncWindow() would suppress
TODAY_TAG repair and day-change effects INDEFINITELY.
Add a duty-cycle floor: record the settle time and re-arm a timer rather
than run when less than SYNC_MIN_INTERVAL has passed. It lives in the
scheduler because only the scheduler can see run duration. The
defer-don't-drop win is unaffected — the trailing run still happens, just
not immediately.
Both fixes are pinned by specs that fail without them.
Also corrects initialSyncGateOpen$'s docblock, which claimed to exclude the
MAX_WAIT_FOR_INITIAL_SYNC failsafe. It does not: the 8s failsafe timers call
setInitialSyncDone(true) from a tap, writing the very subject the gate reads.
I had verified setInitialSyncDone was the sole writer of the subject and
never checked who calls it. The gate therefore means "the gate is open", not
"the initial sync landed" — harm is low since both paths call the same
sync(), but Task 4+ must not build a stronger invariant on it.
* fix(sync): make the duty-cycle floor monotonic and cross-source
Three more real bugs, all found by a second adversarial review of the
already-"finished" branch. None were found by reasoning about the code.
1. A backward wall-clock jump stalled ALL background sync. _lastSettleAt
used Date.now(), so an NTP correction after Android Doze / Electron
resume / a VM restore made the elapsed calculation negative and armed a
timer of `floor + jump` — a one-hour correction armed a one-hour timer,
and _armSpacingTimer early-returns while one exists, so nothing shortened
it. Background sync is now the only automatic sync path, so that is a
total stall. Clamping the delay would NOT have fixed it: each retry
recomputes the same negative elapsed and re-arms, stalling just as hard
in a loop. Use performance.now(), which cannot go backwards.
2. The floor only spaced the scheduler against ITSELF. _lastSettleAt was
written solely inside our own run, so initial, manual, after-enable, quit
and forceUpload syncs settled without touching it. Consequences: the
session's first background sync ran back-to-back with the initial sync
with zero gap — exactly the "blur right after initial sync" case the old
shared throttleTime guarded, which the effect split dissolved — and a
trigger deferred during the before-close sync started fresh I/O at the
instant the window closed, turning a rare crash-mid-sync into a routine
one. Stamp on any busy->idle transition, so the floor means "5s idle
after ANY sync work".
3. Fixing (2) naively introduced a third bug, caught by our own spec:
isBusy$ replays `false` to every subscriber, so filtering on the value
stamped _lastSettleAt at construction and delayed the session's first
background sync by the whole floor. pairwise() makes it a real
transition.
The specs drive performance.now() explicitly — jasmine's mockDate fakes
Date, not the monotonic clock, so the floor would otherwise read real time
and the coverage would be silently meaningless.
Known and deliberate: the floor narrows the skipDuringSyncWindow idle
window versus master (~25% -> ~5% in the slow-provider case), since
SYNC_MIN_INTERVAL is 5s. skipDuringSyncWindow is a drop-filter, so the idle
FRACTION is what matters to TODAY_TAG repair and day-change effects. Still
strictly better than the unbounded loop it replaces, but it is a tradeoff,
not a win, and is worth revisiting with a provider-aware floor.
|
||
|
Johannes Millan
|
6da578aa8d
|
fix(sync): defer LocalFile folder pick commit to settings Save (#9075) (#9085)
* fix(sync): defer LocalFile folder pick commit to settings Save (#9075) * test(sync): pin Android SAF pick route through form value (#9075) |
||
|
Johannes Millan
|
5a50297215
|
fix(sync): make no-pending concurrent crossings deterministic (#9073) (#9086)
A CONCURRENT remote op for a still-existing entity with no pending local ops was applied as-is, so two clients that had each already synced one side of a concurrent pair kept whichever op arrived last and silently diverged forever (arrival-order apply, no conflict, no LWW, no journal). Detection now reconstructs the local side from ALL retained ops still concurrent with the incoming clock (getEntityFrontierWithOps: same scan and rejectedAt filter as the frontier — using only the last op would let a newer disjoint edit mask an older overlapping one) and routes the crossing through the existing autoResolveConflictsLWW pipeline. Both clients therefore resolve the same unordered op pair identically (archive precedence, timestamps, clientId tie), and a local win emits the usual dominating LWW Update op that heals clients whose frontier could not see the crossing. Crossings where apply-both is already lossless and convergent keep today's behavior: identical content, disjoint real fields, noise-only sides, and concurrent task-time deltas. Multi-entity ops and post-compaction crossings also keep the status quo (documented residuals) since a rejection-free resolution cannot compensate atomic siblings. New invariant guard: an already-synced op is never markRejected (frontier scans skip rejected rows; rejecting a synced op would erase it locally while the rest of the fleet keeps it). A no-op for the pending path, whose rejection ids are unsynced by construction. Verified by a mirrored-delivery unit battery (fails 5/13 with the ladder disabled) and an integration spec against the real IndexedDB store with the production freeze flags: both delivery orders converge, the loser row is persisted as recorded-as-seen and rejected, and re-delivery after the heal is skipped without a second resolution. |
||
|
Johannes Millan
|
afce65dcd7
|
fix(sync): quiesce op capture before snapshot and compaction (#8469) (#9083)
* fix(sync): quiesce op capture before snapshot and compaction (#8469) NgRx state mutates synchronously at dispatch while an op's seq is only assigned later in the persist effect under the OPERATION_LOG lock. The snapshot save and threshold compaction captured state + lastSeq under a bare lock, so an op dispatched-but-unsequenced could have its reducer effect baked into the state cache while its seq landed after lastAppliedOpSeq - the next boot's tail replay then re-applied it, double-applying non-idempotent reducers (removeTimeSpent, logFocusSession, addNote, addBoard, addTaskAttachment, plain-append move/batch branches). Compaction made this a live-use window (every COMPACTION_THRESHOLD ops), not just a hydration edge. Both writers now run through flushThenRunExclusive: drain the capture pipeline before acquiring the non-reentrant lock, re-check the pending counter inside it. The snapshot body reads state first (synchronously at the quiesce cutoff) and lastSeq after - exact while the lock blocks seq assignment; compaction re-checks the counter AND the deferred-actions buffer (uncounted, kept across windows after a failed drain) in the same synchronous block as its state read and bails, retriggering on the next threshold. Emergency compaction keeps the bare lock: it runs inside the failing write's call stack during quota handling, where that write's pending entry is still elevated - flushing would self-wait until timeout and break quota recovery. Known trade-off: a wedged persist effect now surfaces as a flush timeout (skipped save / compaction failure counter) instead of a silently tagged-behind cache. * test(sync): isolate module-level deferred buffer across specs The #8469 capture bail reads the module-level deferred-actions buffer, which is shared across the whole karma bundle. The meta-reducer-ordering integration spec buffers persistent actions during its simulated sync window and never cleared them, so under jasmine's random spec order any later spec using the real compaction/snapshot services could see a dirty buffer and skip its capture - the CI-only failure in performance.integration.spec.ts ("Expected null not to be null"). Clear the buffer in the leaking spec's afterEach, and defensively in the beforeEach of every spec that exercises the real bail, so no other leaker can fail them order-dependently. |
||
|
Johannes Millan
|
d1ff7963d0
|
fix(sync): rebase repair op clocks on the durable clock (#8939) (#9080)
The REPAIR paths built their vector clock from the per-tab in-memory clock cache and then REPLACED the durable clock with it. A stale cache (another tab advanced the clock) regressed the durable clock, letting subsequent captures reuse counters already shipped — silently corrupting cross-device dominance comparisons. - createRepairOperation: route through appendMixedSourceBatchSkipDuplicates; the in-transaction rebase makes regression unrepresentable. State cache stores the clock actually written. - replaceRejectedRepair: rebase the replacement clock onto the durable clock inside its transaction (shared rebaseLocalClockOnDurable helper). - Drop client-side clock pruning from repair op building: inert under the rebase, and it dropped client IDs the server still tracks (false CONCURRENT). Server prunes after conflict detection. - Rename appendWithVectorClockUpdate -> appendWithVectorClockOverwrite and document the derivation invariant; the capture path (its only remaining production caller) already satisfies it. |
||
|
Johannes Millan
|
5fcb1cb2d7
|
fix(project): keep taskIds unique on replayed move-to-done (#8469) (#9076) | ||
|
Johannes Millan
|
470c700358
|
fix(schedule): keep the layout reference live instead of freezing at first render (#9064)
* fix(schedule): keep the layout reference live instead of freezing _contextNow decides the reference time the Schedule lays events out against. It is a computed that returns Date.now(), but nothing it reads advances with the clock — so the value it returns is cached until _selectedDate changes. In practice that means the layout reference freezes at whatever instant the view was first rendered. Leave the Schedule open from 09:00 to 17:00 and msLeftToday() still budgets 15h, and today's unscheduled tasks still lay out from 09:00 — above a now-line that has since marched down the grid. scheduleDays already refreshes every 2 minutes via scheduleRefreshTick, so the layout re-runs all day while being handed a stale reference. Read scheduleRefreshTick() in _contextNow, the same 2-min cadence currentTimeRow and scheduleDays already ride. No added change-detection work: scheduleDays recomputes on that tick regardless. This also fixes the midnight-rollover case. The view does not move when the clock does, so a day picked as "tomorrow" silently becomes today while _contextNow still reports its 00:00. Rather than comparing day strings to detect that, decide by where the wall clock sits inside the selected day. Comparing daysToShow()[0] against _todayDateStr() would mix two notions of "today": todayStr() applies the start-of-next-day offset, todayStr(date) does not (see date.service.ts). With a custom rollover the guard could then return a now outside day 0, and since create-schedule-days anchors dayDates[0] with `startTime = i == 0 ? now`, every day-0 entry would be pushed past its boundary and the first column would empty. Testing the clock's position in the day makes that structural: the reference can never land outside day 0. It also drops _todayDateStr and daysToShow from _contextNow, leaving _selectedDate as the single source. Tests: the rollover spec now moves only the refresh tick, as production does; a spec pins the reference staying live as time passes. The future-date spec asserted only that contextNow !== realNow, which passes for arbitrary wrong values — it now names both timestamps exactly. * test(schedule): pin that day 0 is never anchored outside itself The invariant the _contextNow rewrite rests on: contextNow anchors dayDates[0] via `startTime = i == 0 ? now`, so a now past that day's end would push every day-0 entry over its boundary and empty the column. It was guaranteed by construction but not by a spec. Reachable with a custom start-of-next-day, where the logical today is still Jan 20 while the wall clock already reads 02:00 on Jan 21. |
||
|
Johannes Millan
|
542da1eb6c
|
fix(sync): isolate file-provider state across target changes (#9063)
* fix(sync): invalidate file-provider target state on config change Task 2 (sync-simplification plan), core increment. The file adapter keys all per-target state (sync version, revs, vector clocks, seq cursor, within-cycle caches) by provider id only, and nothing cleared it on a configuration save. A provider switch, an account switch behind the same provider id, or an identity-affecting setting change would reuse the previous target's state against the new target — reading or writing one target's data against another. - Extract the delete-all reset into a shared _resetTargetState() over a single _targetScopedMaps source of truth; this also closes the one-field gap where deleteAllData never cleared _lastRecoveredCorruptRev. - Add invalidateAllTargets() (+ a target generation counter for the later in-flight guard) and call it from WrappedProviderService's existing providerConfigChanged$ subscription. Machine-only token refreshes go through the credential store (setComplete), not setProviderConfig, so they do not fire providerConfigChanged$ and correctly do not invalidate. Remaining Task 2 scope (follow-ups): Electron LocalFile picker + Android setupSaf ingresses that bypass providerConfigChanged$, and in-flight generation validation before each remote side effect (incl. the #9023 REPAIR rebase loop). * fix(sync): invalidate file target on LocalFile picker/SAF change Task 2 follow-up. The Electron LocalFile folder picker (persists main-side post-#8228) and Android setupSaf() (writes safFolderUri to the credential store) change the sync target without going through setProviderConfig(), so they never fired providerConfigChanged$ — leaving the file adapter's per-target revs/clocks/ caches (keyed only by the unchanged LocalFile provider id) pointed at the old folder. Route both ingresses through the existing providerConfigChanged$ signal via a module-level bridge, so WrappedProviderService clears its cache and calls invalidateAllTargets() exactly as it does for a config save. Remaining Task 2 scope: in-flight generation validation before each remote side effect (incl. the #9023 REPAIR rebase loop). * fix(sync): abort file upload when the target changes mid-operation Task 2 follow-up (in-flight guard). The file adapter keys per-target state by provider id, and the same provider object reads live config, so a target switch (provider/account/folder/identity-affecting setting) DURING an upload would let the in-flight write commit the previous target's merged data to the new target. Capture the target generation at the _uploadOps boundary and thread a write-guarded provider (Proxy over uploadFile/removeFile) through every write path — single-file, split, REPAIR snapshot, and backups. A generation bump (invalidateAllTargets) between capture and a write throws FileSyncTargetChangedError before the write; reads pass through. SyncWrapper maps it to UNKNOWN_OR_CHANGED (silent self-healing re-sync), like a concurrent-upload rev mismatch. Residual (documented): a check->write TOCTOU window remains (narrowed, not closed), and the REPAIR rebaseStaleRepair loop across RejectedOpsHandlerService/ RepairOperationService is not yet generation-threaded (follow-up #2b). Tests: adapter guard (mid-op abort writes nothing; removeFile guarded; reads pass), sync-wrapper mapping (silent UNKNOWN_OR_CHANGED). * fix(sync): extend in-flight target guard to snapshot uploads Completes the Task 2 in-flight guard. #2a guarded _uploadOps, but the file adapter has a SECOND remote-write entry point — uploadSnapshot/_uploadSnapshot (initial/recovery/migration + the #9023 REPAIR snapshot via _conditionalUploadRepairSnapshot and its backups) — which bypassed _uploadOps and was left unguarded. Apply the same generation-capture-at-boundary + write-guarded-provider shadow to _uploadSnapshot. The REPAIR rebase loop itself needs no guard: rebaseStaleRepair (RepairOperation Service) performs NO remote I/O — it rebuilds the repair op from local state (stateSnapshotService + opLogStore.replaceRejectedRepair) and defers the re-upload to the next sync cycle, which flows through the now-guarded write paths. deleteAllData's removeFile stays unguarded by design: it is a deliberate user-initiated wipe of a chosen target, not an in-flight-sync write race. Test: a target switch during the snapshot's archive-load phase aborts before the write with FileSyncTargetChangedError. * fix(sync): guard split-migration writes on the download path Review follow-up (two independent reviewers). The in-flight target guard covered the two upload entry points, but the split-format DOWNLOAD path also writes: when a remote carries a pending split-migration marker, _downloadOpsSplit -> _resumePendingSplitMigration force-writes the state file, tombstone/.bak, and migration marker via the RAW provider. A target switch during that resume could land the previous target's migration on the new one — the same corruption class the guard prevents. Apply the same generation-capture-at-boundary shadow to _downloadOps so the resume writes are guarded; reads (downloadFile/getFileRev) still pass through, so normal downloads are unaffected. Test (e2c): a target switch during a pending-migration download aborts the resume writes with FileSyncTargetChangedError. * docs(sync): correct Task 2 guard comments; make targetGeneration private Multi-review cleanup (no behavior change): - Fix the _targetGeneration doc comment (the in-flight guard it called a 'follow-up' shipped in the same work; it drives _withTargetGuard now). - Correct the _targetScopedMaps comment, which overstated the guarantee: a mid-download switch can repopulate a cache (read path) and a switch between download and upload is caught by neither per-operation guard. What actually prevents a cross-target write is the generation guard + the conditional-write rev check, which self-heals on the next sync — document that honestly. - Make the targetGeneration getter private (test-only surface; the guard reads the private field directly); specs use bracket access. * fix(sync): abort a download whose target changed before committing its baseline Multi-review (Codex) found a data-loss window the write-guard missed: a download READS target A, and if the target then switches mid-download, staging A's baseline (sync-version/clock/rev) and letting the caller advance the seq cursor under the shared provider id makes the NEXT sync skip the new target's ops from a stale cursor. The write-guard only covers writes; reads pass through. Capture the generation at the download boundary and, before committing the baseline (single-file and split paths), abort + reset the target-scoped state if it changed — SyncWrapper maps the error to a silent self-heal. Closes the dominant switch-during-download window; a switch after a successful download (during op-apply / an upload's own cursor commit) is a narrower residual only per-cycle session capture can fully close (documented). * fix(sync): map FileSyncTargetChangedError on the force-upload paths Multi-review (Codex): the error was only mapped in the normal sync() catch. The forceUpload and USE_LOCAL conflict-resolution catches handled only EncryptNoPasswordError, so a target switch during a force upload aborted safely (guard held — no data loss) but surfaced a scary ERROR snack instead of the silent UNKNOWN_OR_CHANGED self-heal. Add the branch to both force paths. * refactor(sync): compile-enforce the in-flight guard via a branded provider type Multi-review (Architecture) hardening: the guard was threaded by an implicit call-graph convention — a future entry point or write-helper caller that forgot the _withTargetGuard shadow would silently re-open cross-target writes, the worst failure class here. Introduce a phantom-branded GuardedFileSyncProvider (FileSyncProvider & { [unique symbol]: true }) returned by _withTargetGuard and required by every write-path helper param. Passing a raw provider to a write path is now a compile error (verified: TS2345). Only createAdapter and the intentionally-unguarded _deleteAllData keep the raw type. No runtime/behavior change (brand is a phantom type); adapter suite 135/135. * fix(sync): invalidate file-adapter state only on real target moves Task 2's invalidation was wired to "any privateCfg was saved", but its semantics are "the sync target moved". setProviderConfig() fires providerConfigChanged$ unconditionally, so two content-only writes reached invalidateAllTargets(): 1. The sync-settings dialog saves with isForce=true, which bypasses the JSON-equality dedup, and _updatePrivateConfig then rewrites privateCfg unconditionally. So changing the sync interval, toggling compression, or pressing Save with nothing changed wiped _localSeqCounters and persisted it. A cursor back at 0 makes the next download return a snapshotState (isForceFromZero); for a client holding unsynced ops that classifies CONCURRENT, and with AUTO_MERGE_CONCURRENT_SNAPSHOT false it dead-ends in a binary conflict dialog whose either answer discards data. file-based-sync-adapter.service.ts already documented this exact hazard at the latestSeq computation. 2. WrappedProviderService fires the GHSA-9544 isEncryptionEnabled backfill fire-and-forget on adapter creation. Its setProviderConfig() lands within ms (local IO) while the sync it was spawned from is still on the network, so it wiped the cursor mid-cycle and aborted that sync with FileSyncTargetChangedError even though nothing moved. (The abort does not heal the cursor: invalidateAllTargets clears and persists it before the abort throws, so the next cycle still bootstraps from 0.) providerConfigChanged$ now carries isTargetChanged, set from isSyncTargetChanged(). Every subscriber still drops config-derived caches (the adapter closes over the resolved encryption key/intent); only a real move additionally calls invalidateAllTargets(). Both facts ride one emission so a caller cannot raise a move without the cache drop. isSyncTargetChanged treats only encryptKey/isEncryptionEnabled as content-only and everything else as identity-affecting, so an unknown or newly added field errs toward invalidating rather than reusing one target's cursor against another. Both directions can lose data; the false negative is worse only because it is silent. The OneDrive pre-auth cfg write (dialog-sync-cfg) bypasses setProviderConfig, so by the time the save's setProviderConfig runs the diff is a no-op and a folder move would have silently kept the previous folder's cursor. It now asserts the move directly, gated on a real diff since it runs on every save. Covered by a test verified to fail without the fix. Also correct two comments: invalidateAllTargets' trigger, and the download-abort's mechanism (a stale cursor suppresses gap detection so the new target's snapshot is never loaded — it does not skip ops; both download paths return every op and dedup via appliedOpIds). Known residual: OneDrive's PROVIDER_FIELD_DEFAULTS are the only ones not seeded with '', so a config missing one still reports a spurious move on its first save (one-time, self-correcting). Documented at isUnset. |
||
|
Johannes Millan
|
71a9a4338a
|
fix(sync): freeze the conflict-review producers before the next release (#9061)
* fix(sync): freeze the conflict-review producers before the next release
The conflict-review feature (
|
||
|
Johannes Millan
|
011a92585d
|
docs(sync): add sync simplification roadmap (#9062)
* docs(sync): add simplification roadmap * docs(sync): strengthen simplification roadmap * docs(sync): simplify sync implementation plan Prioritize deletion, atomic single-tab ownership, and full-sync trigger consolidation while deferring compatibility removals and speculative abstractions. * docs(sync): make simplification roadmap compatibility-safe Narrow deletion work behind persisted-data, provider-eligibility, delivery, and request-cost gates. Keep schema readers and separate unrelated startup and maintenance correctness projects. * docs(sync): fold code-verified review findings into plan * docs(sync): align simplification plan with latest changes * docs(sync): correct trigger-branch claim and add footprint estimate * docs: cap services at 1000 LOC * chore(lint): warn when a service exceeds 1000 lines * chore(lint): raise service size cap to 1200 lines * docs(sync): re-align simplification plan to master |
||
|
Johannes Millan
|
104043e2d2
|
fix(locale): localize ISO 8601 recurring-task start date & weekday labels (#8987) (#9055)
* fix(locale): localize ISO 8601 recurring-task start date & weekday labels (#8987) Follow-up to #9013. The ISO 8601 option stores dateTimeLocale='sv' (the sentinel for YYYY-MM-DD + 24h). Components that format spelled-out weekday/ month names via toLocaleDateString(currentLocale(), ...) bypass the CustomDateAdapter fix, so they still rendered Swedish (e.g. the recurring- task dialog's Start-date value 'ons 15 juli 2026', and quick-setting labels 'Weekly on onsdag') regardless of the UI language. Add DateTimeFormatService.textLocale (= isoTextLocale() ?? currentLocale()) as the canonical locale for spelled-out names, and route those paths through it while keeping numeric dates and clock times on currentLocale() so ISO stays YYYY-MM-DD and the 24h clock is preserved. - plannedStartDateStr: spelled-out date -> textLocale, time -> currentLocale - buildRepeatQuickSettingOptions: new weekdayLocale param (numeric day/month stay on locale for ISO day-first ordering); dialog + add-task-bar pass it - dialog quick-setting memo key now tracks textLocale * fix(locale): extend ISO 8601 name localization to repeat-info & date chips Multi-review follow-ups on the #8987 fix: - getTaskRepeatInfoText (rendered in the work-view task list, task detail, scheduled list, tag chips, archived-task dialog): spelled-out weekday names now follow textLocale (derived from the already-passed DateTimeFormatService), while numeric day/month stay on locale. Previously leaked Swedish weekday names under the ISO option. - add-task-bar date/deadline chips: month:'short' name now uses textLocale so it isn't shown in Swedish. - plannedStartDateStr: hoist the duplicated toLocaleDateString options out of the if/else (DRY). - Harden DateTimeFormatService mocks in caller specs with textLocale. Adds regression tests for the weekday split and the date-chip localization. * refactor(locale): require weekdayLocale + correct the memo-key rationale Review follow-ups on the #8987 fix: - buildRepeatQuickSettingOptions: drop the `weekdayLocale = locale` default. Both callers already pass it, so the default was dead flexibility whose only effect would be to silently render a Swedish weekday for a future caller under the ISO option. Making it required forces the choice consciously. The spec that pinned the fallback now covers the real sv/sv case instead (an actual Swedish user should get 'onsdag'). - dialog-edit-task-repeat-cfg: the memo-key comment claimed the composite key was mere hygiene because the dialog's UI language can't change mid-life. That premise is wrong: applyLanguageFromState$ applies the UI language from remote sync too, so lng can change while the dialog is open, moving textLocale() with currentLocale() frozen at 'sv'. State the real reason so the key isn't removed later as redundant. Rename lastLocale -> lastLocaleKey. |
||
|
Johannes Millan
|
7ef7e69e96
|
fix(locale): localize remaining ISO 8601 spelled-out date names (#8987) (#9056)
Under the ISO 8601 option, dateTimeLocale is the 'sv' sync sentinel, so any spelled-out weekday/month name rendered with currentLocale() prints in Swedish regardless of UI language. Follow-up to #9013/#9055 covering the surfaces those PRs didn't: spelled-out names now follow the UI language (isoTextLocale), while numeric dates and clock times keep currentLocale(). - scheduled-list-page: locale signal -> isoTextLocale ?? currentLocale (all 6 localeDate labels are spelled-out; fixes the reproduced 'ons, 15 juli' leak) - dialog-focus-session-edit.formatSelectedDate: weekday+month long - habit-tracker.dateRangeLabel: month short - scheduled-date-group.pipe: work-view group-header tooltip weekday - worklog (formatDayStr via mapArchiveToWorklog): day-header weekday Inlines the isoTextLocale ?? currentLocale expression (not #9055's textLocale) so this PR shares no files with #9055 and is conflict-free / mergeable in any order. Adds an ISO regression test to scheduled-date-group.pipe.spec and updates mocks so the new isoTextLocale() calls resolve. Also adds docs/handover.md. |
||
|
Johannes Millan
|
6f88775ea2
|
fix(sync): authenticate LWW project-move footprint (#9053) (#9054)
SuperSync E2EE (AES-256-GCM) authenticates only op.payload; envelope fields including op.entityIds travel as plaintext. The two [TASK] LWW Update meta-reducers trusted meta.entityIds (copied from that envelope) to decide which tasks to relocate, so a compromised sync server could inject victim ids and relocate arbitrary tasks. GHSA-8pxh-mgc7-gp3g. Carry the deterministic move footprint (#9001) inside the encrypted, authenticated payload as LwwUpdatePayload.projectMoveFootprint: - createLWWUpdateOp writes the footprint to both op.entityIds (the server still needs plaintext ids for its indexed conflict detection and cannot read the ciphertext) and payload.projectMoveFootprint. - convertOpToAction surfaces the authenticated copy onto meta.moveFootprint (mirrors the recreatesEntityAfterDelete pattern). - Both LWW-TASK reducers (project repair + section membership) read meta.moveFootprint via a shared parseMoveFootprint helper and no longer trust meta.entityIds. Legacy ops without the field fall back to receiving-state repair. - getTaskProjectMoveEntityIds (footprint re-derivation during conflict resolution, fed remote ops) reads the authenticated payload instead of op.entityIds, so a tampered remote envelope cannot be laundered into a freshly-authenticated merged op. Covers the disjoint-merge, local-win, and superseded-op callers via one choke point. Additive optional field: forward/backward compatible, no crypto or envelope-version change. The DELETE/archive (bulk-archive-filter) and conflict-detection (get-op-entity-ids) envelope reads are suppression- only, not relocation, and remain for the durable AAD hardening. |
||
|
Johannes Millan
|
56ddeafd90
|
fix(tasks): self-heal orphan tasks to Inbox on navigation (#8780) (#9052)
* fix(tasks): self-heal orphan tasks to Inbox on navigation (#8780) Tapping a global-search result did nothing for a task with a falsy projectId (an empty-string projectId survives hydration because it passes typia validation), no tags, and not due today. Such a task's id is in no work context's `taskIds` ordering array, so it renders in no list. The previous fix routed these to the Today list, but the task is not shown there unless it is due/overdue today, so navigation still landed on the wrong view with nothing focused. Re-home the orphan into the Inbox via the existing moveToOtherProject op (which assigns projectId and adds the id to the Inbox list), then navigate there so the task actually renders and can be focused. The move reducer no-ops the source removal for an empty/dangling source and reads canonical subtask data from state, so replay is deterministic and concurrent heals converge (LWW projectId=INBOX + unique() taskIds). Also surface the existing error snack when a same-context task can never be focused, instead of silently leaving the user on the wrong view. * fix(tasks): suppress 'moved to project' snack on orphan self-heal (#8780) The #8780 self-heal dispatches moveToOtherProject to re-home an orphan task (falsy projectId, no tag) into the Inbox on navigation. That fired goToProjectSnack$ — a SUCCESS 'Moved task to project Inbox' snack — even though the user only tapped a search result to view the task and is being navigated to it anyway. Guard the snack on the moved task having a real source project, so the silent data-repair path stays silent while genuine user-initiated moves still announce. Tests: - task-ui.effects.spec: cover goToProjectSnack$ (orphan heal = no snack; real move = snack), previously untested. - layout.service.spec: cover the new onFailure retry-exhaustion branch. - navigate-to-task.service.spec: model the real orphan (empty-string projectId, no due date) with an accurate root-cause comment. * refactor(tasks): address multi-review findings for #8780 orphan heal Applied fixes from a three-way review (correctness, sync-correctness, tests): - Command/query separation: `_getLocation` renamed to `_resolveNavTarget` and made pure (returns {location, orphanToHeal}); `navigate()` now owns the synced moveToOtherProject dispatch, so it's an explicit navigation step, not a hidden side effect of computing a URL. Prevents a future display-only caller from silently syncing a move. - Scope the snack suppression to Inbox filing (`!task.projectId && targetProjectId === INBOX_PROJECT.id`) instead of any falsy-source move, so quick-add default-project assignment to a REAL project (short-syntax.effects) keeps its 'Moved to project' wayfinding snack. - Tests: add reducer coverage for the empty-string-projectId heal into Inbox (the exact repair the fix relies on, previously verified nowhere); add navigate cases for same-context heal and for an orphaned subtask whose parent can't load (routed, not moved); add effect case proving a non-Inbox falsy-source move still announces. Specs: navigate 7/7, task-ui.effects 14/14, project-shared.reducer 19/19, layout 13/13. checkFile clean on all touched files. * docs(tasks): note route-change focus delegates to AppComponent (#8780) |
||
|
Johannes Millan
|
6fefd741c5
|
fix(sync): sync review follow-up fixes (hydration data-loss + journal privacy) (#9045)
* fix(sync): authenticate the project-move footprint on encrypted LWW ops
SuperSync E2EE (AES-256-GCM) covers only `op.payload`; `op.entityId`/`entityIds`
travel as plaintext beside the ciphertext. The LWW project-repair reducer trusts
`meta.entityIds` (copied verbatim from that envelope) to move EVERY declared task
out of its project, so a compromised sync server could append victim task ids to
the envelope of an otherwise-valid encrypted move and orphan those tasks — without
touching the authenticated payload.
Bind the envelope footprint to the authenticated one: on the decrypt path, require
exact-set equality between `op.entityIds` and `{op.entityId} ∪
payload.projectMoveSubTaskIds`.
Interim hardening: enforced only when the authenticated payload carries a
`projectMoveSubTaskIds` array. Synthetic conflict-resolution LWW ops legitimately
carry `entityIds` with no such payload field (footprint lives only in the
envelope), so they are left untouched to avoid rejecting valid ops. Fully closing
the envelope-injection vector needs the durable fix (bind the footprint as GCM
AAD behind an envelope-versioned migration), which remains open. GHSA-8pxh-mgc7-gp3g.
* fix(sync): two data-loss fixes on the snapshot-hydration path
Both bugs surface when a remote snapshot replaces live state, and both live in
sync-hydration.service.ts — kept together as one change.
(1) Atomic rejection of superseded local ops. The file-based bootstrap rejected
pending local ops via a standalone markRejected() that commits in its own
transaction BEFORE commitFileSnapshotBaseline(). If that baseline commit then
threw (e.g. the op-log tail changed), the old state survived but the ops were
already durably rejected — permanently non-uploadable local edits. Fold the
rejection into the baseline transaction via a new `rejectOpIds` param so it is
atomic with the state replacement, and notify the user only after it commits.
(2) Flush the tracked-time accumulator before replacing NgRx. Time tracking
buffers a delta in memory for up to five minutes; a hydration in that window
replaced live state without flushing it, so the later flush dispatched a LOCAL
syncTimeSpent the reducer ignores — silently dropping the tracked time. Flush the
accumulator into a durable op before loadAllData (mirroring how clean-slate and
backup handle their own replacement paths), placed after the baseline commit so
the appended op can't move the tail past the asserted seq.
* test(sync): cover the #3 tracked-time flush-across-hydration path
Adds a deterministic scenario to the task-time replay state machine: a delta
tracked but not yet flushed, flush()ed before a snapshot replaces the store,
becomes a durable syncTimeSpent op that re-applies additively on replay onto the
delta-less remote baseline (and, without the flush, the baseline stays
under-counted). Together with the sync-hydration unit test (flush runs before
loadAllData) this covers the flush → durable-op → replay chain the #3 fix relies
on.
* fix(sync): fail-safe the conflict-journal clear on profile switch (privacy)
ConflictJournalService.clearAll() swallows IndexedDB clear() failures — by
contract, it must not throw after the dataset was already replaced during a
profile switch. But if the bulk clear failed, the next profile could still see
the previous dataset's task titles / field values in the conflict-review UI: a
cross-profile privacy leak.
Persist a durable "cleared before" timestamp in localStorage (which does not
depend on the journal's own IndexedDB being healthy) before the bulk clear. The
read paths — list(), getEntry(), and the badge count — hide every entry resolved
at or before that boundary, so a swallowed clear failure can never surface stale
content. The marker is dropped on a successful clear, and pruneOnStart physically
reclaims any hidden survivors and drops the marker on the next app start.
The boundary favors hiding (entries resolved at the exact clear instant are
treated as pre-clear), matching the privacy-fail-safe intent. Known edge-case
limitations (timestamp vs monotonic clock, localStorage durability, a future
localStorage.clear()) are documented on the marker constant.
Held from public issue tracking (privacy). 4 tests added; 31/31 green.
Hardened per a 3-agent review (correctness / privacy-completeness / lean).
* test(sync): round-trip the encrypted project-move footprint through real crypto (#2)
Exercises the #2 integrity gate through the actual encrypt→decrypt flow (real
AES-GCM), not just the isolated integrity function: a legitimate move round-trips
(entityIds ride OUTSIDE the ciphertext — no false positive), a server-tampered
entityIds footprint is rejected on both the single and batch decrypt paths, and a
synthetic envelope-only op (no authenticated projectMoveSubTaskIds) is accepted.
Closes the one gap flagged in the confidence review: proves the attack model and
the fail-closed behaviour in the real pipeline.
|
||
|
Johannes Millan
|
953d680d34
|
fix(sync): don't block on repair dialogs while holding sp_op_log (#9026) (#9049)
* fix(sync): don't block on repair dialogs while holding sp_op_log (#9026) Post-remote-apply validation failure ran the data-repair flow — a native confirm() before repair AND a native alert() after — from inside the sp_op_log lock. During unattended background sync the dialog sat open for ~10 min, freezing the event loop and starving every sp_op_log contender (snapshot compaction, write-flush, upload, backup) until the 30s LockAcquisitionTimeout, and widening the file-based recovery-snapshot clobber window (#9023). Make data-repair non-interactive by default (fail-safe), so no automatic or in-lock caller can block on a native dialog: - ValidateStateService.validateAndRepair gains `interactive` (default false): the confirm-before-repair and the repair-failed alert only run for interactive callers. - RepairOperationService.createRepairOperation / _notifyUser gain the same flag: the post-repair "data repaired" acknowledge alert (the common-case blocker the first cut missed) is skipped when non-interactive, and the record is logged non-blockingly instead of via devError (which itself pops a dialog in dev builds). - Since essentially every caller runs automatically and/or inside the lock, the safe behavior is the default and cannot be forgotten. Only the user-initiated, pre-lock USE_REMOTE recovery opts into interactive:true. Automatic in-lock callers (validateAfterSync / conflict-resolution via validateAndRepairCurrentState, snapshot hydration, server migration) all inherit the non-interactive default. The REPAIR op is built identically (dataRepair unchanged), so replay determinism is preserved; repair still re-validates and refuses to dispatch a still-invalid state. Failures stay surfaced via the session-validation latch (#7330) and error snack. Note: silent auto-repair no longer signals the user that data was modified — a gated non-blocking "data repaired" notification is a possible follow-up. * feat(sync): surface a non-blocking snack when background repair changes data (#9026) Making automatic repair non-interactive removed the only signal that the user's data was auto-modified. Restore awareness without reintroducing a blocking dialog: when a non-interactive repair actually changes something (totalFixes > 0), RepairOperationService._notifyUser opens a single non-blocking WARNING snack per session (reusing T.F.SYNC.D_DATA_REPAIRED), so a silent, cross-device-propagating data change isn't wholly invisible. The once-per-session latch mirrors the version-block snack guard so a repeat-repair loop can't spam it. Interactive (foreground) callers keep the existing blocking acknowledge alert. * test(sync): stitched proof the background-sync repair path is dialog-free (#9026) Adds an integration test that drives the full path with BOTH ValidateState- Service and RepairOperationService real — validateAndRepairCurrentState ('sync', { callerHoldsLock: true }) → real dataRepair → real createRepairOperation → real _notifyUser — and asserts it never reaches a blocking window.confirm/window.alert, while still creating the REPAIR op and surfacing the non-blocking "data repaired" snack. Closes the coverage seam the unit specs (which mock RepairOperationService) left open. |
||
|
Johannes Millan
|
f75346613d
|
fix(sync): write split-compaction snapshots to immutable files (#9040) (#9047)
* test(sync): reproduce file-based compaction stranded ops pointer (#9040) Concurrent split-file compactions can leave the committed sync-ops.json referencing a snapshot present in neither sync-state.json nor its .bak: the snapshot is force-written unconditionally while only the ops file is gated, so the loser of the ops race can clobber the winner's snapshot after backing up an older generation. Adds a failing integration test that drives two interleaved compactions against the stateful MockFileProvider and asserts a fresh client can still hydrate the committed generation. Fails today (unrecoverable gap); passes once the snapshot is written under a generation-unique, immutable name. * fix(sync): write split-compaction snapshots to immutable files (#9040) Concurrent split-file compactions could strand the winning ops pointer: the snapshot was force-written to the fixed sync-state.json, so the loser of the conditional ops-commit race could clobber the winner's snapshot, leaving the committed sync-ops.json referencing a snapshot absent from both sync-state.json and its .bak — an unrecoverable gap for a fresh client. Write the compaction snapshot to a generation+client-unique immutable file (sync-state__<syncVersion>__<clientId>.json) recorded in snapshotRef.file. A concurrent compactor writes a DIFFERENT file, so the winning pointer can never be clobbered. Readers prefer snapshotRef.file and fall back to sync-state.json / .bak (kept dual-written for pre-#9040 clients). The superseded snapshot is GC'd O(1) after each commit; a losing compactor's same-generation orphan is a rare, bounded leak (documented, with a listFiles-prune upgrade path). Also fixes MockFileProvider to model create-if-absent (If-None-Match: *) so the fresh-folder concurrent-compaction path is faithfully gated in tests. Tests: 3 concurrent-compaction integration scenarios (harmful/benign interleave, fresh-folder race) + 2 unit tests pinning the snapshot resolution order. Full op-log suite (3427) and sync-providers package (404) green. * fix(sync): reclaim orphaned snapshot when compaction loses the commit race (#9040) A concurrent compactor that writes its immutable snapshot but then loses the conditional ops-commit left that snapshot orphaned — no committed ops file ever referenced it, and the O(1) predecessor-GC never reclaimed it (same generation). The losing compactor now deletes the snapshot it just wrote when its commit throws, eliminating the leak at its only realistic source. Only a crash between the snapshot write and the commit/cleanup can still orphan a file (rare crash window; listFiles-prune remains the documented backstop). Refactors the GC helper into a single-file _removeGenStateFile primitive used by both the post-commit predecessor delete and the new failure-path cleanup. Tests assert the loser's orphan is gone after both the near-cap and fresh-folder concurrent-loss scenarios. Full op-log suite (3427) green. * fix(sync): only reclaim orphaned snapshot on confirmed rev-mismatch (#9040) The loser-orphan cleanup deleted the just-written immutable snapshot on ANY commit failure. A rev-mismatch is a confirmed server rejection (ops did not commit), but a network/5xx error is ambiguous — the ops PUT may have landed and committed, in which case the snapshot is still referenced and deleting it would strand a reader. Restrict the cleanup to UploadRevToMatchMismatchAPIError. Adds a test asserting the immutable snapshot survives a non-mismatch commit failure. Full op-log suite (3428) green. * fix(sync): use a random suffix, not clientId, in immutable snapshot names (#9040) Filenames are not encrypted, so embedding the clientId in sync-state__<syncVersion>__<clientId>.json leaked device count/platform and a per-device counter to the remote — a metadata downgrade for E2EE file-based users. Replace the clientId with an opaque random suffix, which still gives two concurrent compactors distinct files. A collision is astronomically unlikely and self-heals (the reader validates loaded content against snapshotRef, so a wrong file fails validation and falls back to sync-state.json/.bak). syncVersion stays in the name for legible ordering and a future listFiles-prune. Tests now assert on the count of immutable snapshot files (and a captured name) rather than hardcoding the now-random filenames. Full op-log suite (3428) green. |
||
|
Johannes Millan
|
e3581add2f
|
fix(sync): recover project notes/sections/repeat-cfgs on losing delete (#9048)
* fix(sync): recover project notes/sections/repeat-cfgs on losing delete When a legacy (pre-schema-v4) deleteProject loses an LWW race to a concurrent project UPDATE, the recovery path previously recreated only the project entity and its active/backlog tasks. Notes, sections, and task-repeat-cfgs stayed deleted, so every client converged to a lossy shape on hydration replay of the durable loser row — data loss against the winning "keep the project" intent (#9037). Recreate those three adapter entities at resolution time on the winner (which still holds them) via the generic lwwUpdateMetaReducer recreate path. Sections/repeat-cfgs are enumerated from the store (not carried in the delete payload) through the non-throwing selectEntities selectors; notes come from the delete payload's noteIds. Guard against resurrecting an entity another device is concurrently deleting, and strip dead task refs from recreated sections. Deferred (own snapshot design): time-tracking and menu-tree (singletons, no safe single-project restore) and archived tasks (separate persistence). Documented, converging limitations: section/repeat-cfg have no modified field so a concurrent content edit can be clobbered; a note's todayOrder slot is not restored. Tests: producer recreation + concurrency guard (conflict-resolution spec) and a SECTION recreate round-trip (lww-update meta-reducer spec). * test(sync): converge losing deleteProject cascade recovery (#9037) End-to-end convergence test through the real cascade reducers, op-log persistence, and conflict-resolution service: a losing deleteProject's cascade wipes the project's note, section, and repeat-cfg, and replaying the full durable log (delete + compensations) restores all three on both the restarted local client and the originating delete client. Closes the round-trip coverage gap left by the unit-level producer and meta-reducer specs. * test(sync): cover cross-batch deleteProject cascade recovery (#9037) Extract the losing-deleteProject convergence fixture into a shared helper and add a cross-batch case: the note/section/cfg recreations are replayed in a separate page from the durable loser and the project/task compensations. Proves the recreation ops carry no cross-batch dependency — they stay wiped after batch 1 (delete + project/task comps) and converge once the later batch arrives. |
||
|
Johannes Millan
|
0cd069fc74
|
fix(gitlab): stop connection test/search from firing thousands of requests (#9034) (#9051)
* fix(gitlab): avoid request storm on connection test and search (#9034) searchIssueInProject$ paginated through every matching issue and then fired a paginated /notes request per issue via forkJoin. With an empty search term (used by testConnection) or a broad one, a project with thousands of issues produced thousands of rapid requests, which GitLab rate-limited with a 429 and surfaced as "connection failed". The fetched comments were never used: search results only need id + title, the comment count already rides along via user_notes_count, and full notes are re-fetched fresh in the detail view via getById$. Fetch only the first page for search (results are already ordered by updated_at) and drop the per-issue comment fan-out. This turns the connection test and each search from thousands of requests into one. Add gitlab-api.service.spec.ts asserting search issues a single first-page request, never follows x-next-page, and never requests notes. * fix(gitlab): make project pattern valid as a native input attribute (#9034) Formly writes templateOptions.pattern verbatim to the native <input pattern> attribute, and Chromium 146 compiles that attribute with the RegExp `v` flag. The field passed a RegExp object, so its `/…/i` stringification became the attribute value, and the unescaped `/` and `-` in the `[\w.%/-]` character class are reserved under `v` — Chromium logged "Invalid regular expression: Invalid character in character class" on every change-detection cycle (hundreds of times during the request storm from the connection test). Pass the regex `.source` string instead, escape `/` and `-` in the class (`[\w.%\/\-]`), and make the pattern flag-independent by writing the only case-sensitive literal as `%2[Ff]` so dropping the `i` flag preserves the lowercase `%2f` separator. Matching behaviour is unchanged (verified against all existing valid/invalid cases). Angular's Validators.pattern still enforces the same rule via the source string. Add a regression test that the source compiles under the `v` flag. |
||
WikG
|
30139c7f88
|
fix(i18n): complete Simplified Chinese translations #5362 (#9036) | ||
|
Johannes Millan
|
cbd0641681
|
fix(sync): sanitize invalid LWW projectId to the current project (#9025) (#9041)
On the LWW conflict-apply path, an invalid projectId destination was handled inconsistently with the local handleUpdateTask strip. Unknown or non-string ids fell back to the task's current project, but an explicit null orphaned the task from every list (projectId = undefined). So a null destination replayed via a disjoint-merge patch diverged: a passive client kept the task in its project, while a conflict client orphaned it. Sanitize any invalid destination — null/undefined, non-string, or unknown project — to the task's current project, in every mode, mirroring the local strip. Tasks use '' for "no project", so a null is a malformed value to sanitize (not a signal to clear); this also matches how #9001 already handled unknown/non-string ids. '' remains a valid no-project assignment. The same synthetic LWW-TASK action is processed by a twin handler in section-shared.reducer, which had the same null bug and would otherwise strip the task from its current-project section while the task slice kept it. Both handlers now leave the task in its current project for any invalid destination. Tests: - lww-update / section-shared specs: null and unknown destinations retain the current project and section, in patch and replace modes; the "current project itself deleted -> undefined" fallback is covered. - New integration spec composes section -> crud -> lww in registry order and asserts the normal-replay and LWW-patch paths converge on the same projectId, project.taskIds and section membership. Verified to fail on the pre-fix code (LWW path orphaned the task from both project and section). |
||
|
Johannes Millan
|
0923fe66b2
|
fix(planner): don't erase day-scheduled subtasks on parent plan (#9019) (#9027)
* fix(planner): don't erase day-scheduled subtasks on parent plan (#9019) Planning a parent task ran removeTasksFromPlannerDays(task.subTaskIds), which stripped its subtasks from EVERY planner day. As a result, any day-scheduling the user had given a subtask was silently wiped the moment its parent was planned anywhere, so those subtasks disappeared from the planner and schedule (timed subtasks survived via a separate, plannerDays- independent path). handlePlanTaskForDay now collapses the parent's subtasks on the target day only, matching the target-day-only behaviour handleTransferTask already had. Subtasks scheduled for other days keep their scheduling and stay visible. Add removeTasksFromSinglePlannerDay helper (reuses removeTasksFromList) for this; the explicit "SinglePlannerDay" name keeps it from being confused with the all-days removeTasksFromPlannerDays (the mix-up that caused the bug). Same-day subtasks still fold into the parent by design: showing them as siblings would double-count their time (the parent estimate rolls up its subtasks) in the day total and on the schedule timeline. No schema barrier: the op payload and planner.days/dueDay shapes are unchanged, so this is a pure reducer behaviour fix, not a new payload semantic (op-log rules 2.5). In a mixed-version fleet a not-yet-upgraded client still applies the old all-days removal for these ops, but the task's dueDay is preserved either way and state re-converges once clients upgrade. * test(planner): e2e for day-scheduled subtask visibility (#9019) Drives the real UI: add a task + subtask, schedule the subtask for a day (day-only, via the 'S' shortcut + quick-access Tomorrow), plan the parent for a different day, then assert the subtask is still visible on its own day in the planner. Fails against the pre-fix all-days removal, passes with the target-day-only fix (both verified locally). |
||
|
Johannes Millan
|
a4dee9d5f7
|
fix(sync): isolate provider encryption settings + enforce critical e2e coverage (#9044)
* docs(plugins): add microsoft 365 calendar provider plan * docs(plugins): add microsoft 365 calendar provider plan * docs(ios): plan internal testflight builds * fix(sync): isolate provider encryption settings * test: enforce critical end-to-end coverage |
||
|
Johannes Millan
|
000d4726ac
|
fix(sync): guard file-based recovery snapshot vs concurrent ops (#9023) (#9043)
* fix(sync): guard file-based recovery snapshot vs concurrent ops (#9023) An automatic REPAIR recovery snapshot on a file-based provider (Dropbox/ WebDAV/local) force-overwrote the whole shared sync file unconditionally. If another device had uploaded a concurrent op this client had not yet merged, that op was silently dropped. SuperSync guards this via a server-side repairBaseServerSeq check; the file-based path had none. Add a rev-based guard: for a REPAIR (not BACKUP_IMPORT/initial/migration), write the primary sync file CONDITIONALLY on the rev this client last downloaded+applied (_lastSeenRevs). On a rev mismatch the adapter returns REPAIR_STALE, which routes into the existing provider-agnostic rebase path (RejectedOpsHandlerService -> rebaseStaleRepair): it downloads the concurrent ops, rebuilds the repair, and retries. That download promotes _lastSeenRevs, so the retry converges. A rev (not a vector clock) is used deliberately: two independently-pruned clocks (MAX_VECTOR_CLOCK_SIZE) can compare CONCURRENT even when one dominates, which would wedge the repair forever. The split path applies the same guard via a getFileRev pre-check (metadata only, so a torn remote does not block). _conditionalUploadRepairSnapshot writes primary-before- .bak so a lost race leaves the stale repair out of .bak. Tests: adapter unit specs for the guard (conditional-on-base-rev, stale -> REPAIR_STALE with .bak untouched, null-rev expect-absent, BACKUP_IMPORT still forces, split stale pre-check) and an end-to-end two-client convergence integration test (no clobber -> rebase -> converge, both clients' data preserved). * refactor(sync): centralize REPAIR_STALE error code (#9023) The file-based adapter (producer) and RejectedOpsHandlerService (consumer, routes it to a rebase) both matched the bare literal 'REPAIR_STALE', so a rename on one side could silently break the rebase without any test failing. Extract REPAIR_STALE_ERROR_CODE so the client-internal producer/consumer contract is a single shared symbol. Behavior-identical; the value still mirrors the server's SYNC_ERROR_CODES.REPAIR_STALE (the wire contract), pinned by the existing handler spec (feeds the literal) and adapter specs. |
||
|
Johannes Millan
|
18262eb1f3
|
fix(sync): retention pruning, misc→tasks alias boundary & WS local-win re-upload (#9028)
* fix(supersync): exclude legacy REPAIR from retention cleanup pruning
The daily-cleanup fallback (used when `latestFullStateSeq` is absent, i.e.
legacy/pre-marker installs) selected the pruning boundary with a raw
`opType: { in: [SYNC_IMPORT, BACKUP_IMPORT, REPAIR] }` filter that includes
legacy REPAIR rows (`repairBaseServerSeq` NULL). Such a repair carries no
causal base cursor proving its state is current as of its seq, so pruning
history behind it can permanently drop ops committed between its logical
base and its seq for any device replaying from before it.
#8971 migrated five full-state queries to `CAUSAL_FULL_STATE_OPERATION_WHERE`
but missed this one — the single query whose result directly authorizes a
DELETE. Route it through the same causal-only predicate; a legacy-repair-only
user then yields `protectedFromSeq === null` and is skipped (no deletion),
the safe default already implemented below.
Adds a regression test whose mock `findFirst` honours the causal
`repairBaseServerSeq` predicate.
* fix(supersync): gate misc→tasks conflict alias on the split boundary
`detectConflictForEntity` and `prefetchLatestEntityOpsForBatch` looked up
the legacy `GLOBAL_CONFIG:misc` alias for an incoming `tasks` write with
`schemaVersion < CURRENT_SCHEMA_VERSION`. Once v4 shipped, that aliases
post-split v2/v3 misc writes — disjoint from `tasks` — and fabricates false
`CONFLICT_CONCURRENT` rejections of tasks-settings writes for multi-device
users.
Gate both read-side lookups on the fixed `MISC_TASKS_SPLIT_SCHEMA_VERSION`
(2), matching the `isLegacyMiscConfigOperation` incoming gate and the
warning comment it already carries. Drops the now-unused
`CURRENT_SCHEMA_VERSION` import.
Adds real-PostgreSQL integration coverage for both changed lookups
(`detectConflictForEntity` and the batch `prefetchLatestEntityOpsForBatch`):
a post-split v2/v3 misc write must not alias, a legacy v1 one still must.
* fix(sync): re-upload LWW local-win ops after a WS-triggered download
A WS-triggered download that resolves a conflict against pending local ops
appends LWW local-win replacement ops straight to the op-log, bypassing the
capture effect. Unlike `ImmediateUploadService` and the main sync loop, this
path had no follow-up upload, so the preserved edit sat unsynced until the
next user edit or periodic sync — an unbounded window for manual-sync-only
users.
Re-upload when `localWinOpsCreated > 0`, mirroring the other two paths, and
surface the same terminal outcomes `ImmediateUploadService` does — permanent
rejection / rejected-full-state baseline → ERROR, mandatory-but-missing key →
UNKNOWN_OR_CHANGED — so a preserved edit that fails to converge is not left
silent (the WS path never claims IN_SYNC). Single follow-up only, matching the
sibling side channel.
* fix(supersync): validate the primary latestFullStateSeq marker as causal before pruning
The scheduled retention cleanup trusted `state.latestFullStateSeq` as a pruning
boundary whenever it was `<= lastSnapshotSeq`, with no check that the marked op is
a causal full-state operation. The earlier fix (
|
||
|
Johannes Millan
|
9132ab6722
|
fix(sync-core): break whole-entity LWW timestamp ties by clientId (#9035)
* fix(sync-core): break whole-entity LWW timestamp ties by clientId An exact-millisecond timestamp tie on the same field of one entity fell to remote-wins with no tiebreak. Because local/remote are swapped between devices, each kept the other's value and diverged permanently. Fall back to a deterministic larger-clientId compare on the tie (mirrors noiseTiebreakSide), routing a local tie-win through the existing localWinOperationKind: 'update' path so the compensating op preserves the local value and dominates the loser's clock. * refactor(sync-core): tidy LWW tiebreak helper + strengthen tie tests Multi-review follow-up (no behavior change): - drop unreachable `?? ops[0]` fallback and the unused generic in winningClientId; correct the doc comment's "mirrors" overstatement. - rewrite the mislabeled service-level tie test that never exercised the tiebreak and add the local-clientId-larger direction (compensating op). * test(sync): cover LWW client-ID tie-win over a concurrent remote DELETE The tiebreak reaches _createLocalWinUpdateOp's delete-recreation branch via a tie (not just a newer timestamp); assert the entity is still recreated. |
||
|
Johannes Millan
|
463709e2de
|
fix(electron): assert renderer IPC boundary at window creation (#9018)
* docs(plugins): add microsoft 365 calendar provider plan * docs(plugins): add microsoft 365 calendar provider plan * docs(ios): plan internal testflight builds * fix(electron): assert renderer IPC boundary at window creation Every IPC trust boundary (Jira one-shot capability, plugin node-exec consent, the window.ea preload bridge) rests on the renderer main world not having require/ipcRenderer, which is guaranteed solely by contextIsolation: true + nodeIntegration: false and sub-frames not getting node integration. If that webPreferences ever silently regressed (a refactor spreading a shared options object, a bad merge), every gate would collapse at once while still looking correct in review. Add web-preferences-guard.ts (assertSecureWebPreferences) and fail closed before creating a window if the boundary is not intact. It rejects a non-true contextIsolation, a non-false nodeIntegration, and (fail-closed) a nodeIntegrationInSubFrames that is not explicitly false; it also directionally rejects an explicit sandbox: false, nodeIntegrationInWorker: true, and webviewTag: true (each off by default, so no call site is forced to set it). Wire it at all three new BrowserWindow sites (main window, task widget, full-screen blocker); the full-screen blocker previously relied on Electron defaults, so set its webPreferences explicitly. A *.test.cjs backs it with behavioral coverage plus a wiring guard that counts constructor sites vs guard calls per file, so a future window cannot silently ship without the check. Closes #9015 * fix(electron): extend webPreferences guard to webSecurity Follow-up hardening from the multi-agent review of #9018: - Reject an explicit `webSecurity: false` (directional, like the sandbox /worker/webviewTag trio). With the app's blanket Access-Control-Allow-Origin: *, disabling the same-origin policy in a node-bridged renderer would widen cross-origin reach — and no call site currently guards against it. - Broaden the wiring-guard test to also require the assert for `new BrowserView` / `new WebContentsView`, closing the tripwire's blind spot for future non-BrowserWindow renderers (none exist today). - Correct the fail() comment: the `throw` narrows the type regardless of return-vs-throw; fail() returns an Error only to DRY the message. 230/230 electron tests pass; checkFile + prettier clean. |
||
|
Johannes Millan
|
f665536ef3
|
test(sync): cover project delete-wins conflicts (#9021) | ||
Parman Mohammadalizadeh
|
7fba6f55e2
|
fix(tasks): decode multipart and transfer-encoded eml bodies #8975 (#8999)
* fix(tasks): decode multipart and transfer-encoded eml bodies #8975 Dropping a real-world .eml (e.g. saved from Outlook) onto the add-task button created a title-only task: the parser only accepted a single, unencoded text/plain body, but most clients send multipart/alternative (plain + HTML) with quoted-printable or base64 transfer encoding on the plain part. eml-parser.ts now walks multipart/* structures (bounded recursion depth) for the first supported text/plain leaf, and decodes quoted-printable/ base64 transfer encodings on it. HTML bodies and non-UTF-8/ASCII charsets are still never decoded, matching the existing threat model for this untrusted, inert note content. * fix(tasks): keep unsupported-encoding eml test accurate after decode support The service-level test for "unsupported body encoding" used a base64 fixture, which the eml-parser fix now legitimately decodes. Switch it to a genuinely unsupported transfer-encoding token, and add a dedicated test asserting base64 bodies decode and wrap in the notes code fence as expected. * fix(tasks): skip Content-Disposition: attachment parts in eml import _extractPlainText() picked the first supported text/plain leaf in wire order regardless of Content-Disposition, so a text/plain attachment could be imported as the note when the real body was HTML, or shadow the real body if it appeared first in the multipart structure. Skip any part (leaf or multipart container) marked as an attachment before inspecting it further. Addresses review feedback from @johannesjo on PR #8999. * fix(tasks): treat non-inline disposition and legacy name= as attachment Content-Disposition is optional (legacy mail marks a filename via Content-Type's name= parameter instead), and RFC 2183 §2.8 requires any disposition type other than inline — recognized or not — to be treated as attachment. The previous fix only matched the literal token "attachment", so a text/plain part identified solely by a legacy name= parameter, or one with an unrecognized disposition type (e.g. x-download), still shadowed the real body. _isAttachmentPart() now treats any present disposition other than inline as an attachment, and falls back to the Content-Type name= hint only when Content-Disposition is absent entirely. Addresses further review feedback from @johannesjo on PR #8999. * fix(tasks): recognize RFC 2231 name*/name*0 attachment filename hints The legacy Content-Type name= fallback (used when Content-Disposition is absent) only matched the literal key "name", missing RFC 2231's encoded (name*) and continuation (name*0, name*0*, name*1, ...) spellings of the same parameter. A part identified solely by a continued name*0/name*1 filename hint slipped through undetected and could be imported as the note instead of producing a title-only task. _parseContentType() now matches any RFC 2231 spelling of name via a presence-only regex; the value is never decoded or reassembled since only the filename hint's existence matters. Addresses further review feedback from @johannesjo on PR #8999. * fix(tasks): recognize RFC 2231 filename*/filename*0 disposition hints Content-Disposition's hasFilename fallback (used when the disposition type token fails to parse, e.g. a type-less "; filename=...") only matched the literal key "filename=", missing RFC 2231's encoded (filename*) and continuation (filename*0, filename*0*, ...) spellings of the same parameter -- the mirror of the name*/name*0 gap fixed for Content-Type. _parseContentDisposition() now matches any RFC 2231 spelling of filename via a presence-only regex, symmetric to _NAME_PARAM_KEY_RE. Addresses further review feedback from @johannesjo on PR #8999. |
||
|
Johannes Millan
|
7e273a0e5c
|
fix(sync): recover tasks when a deleteProject loses an LWW conflict (#8997) (#9007)
* docs(plugins): add microsoft 365 calendar provider plan * docs(plugins): add microsoft 365 calendar provider plan * docs(ios): plan internal testflight builds * fix(sync): recreate cascaded tasks when a deleteProject loses LWW (#8997) When a remote deleteProject loses an LWW conflict, its reducer cascade (tasks removed via removeMany(allTaskIds)) is not compensated, so the project resurfaces empty and its tasks are lost on every client that applied the delete and on this client's own hydration replay. Mirror the TASK-parent recovery from #8990 for PROJECT cascades: _createTaskRecreationOpsForWinningProject emits recreate-after-delete TASK snapshots for every still-present task in the delete payload's allTaskIds, plus relationship/membership patch ops so the exact regular/backlog lists and subtask links are restored after the entities exist. enrichDeleteProjectAction expands a stale delete through current project relationships on replay; the lww meta-reducer validates recreate rows against present parents/projects. Covers part (a) of #8997. Parts (b) local-delete-loses and (c) notes/archived-tasks remain open. * fix(sync): make deleteProject-loser recovery interruption-safe (#8997) Hardens the base #8997 recovery for cases where a recreate-after-delete row is itself caught in a later conflict or delivered out of order. - createTaskRecreationFollowUpOps re-emits parent/subtask relationships and PROJECT membership when a recovery row is rejected and replaced, so independent server acceptance cannot drop parent/child links or append a backlog task to the regular list. - _createRemoteWinCompensationForRejectedTaskRecreation reconstructs a local snapshot when a remote TASK winner (move-to-project or a field-safe update) beats a local recovery row, then restores its dependents; opaque/relationship-changing remote winners bail out. - The superseded-op resolver re-emits the same follow-ups and persists each replacement group atomically before retiring the stale rows. - Preserve the recreate guard when a recovery row wins a later conflict. Load-bearing, not gold-plating: with this layer stubbed, 8 unit tests plus the move-winner half of the persistence convergence test fail. * fix(sync): don't resurrect concurrently-deleted tasks in project recovery (#8997) Adversarial review of the #8997 recovery found two cross-device residuals in _createTaskRecreationOpsForWinningProject: 1. (HIGH) The recovery reads task presence from the pre-batch store, so it was blind to a delete piggybacked as a non-conflicting op in the same sync batch. Device C deletes task t2 while device A wins a project rename vs B's deleteProject; A recreated t2, resurrecting it (via a borrowed newer timestamp) on every client that applied C's delete while A's own delete won locally — a silent split-brain. _collectDeletedTaskIds now gathers the batch's deleted TASK ids (single + multi-entity) and recovery skips them. 2. (MEDIUM) Recreations borrowed the project's timestamp as their LWW proxy, which is unrelated to task content and could clobber a CONCURRENT edit on another device. They now use each task's own `modified` (fallback: the project timestamp). Clock domination over the delete is unaffected. Both are proven by new failing-first specs. Note: the sibling subtask path (_createSubtaskRecreationOpsForWinningParent, #8956) shares the same same-batch-delete blindness and is a candidate follow-up. * fix(sync): keep bulk-deleted tasks deleted in project recovery (#8997) The deleteProject-loser recovery skips tasks a concurrent non-conflicting op is deleting in the same batch, so it doesn't resurrect them on clients that applied the delete. Its `_collectDeletedTaskIds` guard only read `op.entityId`, but a bulk `deleteTasks` (TASK_SHARED_DELETE_MULTIPLE) op carries every id in `entityIds` and mirrors only the first to `entityId`, with an empty `entityChanges`. So every id after the first was missed and recreated with a borrowed newer timestamp — the same split-brain the single-delete guard closes, via the bulk path. Union both id sources via `getOpEntityIds` (already used throughout this file for the identical reason). Adds a failing-first regression test mirroring the single-delete case with an entityIds-carrying bulk delete. * fix(sync): exclude conflict-won deletes from project recovery (#8997) The deleteProject-loser recovery excludes tasks a non-conflicting op is deleting in the same batch, but a task can also be deleted by its OWN LWW conflict — this client held a competing edit that lost, and the remote delete won. Such a delete is applied this batch yet never reaches `nonConflictingOps`, so recovery read the task from the pre-batch store and re-emitted a recreation for a deletion that had just won. Fold the remote-delete winners of resolved conflicts into the same `_collectDeletedTaskIds` guard so recovery skips them too. Reusing that helper means bulk deleteTasks winners are covered as well. Failing-first regression test added. * test(sync): add replay-convergence tests for project-recovery delete guards (#8997) The concurrent-delete guards were covered only by emission assertions (was the recovery row emitted/skipped) — which cannot catch a resurrection that only manifests after replay. These two real-store integration tests apply the recovery ops on a fresh client that also applied the concurrent delete and assert the deleted task stays deleted: - bulk deleteTasks: every trailing entityIds entry stays deleted - a task whose own LWW conflict the remote delete won stays deleted Both fail (task resurrected) when the respective guard is reverted, confirming they exercise the divergence, not just op emission. The second also disproves the "borrowed modified timestamp self-corrects" theory: a recreation dominates the delete by clock/seq, so the task is resurrected without the guard. |
||
|
Johannes Millan
|
29df7e7719
|
fix(sync): preserve local edits during snapshot hydration (#9010)
* docs(plugins): add microsoft 365 calendar provider plan * docs(plugins): add microsoft 365 calendar provider plan * docs(ios): plan internal testflight builds * fix(sync): prevent file-based sync data loss Preserve post-snapshot operations and stage remote baselines until durable apply. Use conditional writes and a resumable marker for provider concurrency and legacy migration. Report trustworthy remote timestamps and document transport limits. Refs #8960 * fix(sync): avoid biased conflict recommendations Highlight a side only when its timestamp or known change count is strictly greater, leaving unknown and tied metadata neutral. * fix(sync): preserve local edits during snapshot hydration Extracts the one reachable-data-loss fix from the #9005 hardening branch (feat/sync-recovery-hardening). On the default single-file path, a local action dispatched while an async snapshot hydrate is in flight was persisted against the old state and then clobbered by loadAllData (or permanently dropped by markRejected) — a silent edit loss multi-device users can hit on gap-detected re-sync. Runs hydration inside writeFlushService.flushThenRunExclusive so capture stays in deferred mode across the snapshot dispatch, commits the snapshot baseline atomically (commitFileSnapshotBaseline), then replays the buffered local intents and their archive side effects onto the new baseline. Deliberately excludes #9005's snapshot two-phase-commit, structural validation, split-migration crash-resume, and gap persistence: reachability analysis found those defend non-reachable (structural validation), already-fail-safe (crash-mid-write), or default-off (split sync) failure modes, at the cost of ~1k lines and two permanent on-disk formats. * test(sync): add commitFileSnapshotBaseline to hydration spy |
||
|
Johannes Millan
|
631e6e9710
|
fix(locale): localize ISO 8601 calendar weekday/month names (#8987) (#9013)
* docs(plugins): add microsoft 365 calendar provider plan * docs(plugins): add microsoft 365 calendar provider plan * docs(ios): plan internal testflight builds * fix(locale): localize ISO calendar weekday/month names (#8987) PR #8991 localized ISO 8601 weekday labels only in the custom Schedule/Habits/Planner components. Material <mat-calendar> (schedule-task dialog, deadline dialog, date-picker inputs, repeat-task heatmap) still rendered month + weekday names via the global adapter locale 'sv' (the ISO sentinel), so they showed Swedish and ignored the app language. Override getDayOfWeekNames, getMonthNames and the spelled-out branch of format() in CustomDateAdapter to run under a temporary swap to isoTextLocale() (the UI language) when the ISO option is active. Numeric dateInput and time-only formats keep the configured locale, so ISO stays YYYY-MM-DD and the 24h clock is preserved. The swap assigns this.locale directly (not setLocale) so it fires no spurious localeChanges. Add unit coverage for the adapter and a real <mat-calendar> integration spec asserting UI-language headers, a live language switch, and the non-ISO fallback. |