fix(sync): clear example tasks from local state on adoption (#7996)

#7995 rejects a never-synced client's onboarding example-task UPLOAD ops
when it adopts a populated remote, but on the SuperSync (op-based) path
adoption MERGES remote ops into the store rather than replacing it, so the
example *entities* lingered in local NgRx state. Left there, a later
full-state snapshot upload (forceUploadLocalState / enable-encryption) would
re-snapshot and re-upload them, re-polluting the remote — `isExampleTask`
lives only on the op, never the Task entity, so the snapshot cannot tell
them apart.

In the same gated #7995 cleanup block, after rejecting the create ops, also
dispatch the shared bulk `deleteTasks` (scrubbing project.taskIds / backlog /
tag ordering refs in one meta-reducer pass) with `meta.isRemote: true`. That
makes the capture meta-reducer skip op-capture — so NO Delete op is created
and nothing uploads a delete for a task the server never had — and keeps the
LOCAL_ACTIONS effects (plugin TASK_DELETE, reminder cascade) from firing for
throwaway scaffolding. Same primitive ValidateStateService and the op-replay
path use; the state reducers still run. Ids derive only from the local
pending example-create ops, never remote ops.

Tests: +3 unit cases (operation-log-sync.service.spec) asserting the
isRemote dispatch fires only on the gated condition and rejects only the
create ops; a docker-only e2e asserting clean local state + no cross-device
propagation.
This commit is contained in:
Johannes Millan 2026-06-03 20:56:48 +02:00
parent 3e58b6d8d2
commit 56fda20fa0
5 changed files with 616 additions and 27 deletions

View file

@ -0,0 +1,394 @@
# SuperSync: remove lingering onboarding example tasks from local state on first adoption
**Issue:** #7996 (follow-up to #7995#7985#7976/#7980)
**Date:** 2026-06-03
**Status:** **Implemented (2026-06-03)** on `feat/issue-7996-3b222c` after #7995 merged to
`master` (PR #7995, commit `3e58b6d8d2`) and this branch was fast-forwarded. **Final mechanic
is the `meta.isRemote` dispatch — see §9, which supersedes the A1 "generate-then-reject" design
in §3** (a second multi-review found the simpler primitive). Unit
(`operation-log-sync.service.spec.ts`, +3) specs green; the op-log integration spec
(`example-task-adoption-cleanup.integration.spec.ts`) keeps its #7995 cases (the #7996 half is
a pure NgRx dispatch with no op-log effect); e2e (`#7996` test added to
`supersync-example-task-fresh-client.spec.ts`) authored — **docker-only, run in a real shell**
(reproduce-first: local-store-clean assertion fails pre-fix). Decision was (b) sequence after
#7995.
**Area:** op-log sync (`src/app/op-log/sync/operation-log-sync.service.ts`)
---
## 0. Hard prerequisite — this builds on #7995, which is NOT yet merged
#7996 extends the **`uploadPendingOps` identity-based cleanup hook added by PR #7995**
(branch `feat/issue-7985-7f39b1`, still **OPEN**). That hook does not exist on `master`
nor on this worktree's base. What master/this branch already has (from #7976/#7980) is the
_older_ `_discardExampleTaskOps` call on the **piggyback** path (`operation-log-sync.service.ts:284`)
and the `isNeverSynced` threading — **not** the new pre-upload cleanup block.
The #7995 hook we extend sits just after the `isWhollyFreshClient` guard in `uploadPendingOps`
(reproduced below; note #7995 imports `isExampleTaskCreateOp` from
`op-log/validation/is-example-task-op.util`, it is not inline):
```ts
if (isNeverSyncedAtSyncStart && (await this.opLogStore.hasSyncedOps())) {
const pendingOps = await this.opLogStore.getUnsynced();
const isPristinePostBootBatch = pendingOps.every(
(entry) => isExampleTaskCreateOp(entry) || entry.op.entityType === 'GLOBAL_CONFIG',
);
if (isPristinePostBootBatch) {
const exampleOpIds = pendingOps.filter(isExampleTaskCreateOp).map((e) => e.op.id);
if (exampleOpIds.length > 0) {
await this._discardExampleTaskOps(exampleOpIds);
OpLog.normal(/* … discarded N untouched example-task op(s) … */);
}
}
}
```
**Sequencing decision required (pick one before coding):**
- **(a) Land #7996 on top of `feat/issue-7985-7f39b1`** and merge it together / right after #7995.
- **(b) Wait for #7995 to merge to `master`, then rebase this branch** and implement.
There is a **third option raised in review and worth serious consideration:**
- **(c) Fold #7996 INTO #7995.** #7995 rejects the example _create ops_; #7996 removes the
_entities those ops created_. They are two halves of one invariant — _a never-synced adopter
ends up with neither the example ops nor the example entities_. Shipping #7995 alone merges a
**knowingly-incomplete fix with a live re-pollution vector** (the lingering entities re-upload
on the next snapshot). Folding also dissolves this entire §0 sequencing/duplicate-guard problem
and lets one integration spec assert the complete end state.
**Trade-off:** (b)/(c) keep the diff clean and the fix complete; (c) may delay #7995 if it is
already close to merge. This is a **decision for the maintainer** (see "Decision needed" at the
end). Whichever path: **do not** re-implement the #7995 hook standalone on this branch — that
creates a conflicting duplicate. Also note `feat/issue-7985-7f39b1` carries unrelated
onboarding/dialog changes, so option (a) "branch on top of it" inherits that noise. This plan
assumes the #7995 hook is present.
---
## 1. Problem (one paragraph)
#7995 stops a never-synced client from **uploading** its 4 onboarding example-task create ops
onto a populated remote it just adopted. But on the **SuperSync (op-based)** path, adoption
_merges_ remote ops into the existing NgRx store (`RemoteOpsProcessingService.processRemoteOps`),
it does **not** replace it. So the example **tasks themselves stay in local NgRx state** — only
their op-log create ops were rejected. Two harms:
1. **Cosmetic:** the just-onboarded device shows 4 example tasks the other devices don't have.
2. **Re-pollution vector (the real reason):** the examples live in NgRx but not on the server.
A later **full-state snapshot upload**`forceUploadLocalState` (USE_LOCAL conflict choice)
or enabling encryption (`SYNC_IMPORT` with `isCleanSlate: true`) — snapshots current NgRx
(incl. the lingering examples) and re-uploads them, re-polluting the remote.
The file-based path (Dropbox/WebDAV) does not have this residual because snapshot hydration
_replaces_ NgRx state wholesale.
**Why local-state removal is the only viable layer (the decisive architectural fact).**
`isExampleTask` lives **only on the op-log create op, never on the NgRx `Task` entity**
(`example-tasks.service.ts:96-102` sets it on the `addTask` action; the resulting entity is
byte-identical to a real task). So by the time `forceUploadLocalState` builds its snapshot via
`getStateSnapshotAsync()` (`server-migration.service.ts:174-178`), example and real tasks are
**indistinguishable** — you cannot filter them out at the snapshot layer without re-deriving
identity from the (already-rejected) op-log create ops, which is strictly more coupling than
just deleting the entities. This rules out every "fix it at the upload/snapshot layer" shortcut
and is the real load-bearing reason the fix must touch local NgRx state. Re-pollution — not the
cosmetic clutter — is the harm that justifies the change.
## 2. Goal / acceptance criteria (from the issue)
- After a never-synced SuperSync client adopts a populated remote, the onboarding example tasks
are **absent from local NgRx state**, with no dangling list references (`project.taskIds`,
`backlogTaskIds`, tag `taskIds`).
- No example task — and no delete-of-example op — reaches the server.
- A subsequent `forceUploadLocalState` / enable-encryption from that device does **not**
re-introduce example tasks to the remote.
- Real or edited data is untouched; the gate still skips when any non-config op is pending.
---
## 3. Chosen approach — **Option A1: surgical delete via meta-reducer, reject the generated Delete op**
Extend the **same** #7995 guarded block (`isNeverSyncedAtSyncStart && hasSyncedOps()` + pristine
post-boot batch + `exampleOpIds.length > 0`). After rejecting the create ops, also:
1. Derive the example **task ids** from the _same pending example-create ops_ (their `entityId`,
never from remote ops — `isExampleTask` lives only on the local op).
2. Dispatch `TaskSharedActions.deleteTasks({ taskIds })` — a single bulk action that the
**task-shared delete meta-reducer** (`handleDeleteTasks`, `task-shared-crud.reducer.ts:377`)
handles in one reducer pass: removes the tasks (+ any subtasks) and scrubs every
`project.taskIds` / `backlogTaskIds` / tag `taskIds` reference. (Complies with sync rule #3:
one multi-entity change = one meta-reducer pass = one op, **not** an effect fan-out.)
3. `await this.writeFlushService.flushPendingWrites()` so the captured Delete op is persisted.
4. Reject that Delete op too (so we never upload a delete for a task the server never had).
**Primary strategy — snapshot-and-diff:** capture the set of unsynced op ids _before_ the
dispatch, then after the flush reject only the _new_ `Delete` op ids. This is robust by
construction (it does not rely on "only ours can be present") — and since the create ops are
already rejected before the snapshot, they are excluded from both sides of the diff, so the
sole new unsynced `TASK`/`Delete` op is the cleanup one.
**Load-bearing safety invariant (do not omit from code comments).** The genuine hazard here is
**not** flush latency — it is capture _deferral_. The capture meta-reducer _buffers_ (does not
enqueue) any persistent action while `isApplyingRemoteOps` is true
(`operation-capture.meta-reducer.ts:226-233`), and `flushPendingWrites()` drains only the
_enqueued_ queue, never the deferred buffer. If this dispatch ran inside the apply window, the
Delete op would be silently deferred and never appear in `getUnsynced()`. It is safe **only
because** the download/merge phase fully completes — `replayOperationBatch` calls
`endApplyingRemoteOps()` + awaits `processDeferredActions()` in its `finally` — _before_
`uploadPendingOps` runs (`sync-wrapper.service.ts` download@~467 → upload@~493). State this
explicitly in the code comment so a future reordering that runs this hook inside the apply
window is caught in review, and assert it in the integration spec.
### Sketch (inside the existing `if (exampleOpIds.length > 0) { … }`)
```ts
await this._discardExampleTaskOps(exampleOpIds); // #7995 — reject the create ops
// #7996: also remove the untouched example tasks from local NgRx state, so op-based
// adoption matches the file-based replace semantics. Ids come ONLY from the local pending
// example-create ops (never remote). Without this the examples linger in NgRx and a later
// forceUploadLocalState / enable-encryption snapshot would re-pollute the remote.
const exampleTaskIds = pendingOps
.filter(isExampleTaskCreateOp)
.map((e) => e.op.entityId)
.filter((id): id is string => !!id);
if (exampleTaskIds.length > 0) {
const unsyncedBefore = new Set(
(await this.opLogStore.getUnsynced()).map((e) => e.op.id),
);
this.store.dispatch(TaskSharedActions.deleteTasks({ taskIds: exampleTaskIds }));
await this.writeFlushService.flushPendingWrites();
const cleanupDeleteOpIds = (await this.opLogStore.getUnsynced())
.filter((e) => !unsyncedBefore.has(e.op.id) && e.op.opType === OpType.Delete)
.map((e) => e.op.id);
await this._discardExampleTaskOps(cleanupDeleteOpIds);
}
OpLog.normal(/* … cleaned N example task(s) from local state (#7996) … */);
```
New imports in the service: `TaskSharedActions` (`root-store/meta/task-shared.actions`) and
`OpType` (`../core/operation.types`). `Store` and `writeFlushService` are already injected.
### Why A1 over the alternatives
| Approach | Mechanic | Why not |
| ------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **A1 (chosen)** | delete via meta-reducer, then reject the captured Delete op | self-contained; reuses existing `_discardExampleTaskOps`/`flushPendingWrites`/`getUnsynced`; matches the issue text; worst case is one harmless extra rejected op |
| **A2: "suppress" capture** | wrap the dispatch in `HydrationStateService.start/endApplyingRemoteOps()` | **Does not work as imagined.** Review found that flag _defers_ (buffers → reprocesses via `processDeferredActions()`), it does **not** suppress — so the Delete op is still created, just later and at an unpredictable time. There is **no clean "dispatch-without-capture" primitive** in the codebase today; adding one is scope creep. Combined with the process-global flag's blast radius (a missing `finally` → permanent silent capture loss), A2 is strictly worse than A1. Reject. |
| **B: replace-not-merge on adoption** | for a never-synced client adopting a populated remote, `loadAllData(default)` then apply remote ops (mirror `forceDownloadRemoteState`) | conceptually cleanest ("never-synced adoption replaces, not merges") but rewrites the core adoption path, must reason about concurrent local ops + sync rule #7, and risks data loss for a standalone user who built real offline tasks before connecting. Out of scope here; revisit only if merge-vs-replace is worth unifying. |
---
## 4. Safety / invariants
- **Gate is unchanged** — fires _only_ on `isNeverSyncedAtSyncStart` (pre-download capture) +
`hasSyncedOps()` true now (download adopted remote ops this cycle) + **pristine post-boot
batch** (only example creates + `GLOBAL_CONFIG` writes). Any real work — edit, reorder
(`Move`), real task, real project — adds a non-config op, fails the pristine check, and
skips cleanup entirely. An **edited example still syncs as real data** (#7995 property,
preserved untouched).
- **Ids derived from local pending example-create ops only** — never from remote ops. A
remote-supplied `isExampleTask` flag can never reach this path (it isn't read here).
- **No dangling references**`handleDeleteTasks` is the proper meta-reducer; it scrubs
`project.taskIds`, `backlogTaskIds`, and all tag `taskIds`, and follows `subTaskIds`
(example tasks are flat top-level INBOX tasks, but this is defensive).
- **Real data untouched**`handleDeleteTasks` no-ops on ids absent from state, and we only
pass the 4 example ids. Non-example tasks are never in `exampleTaskIds`.
- **No example delete reaches the server** — the captured bulk Delete op is rejected via
`markRejected`, which sets `rejectedAt`; `getUnsynced()` filters `!rejectedAt`, so the upload
that follows (line ~205) never sends it.
- **File-based path safe** — if the hook ever fires for a file-based client whose examples were
already replaced by snapshot hydration, the `deleteTasks` dispatch is a no-op on absent ids
and the Delete op (if captured) is rejected. No harm.
- **Sync rules:** #1 (no `Actions` in effects — N/A, this is orchestration not an effect),
#3 (multi-entity change via meta-reducer ✓), #6 (single dispatch, not a 50+ loop —
`flushPendingWrites` already serializes; review confirmed no extra `setTimeout(0)` yield is
needed), #9 (log only ids/counts, never task titles).
- **Effects that observe `deleteTasks` (all `LOCAL_ACTIONS`, all `{ dispatch: false }` — verified
no op fan-out, rule #1 holds):** `issue-two-way-sync` (reads a sidecar that the bare action
leaves empty → no-op), `time-block-sync`, `task-reminder` (Android native-reminder cancel),
and `plugin-hooks` which **fires `PluginHooks.TASK_DELETE`** for the example ids. The plugin
hook is the one real observable side effect — benign (plugins also saw the adds), but the plan
does **not** claim "nothing reacts": confirm no installed onboarding plugin reacts destructively
to `TASK_DELETE`.
- **Vector clock:** rejecting both the create and the cleanup Delete leaves the local clock
advanced without uploading either — identical to the already-accepted behavior of the existing
`_discardExampleTaskOps` (which rejects creates). Benign: the local clock legitimately dominates
and the server never references these entities; no `setVectorClock` reset is needed (unlike
`forceDownloadRemoteState`). The integration spec should still assert a subsequent real op
uploads cleanly after cleanup (entity-frontier sanity).
---
## 5. Open questions / must-verify before/while coding
1. **The existing #7976 e2e already asserts examples are gone after SuperSync adoption**
(`supersync-example-task-fresh-client.spec.ts:104-106`), yet #7996's premise is that op-based
merge _leaves them present_. SuperSync e2e is docker-only and **never runs in CI**, so that
assertion is **unverified and likely cannot pass until #7996 lands**. → Treat #7996 as the
change that _makes the existing assertion true_, and confirm by running that spec on docker
**before** the change (expect FAIL at 104-106) and after (expect PASS). If it already passes
pre-change, the premise is wrong and the whole approach must be re-examined.
2. **Capture timing — the real risk is deferral, not flush latency** (see the load-bearing
invariant in §3). The dispatch must run with `isApplyingRemoteOps === false`, else the Delete
op is buffered and `flushPendingWrites()` won't surface it. This holds because download
completes before upload, but it is implicit — pin it in the integration spec (assert the
Delete op is in `getUnsynced()` _after_ `flushPendingWrites()` resolves) and guard against a
future reordering.
3. **Bulk op shape.** Confirm `deleteTasks` produces exactly one `Delete`/`TASK`/`isBulk` op
(not N single ops). If it fans out to N ops, the "reject new Delete ops" diff still catches
all of them — but the assertion in the unit test must match the real count.
4. **Will the example tasks actually be in NgRx at hook time?** On the adoption path the
download (merge) ran before `uploadPendingOps`, and merge does not remove local tasks, so
yes. Confirm in the integration/e2e spec rather than by argument.
---
## 6. Test plan
> **Dependency note:** the "extend #7995's added cases" framing only holds if #7995 has landed
> (§0). On the _current_ branch these unit cases and the integration spec below **do not exist**
> (the spec's current example-task tests cover the _incoming-download_ path, not the upload
> pristine-batch hook) — they would be authored from scratch. State the #7995 dependency inline
> for whoever implements.
### Unit — `operation-log-sync.service.spec.ts` (extends #7995's added cases)
- **Fires (happy path):** never-synced + `hasSyncedOps()` true + pristine example+config batch →
asserts (a) `store.dispatch` called with `TaskSharedActions.deleteTasks({ taskIds: <4 example ids> })`,
(b) `markRejected` called for both the create ops **and** the cleanup Delete op,
(c) the example task ids come from the local create ops' `entityId`,
(d) **the `GLOBAL_CONFIG` op is NOT rejected** (it uploads normally — mirror the existing
empty-case assertion that the config op survives).
- **Scoping proof:** with a hypothetical pre-existing unsynced non-example op present in
`unsyncedBefore`, assert the cleanup does **not** reject it — proving the snapshot-and-diff
actually scopes the rejection to the new Delete op.
- **Does NOT fire** when: already-synced at start; nothing adopted (`hasSyncedOps()` still false);
a real (non-example) task op present; a reorder `Move` present; an edited-example op present
(each must skip the dispatch entirely). These mirror the #7995 truth-table — extend them to
also assert **no** `deleteTasks` dispatch.
- **Op-count:** `deleteTasks` is a single bulk op (`isBulk`), so expect exactly **1** new
`Delete`/`TASK` op, not N.
- Mock note: `provideMockStore`'s `dispatch` does not run the real meta-reducer/capture, so the
spec mocks `opLogStore.hasSyncedOps()`, adds a `writeFlushService.flushPendingWrites` spy, and a
two-stage `getUnsynced()` (before-dispatch / after-dispatch returns the new Delete op). The
capture→flush→reject chain is only _truly_ exercised by the integration spec below.
### Integration — `op-log/testing/integration/example-task-adoption-cleanup.integration.spec.ts`
#7995 added this against the **real** `OperationLogStoreService` to prove the pre-download-capture
vs live-read sequencing. Extend it to assert the **end-to-end op-log result** after the full
hook: example create ops rejected **and** the cleanup Delete op rejected, with the config op left
unsynced (uploads normally). Specifically pin:
- the Delete op is present in `getUnsynced()` **after `flushPendingWrites()` resolves** (the
load-bearing sequencing from §3 / §5.2), then is rejected;
- the captured delete is exactly **one** bulk op (§5.3);
- **a subsequent real op still uploads** after cleanup (entity-frontier sanity — guards the
vector-clock-residue concern in §4).
This is where the capture-deferral/flush and bulk-op-shape questions get pinned against
production code (the unit spec cannot, per the mock note).
### e2e — `e2e/tests/sync/supersync-example-task-fresh-client.spec.ts` (docker; real shell only)
**Test 1 — adoption leaves a clean local store + no propagation.** Never-synced client adopts a
populated **non-encrypted** account:
1. Seeder seeds via normal upload (no SYNC_IMPORT) — the exact #7980 residual case (verified:
the seeder is itself fresh, so `ServerMigrationService` bails the migration and the remote is
populated by plain Create ops, no full-state op).
2. Fresh client (example tasks on) adopts silently (`waitForInitialSync:false`, expect `complete`).
3. **Local store clean:** none of the 4 example titles present (this is the existing 104-106
assertion — currently _fails_ pre-#7996 because merge keeps the examples; #7996 makes it pass).
4. **Server clean (no propagation):** spin up a **3rd observer client** (`createSimulatedClient`
- `setupSuperSync` + sync) that re-downloads and asserts none of the 4 example titles appear —
this is the established harness pattern (cf. `supersync-late-join` / `import-other-client-ops`);
there is no server-state-inspection API, so the observer is the only way to assert this.
**Test 2 (separate `test()`) — no re-pollution via snapshot upload.** This guards the _actual_
harm (§1.2), so it must not be a tail step of Test 1 (a flake in adoption must not mask a
re-pollution regression):
1. Reach the adopted state (as Test 1).
2. Trigger a full-state snapshot upload from the fresh client — `forceUploadLocalState`
(USE_LOCAL conflict choice) **or** enable encryption (`SYNC_IMPORT`, `isCleanSlate`).
3. Re-download from a fresh observer client → assert **still no example tasks**.
> Cannot run docker SuperSync e2e in the agent sandbox (memory:
> `reference_supersync_e2e_docker_sandbox`). Author the spec, run it in a real shell, and
> verify reproduce-first (FAIL pre-change at the local-store-clean assertion, PASS after).
> Also update the file-level docblock: the file currently documents itself as a #7976
> _conflict-dialog_ regression; the new tests assert _local-state cleanup_ — keep the two
> purposes distinct so a future maintainer doesn't read 104-106 as a #7976 guarantee.
### Checks
`npm run checkFile <path>` on every touched `.ts`; `npm run test:file
src/app/op-log/sync/operation-log-sync.service.spec.ts` and the integration spec.
---
## 7. Risks & confidence
- **Confidence in approach: ~85%** (raised after multi-review confirmed the core facts). Verified:
the #7995 hook shape, the delete meta-reducer scrubbing refs + no-op-on-absent-ids
(`task-shared-crud.reducer.ts:377`), `deleteTasks` being a single bulk op with `OpType.Delete`
(`task-shared.actions.ts:82`), `getUnsynced()` excluding rejected ops, the capture
enqueue→flush write path _and_ the deferral hazard it must avoid, the `isExampleTask`-only-on-op
fact (rules out snapshot-layer fixes), example tasks being flat top-level INBOX tasks, the
five `deleteTasks` listeners (all `dispatch:false`, only the plugin hook observable), and the
§5.1 merge-keeps-examples premise (confirmed by the #7995 commit message itself).
- **Unsure / risks:**
- The #7995-dependency sequencing/fold decision (§0) — the single biggest practical blocker;
a process decision, not a code risk. **Maintainer's call (see below).**
- The e2e premise (§5.1) reads as verified statically, but the docker-only e2e cannot run in
CI; settle it empirically (reproduce-first) before trusting the test direction.
- The capture-deferral invariant (§3) is the subtle correctness lynchpin — safe today, but
must be commented + asserted so a future reorder can't silently lose the delete.
- A third-party onboarding plugin reacting to `TASK_DELETE` (§4) — confirm none does.
- **Out of scope:** Option B's merge-vs-replace unification; any change to the gate condition
itself (inherited unchanged from #7995); encrypted-account behavior (already handled by the
incoming-`SYNC_IMPORT` discard path).
---
## 8. Decision needed (maintainer)
**Sequencing/structure of the change relative to PR #7995.** Pick one before implementation:
- **(c) Fold #7996 into #7995** — one complete fix ("adopter keeps neither example ops nor
entities"); avoids merging a half-fix with a live re-pollution vector; dissolves §0. May delay
#7995.
- **(b) Land #7995, then implement #7996 as a rebased follow-up** — keeps #7995 reviewable;
accepts a short window where the re-pollution residual exists on merged master.
- **(a) Branch #7996 on top of `feat/issue-7985-7f39b1` now** — fastest to start, but inherits
that branch's unrelated changes and re-resolves #7995 review churn.
Review leaned (c) on correctness grounds, (b) on reviewability. Either (b) or (c) is sound; (a)
is the least clean.
**Decision (2026-06-03): (b).** #7995 merged to `master` (`3e58b6d8d2`); this branch was
fast-forwarded and #7996 implemented on top as a clean increment.
---
## 9. Implementation note — final mechanic differs from A1 above (post-multi-review)
The shipped code does **not** use the "generate a Delete op then reject it" dance described
in §3. A second multi-review surfaced that the codebase already has the right primitive: a
`meta.isRemote` action is skipped by the capture meta-reducer
(`operation-capture.meta-reducer.ts:222`, checked _before_ the `isApplyingRemoteOps` branch)
and filtered out of `LOCAL_ACTIONS` (`local-actions.token.ts:51`) — the same primitive
`ValidateStateService` and the op-replay path use. So `_removeAdoptedExampleTasksFromState`
simply dispatches `TaskSharedActions.deleteTasks({ taskIds })` with `meta.isRemote: true`:
- the state reducers still run (scrubbing `project.taskIds`/backlog/tag refs), but
- **no Delete op is ever captured** → nothing to reject, no `flushPendingWrites`, no
snapshot-diff, and the §3 capture-deferral invariant becomes irrelevant; and
- the `plugin TASK_DELETE` / reminder `LOCAL_ACTIONS` effects don't fire — closing the §4
open question outright.
This is strictly simpler and lower-risk than A1, so it superseded it. The §3 prose is kept
for the design rationale (why touch local state at all), but the mechanic is the `isRemote`
dispatch.

View file

@ -10,27 +10,32 @@ import {
} from '../../utils/supersync-helpers';
/**
* Regression test for #7976 a fresh client's first-run example tasks must not trip the
* SYNC_IMPORT conflict gate when pulling an account that already has data.
* Two distinct guarantees for onboarding example tasks vs an incoming populated account:
*
* THE BUG: ExampleTasksService creates onboarding task-create ops on first run. When a
* fresh client then syncs an account that already has remote data, those pending ops used
* to make the conflict gate treat the client as having meaningful local work, so it showed
* a `dialog-sync-import-conflict` instead of silently accepting the import.
* TEST 1 (#7976) the SYNC_IMPORT conflict GATE. ExampleTasksService creates onboarding
* task-create ops on first run. When a fresh client then syncs an account that already has
* remote data, those pending ops used to make the conflict gate treat the client as having
* meaningful local work, so it showed a `dialog-sync-import-conflict` instead of silently
* accepting the import. This test guards the op-log `isExampleTask` marker + gate exclusion
* (it does NOT exercise the afterInitialSyncDoneStrict$ timing on a fresh e2e client sync
* is disabled at boot, so example tasks are created before sync is configured regardless;
* the marker is what suppresses the dialog here). Method: `waitForInitialSync: false` so
* setup does NOT auto-resolve the dialog, then race "dialog visible" vs "sync complete".
*
* WHAT THIS GUARDS: the op-log `isExampleTask` marker + gate exclusion. It does NOT
* exercise the afterInitialSyncDoneStrict$ timing on a fresh e2e client sync is disabled
* at boot, so example tasks are created before sync is configured regardless of the strict
* gate; the marker is what suppresses the dialog here. Covering the strict-wait timing
* needs a client with sync pre-enabled at boot (a separate test).
* TEST 2 (#7996) LOCAL-STATE cleanup + NO cross-device propagation. On the SuperSync
* (op-based) path, adoption MERGES remote ops into the store rather than replacing it, so
* #7995 rejecting the example UPLOAD ops still left the example tasks lingering in the
* adopting device's local NgRx state (where a later snapshot upload would re-pollute the
* remote). #7996 also removes them from local state on adoption. This test seeds a
* NON-encrypted account by normal upload (the #7980 residual case no SYNC_IMPORT fires
* the incoming-import gate's discard), then asserts the fresh client's local store is clean
* AND that a third observer device adopting the same account never sees the example tasks
* (they never reached the server).
*
* METHOD: configure sync with `waitForInitialSync: false` so setupSuperSync does NOT
* auto-resolve the conflict dialog (it otherwise clicks "Use Server Data"), then race
* "conflict dialog visible" against "sync complete". The fix means sync completes WITHOUT
* the dialog; pre-fix the dialog wins the race.
*
* REPRODUCE-FIRST: expected to FAIL on `bb4b625645^` (race resolves to 'dialog') and PASS
* on the PR head. Run on both before trusting it.
* REPRODUCE-FIRST: Test 1 fails on `bb4b625645^` (race 'dialog'); Test 2's local-store
* assertion fails pre-#7996 (op-based merge keeps the examples). Run on both before trusting.
* Docker SuperSync is required and is NOT runnable in the CI/agent sandbox run in a real
* shell.
*
* Run: npm run e2e:supersync:file e2e/tests/sync/supersync-example-task-fresh-client.spec.ts -- --retries=0
*/
@ -113,4 +118,80 @@ test.describe('@supersync Fresh-client example tasks vs incoming import (#7976)'
}
}
});
test('adoption removes example tasks from local state and they never propagate (#7996)', async ({
browser,
baseURL,
testRunId,
}) => {
test.slow();
const appUrl = baseURL || 'http://localhost:4242';
const uniqueId = Date.now();
let seeder: SimulatedE2EClient | null = null;
let freshClient: SimulatedE2EClient | null = null;
let observer: SimulatedE2EClient | null = null;
try {
const user = await createTestUser(testRunId);
// NON-encrypted account: the first device seeds via normal upload (no SYNC_IMPORT),
// so the incoming-import gate's discard never fires — the exact #7980 residual case.
const syncConfig = { ...getSuperSyncConfig(user), isEncryptionEnabled: false };
// Seed the account with one real task (example tasks suppressed for the seeder).
seeder = await createSimulatedClient(browser, appUrl, 'Seeder', testRunId);
await seeder.sync.setupSuperSync(syncConfig);
const realTask = `Real-Task-${uniqueId}`;
await seeder.workView.addTask(realTask);
await seeder.sync.syncAndWait();
// Fresh client WITH onboarding example tasks (created at boot, before sync config)
// adopts the populated remote silently (no conflict dialog → #7976), then syncs so
// any pending ops are pushed.
freshClient = await createSimulatedClient(browser, appUrl, 'Fresh', testRunId, {
allowExampleTasks: true,
});
await freshClient.sync.setupSuperSync(syncConfig);
await freshClient.sync.syncAndWait();
// #7996: the example tasks are removed from the fresh client's LOCAL state on adoption.
await freshClient.page.goto('/#/project/INBOX_PROJECT/tasks', {
waitUntil: 'domcontentloaded',
timeout: 30000,
});
await freshClient.page.waitForLoadState('networkidle');
await waitForTask(freshClient.page, realTask);
const freshTitles = await getTaskTitles(freshClient);
for (const exampleTitle of EXAMPLE_TASK_TITLES) {
expect(freshTitles).not.toContain(exampleTitle);
}
// Observer device (no example tasks of its own) adopts the same account. Because the
// example tasks (and their delete) never reached the server, they never propagate here.
observer = await createSimulatedClient(browser, appUrl, 'Observer', testRunId);
await observer.sync.setupSuperSync(syncConfig);
await observer.sync.syncAndWait();
await observer.page.goto('/#/project/INBOX_PROJECT/tasks', {
waitUntil: 'domcontentloaded',
timeout: 30000,
});
await observer.page.waitForLoadState('networkidle');
await waitForTask(observer.page, realTask);
const observerTitles = await getTaskTitles(observer);
for (const exampleTitle of EXAMPLE_TASK_TITLES) {
expect(observerTitles).not.toContain(exampleTitle);
}
} finally {
if (observer) {
await closeClient(observer);
}
if (freshClient) {
await closeClient(freshClient);
}
if (seeder) {
await closeClient(seeder);
}
}
});
});

View file

@ -20,7 +20,8 @@ import { RemoteOpsProcessingService } from './remote-ops-processing.service';
import { RejectedOpsHandlerService } from './rejected-ops-handler.service';
import { OperationWriteFlushService } from './operation-write-flush.service';
import { SuperSyncStatusService } from './super-sync-status.service';
import { provideMockStore } from '@ngrx/store/testing';
import { MockStore, provideMockStore } from '@ngrx/store/testing';
import { TaskSharedActions } from '../../root-store/meta/task-shared.actions';
import {
ActionType,
Operation,
@ -260,7 +261,7 @@ describe('OperationLogSyncService', () => {
.and.returnValue(Promise.resolve(1)); // Not fresh (has seq)
});
describe('example-task cleanup on first adoption of a populated remote (#7985)', () => {
describe('example-task cleanup on first adoption of a populated remote (#7985/#7996)', () => {
const exampleCreate = (taskId: string): OperationLogEntry => ({
seq: 1,
op: {
@ -358,6 +359,7 @@ describe('OperationLogSyncService', () => {
});
const mockProvider = { isReady: () => Promise.resolve(true) } as any;
let store: MockStore;
beforeEach(() => {
uploadServiceSpy.uploadPendingOps.and.resolveTo({
@ -367,6 +369,7 @@ describe('OperationLogSyncService', () => {
rejectedOps: [],
});
opLogStoreSpy.markRejected.and.resolveTo();
store = TestBed.inject(MockStore);
});
it('discards untouched example-task ops when a never-synced client adopted a populated remote', async () => {
@ -454,6 +457,70 @@ describe('OperationLogSyncService', () => {
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
});
// ─── #7996: also remove the lingering example *entities* from local NgRx state ───
// (op-based adoption merges remote ops, so rejecting the create ops alone leaves the
// example tasks in the store, where a later snapshot upload would re-pollute the remote)
it('#7996: removes the example tasks via a meta.isRemote bulk deleteTasks (no Delete op to upload)', async () => {
const dispatchSpy = spyOn(store, 'dispatch');
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
opLogStoreSpy.getUnsynced.and.resolveTo([
exampleCreate('ex-1'),
exampleCreate('ex-2'),
]);
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
// The example task ids are removed from NgRx via the shared bulk delete meta-reducer,
// dispatched with meta.isRemote so NO Delete op is captured (nothing to upload) and
// LOCAL_ACTIONS effects (plugin TASK_DELETE etc.) do not fire.
expect(dispatchSpy).toHaveBeenCalledWith(
jasmine.objectContaining({
type: TaskSharedActions.deleteTasks.type,
taskIds: ['ex-1', 'ex-2'],
meta: jasmine.objectContaining({ isRemote: true }),
}),
);
// Only the create ops are rejected; there is no generated Delete op to reject.
expect(opLogStoreSpy.markRejected).toHaveBeenCalledOnceWith([
'ex-op-ex-1',
'ex-op-ex-2',
]);
});
it('#7996: does NOT dispatch a delete when the cleanup gate does not fire (real task pending)', async () => {
const dispatchSpy = spyOn(store, 'dispatch');
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
opLogStoreSpy.getUnsynced.and.resolveTo([
exampleCreate('ex-1'),
realTaskCreate('real-1'),
]);
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
expect(dispatchSpy).not.toHaveBeenCalledWith(
jasmine.objectContaining({ type: TaskSharedActions.deleteTasks.type }),
);
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
});
it('#7996: rejects only the example create ops, never the coexisting GLOBAL_CONFIG op', async () => {
spyOn(store, 'dispatch');
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
opLogStoreSpy.getUnsynced.and.resolveTo([
exampleCreate('ex-1'),
configOp(),
exampleCreate('ex-2'),
]);
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
const rejectedIds = opLogStoreSpy.markRejected.calls.allArgs().flat(2);
expect(rejectedIds).toContain('ex-op-ex-1');
expect(rejectedIds).toContain('ex-op-ex-2');
expect(rejectedIds).not.toContain('config-op-1');
});
});
describe('uploadPendingOps', () => {

View file

@ -33,6 +33,7 @@ import { SyncImportConflictGateService } from './sync-import-conflict-gate.servi
import { SyncProviderManager } from '../sync-providers/provider-manager.service';
import { getDefaultMainModelData } from '../model/model-config';
import { loadAllData } from '../../root-store/meta/load-all-data.action';
import { TaskSharedActions } from '../../root-store/meta/task-shared.actions';
import { SyncLocalStateService } from './sync-local-state.service';
import { SyncImportConflictCoordinatorService } from './sync-import-conflict-coordinator.service';
import { isExampleTaskCreateOp } from '../validation/is-example-task-op.util';
@ -226,14 +227,27 @@ export class OperationLogSyncService {
isExampleTaskCreateOp(entry) || entry.op.entityType === 'GLOBAL_CONFIG',
);
if (isPristinePostBootBatch) {
const exampleOpIds = pendingOps
.filter(isExampleTaskCreateOp)
.map((entry) => entry.op.id);
if (exampleOpIds.length > 0) {
await this._discardExampleTaskOps(exampleOpIds);
const exampleCreateOps = pendingOps.filter(isExampleTaskCreateOp);
if (exampleCreateOps.length > 0) {
// Stop the example create ops from UPLOADing onto the adopted remote.
await this._discardExampleTaskOps(exampleCreateOps.map((entry) => entry.op.id));
// #7996: rejecting the create ops is not enough on the SuperSync (op-based) path —
// adoption MERGES remote ops into the store rather than replacing it, so the
// example *entities* linger in local NgRx state. Remove them too, otherwise a
// later full-state snapshot (forceUploadLocalState / enable-encryption) would
// re-snapshot and re-upload them, re-polluting the remote (isExampleTask lives
// only on the op, never the Task entity, so the snapshot can't distinguish them).
// Ids derive ONLY from the local pending example-create ops, never remote ops.
const exampleTaskIds = exampleCreateOps
.map((entry) => entry.op.entityId)
.filter((id): id is string => !!id);
this._removeAdoptedExampleTasksFromState(exampleTaskIds);
OpLog.normal(
`OperationLogSyncService: Discarded ${exampleOpIds.length} untouched example-task ` +
'op(s) — never-synced client adopted a populated remote this sync (#7985).',
`OperationLogSyncService: Discarded ${exampleCreateOps.length} untouched example-task ` +
`op(s) and removed ${exampleTaskIds.length} example task(s) from local state — ` +
'never-synced client adopted a populated remote this sync (#7985/#7996).',
);
}
}
@ -876,6 +890,33 @@ export class OperationLogSyncService {
}
}
/**
* #7996: removes the untouched onboarding example tasks from local NgRx state after a
* never-synced client first adopts a populated remote companion to the create-op
* discard in uploadPendingOps. Dispatches the shared bulk delete so `project.taskIds` /
* backlog / tag ordering references are scrubbed in one meta-reducer pass (sync rule #3).
*
* The action carries `meta.isRemote` so it is handled like a sync-applied change: the
* capture meta-reducer skips op-capture (so NO Delete op is created the upload can
* never send a delete for a task the server never had), and LOCAL_ACTIONS effects do not
* fire (no plugin TASK_DELETE / reminder cascade for throwaway onboarding scaffolding).
* The state reducers still run. This is the same primitive ValidateStateService and the
* op-replay path use; it sidesteps the capture-deferral hazard a normal dispatch+reject
* would carry (a deferred Delete op would never surface to be rejected).
*
* @param exampleTaskIds ids derived from the LOCAL pending example-create ops only.
*/
private _removeAdoptedExampleTasksFromState(exampleTaskIds: string[]): void {
if (exampleTaskIds.length === 0) {
return;
}
const deleteAction = TaskSharedActions.deleteTasks({ taskIds: exampleTaskIds });
this.store.dispatch({
...deleteAction,
meta: { ...deleteAction.meta, isRemote: true },
});
}
/**
* Shows the SYNC_IMPORT conflict dialog and executes the user's chosen action.
*

View file

@ -138,6 +138,12 @@ describe('Example-task adoption cleanup mechanic (integration)', () => {
expect(isPristinePostBootBatch(pending)).toBe(false);
});
// #7996 (remove example *entities* from local NgRx) is a pure NgRx dispatch with
// meta.isRemote — it deliberately creates NO op-log op (the capture meta-reducer skips
// isRemote actions), so there is nothing op-log-level to assert here. The #7995 create-op
// discard above is the op-log half; the dispatch + state-cleanup half is covered by the
// service unit specs and the docker e2e.
it('empty-server seeding: with nothing adopted, hasSyncedOps() stays false (cleanup does not fire)', async () => {
await store.append(exampleCreate('ex-1'), 'local');
await store.append(configOp(), 'local');