logseq/docs/agent-guide/054-db-sync-test-isolation.md

054-db-sync-test-isolation

Summary

bb dev:lint-and-test would currently fail in CLJS runtime tests, while lint and test compilation would still pass. The observed failures would point to test isolation problems inside frontend.worker.db-sync-test, not to a stable regression in db-sync production logic.

Background

The failing command would be:

• bb dev:lint-and-test

Its execution flow would be:

1. lint tasks
2. CLJS test compilation
3. CLJS runtime test execution

Observed behavior from the captured run:

• lint would pass
• test compilation would pass
• runtime tests would fail with 3 failures

The failing tests would include:

• frontend.worker.db-sync-test/ensure-upload-graph-identity-defaults-e2ee-enabled-test
• frontend.worker.db-sync-test/download-graph-by-id-fails-on-incomplete-snapshot-frame-test

A follow-up run of the full namespace would fail on a different test:

• frontend.worker.db-sync-test/pull-ok-out-of-order-stale-response-is-ignored-test

This changing failure surface would strongly suggest flaky shared state rather than a deterministic implementation bug.

Evidence

1. Failing tests would pass in isolation

Running these tests individually would pass:

• frontend.worker.db-sync-test/ensure-upload-graph-identity-defaults-e2ee-enabled-test
• frontend.worker.db-sync-test/download-graph-by-id-fails-on-incomplete-snapshot-frame-test

This would indicate that the underlying code paths can behave correctly when the test environment is clean.

2. Full-namespace execution would produce a different failure

Running the whole namespace would fail in pull-ok-out-of-order-stale-response-is-ignored-test instead of the two failures seen in the full suite.

This would indicate order dependence or leaked global state between tests in the same namespace.

3. Failure payloads would match leaked state

ensure-upload-graph-identity-defaults-e2ee-enabled-test would unexpectedly receive an existing graph identity with:

• :graph-id "c52bf9d4-3a95-4f17-9c8c-7f86eef5f75d"
• :graph-e2ee? false

That graph id would match a value used by another test in the same file. This would suggest that the later test is reusing state left behind by an earlier test instead of executing its own intended bootstrap path.

download-graph-by-id-fails-on-incomplete-snapshot-frame-test would expect incomplete-snapshot-frame but would sometimes receive an unrelated assertion failure:

• Assert failed: test-db-sync-repo (some? conn)

This would suggest that another asynchronous code path is still interacting with shared worker state while the test is running.

Root Cause

The most likely root cause would be incomplete per-test cleanup of shared global state in:

• frontend.worker.state
• frontend.worker.sync

The highest-risk shared atoms would include:

• worker-state/*datascript-conns
• worker-state/*client-ops-conns
• worker-state/*db-sync-client
• worker-state/*db-sync-config
• db-sync/*repo->latest-remote-tx
• db-sync/*start-inflight-target

The test file would also rely heavily on:

• async done
• nested promesa chains
• cleanup inside p/finally

That structure would make it possible for a test to signal completion before all outer cleanup has fully restored shared state, especially when asynchronous callbacks or listeners are still active.

Proposed Changes

1. Add a per-test fixture for global state reset

A :each fixture in /Users/rcmerci/gh-repos/logseq/src/test/frontend/worker/db_sync_test.cljs would reset the shared worker and db-sync atoms before and after every test.

The fixture would reset at least:

• worker-state/*datascript-conns
• worker-state/*client-ops-conns
• worker-state/*db-sync-client
• worker-state/*db-sync-config
• db-sync/*repo->latest-remote-tx
• db-sync/*start-inflight-target

2. Review high-risk async tests

If fixture-based isolation is not sufficient, the next step would be to tighten async structure in the tests most likely to leak state.

Priority candidates would include:

• upload identity tests
• snapshot download tests
• pull/ok ordering tests

The goal would be to ensure that:

• cleanup happens after the actual promise chain settles
• done is called only after cleanup boundaries are satisfied
• any listener or temporary override is fully restored before the next test starts

Why Production Code Is Less Likely To Be At Fault

The current evidence would not strongly support a production db-sync bug because:

1. the affected tests would pass when run alone
2. the failing test would change depending on suite shape
3. the observed incorrect values would match data seeded by neighboring tests
4. the expected production code path for incomplete snapshot handling already appears to raise the correct error in isolated execution

Because of that, modifying production sync logic first would risk masking a test harness problem instead of fixing the actual instability.

Verification Plan

After applying the test isolation changes, verification would include:

1. bb dev:test -v frontend.worker.db-sync-test
2. bb dev:lint-and-test

Success criteria would be:

• individual failing tests still pass
• the full frontend.worker.db-sync-test namespace passes reliably
• the full bb dev:lint-and-test command no longer fails on these db-sync tests

Risks And Follow-ups

If failures remain after the fixture reset, the next investigation would focus on:

• leaked d/listen! listeners
• background tasks still running across tests
• js/fetch restoration timing
• promise chains whose cleanup is attached at the wrong level
• other shared atoms not yet covered by the fixture

Files

Would modify:

• /Users/rcmerci/gh-repos/logseq/src/test/frontend/worker/db_sync_test.cljs

Would not modify initially:

• /Users/rcmerci/gh-repos/logseq/src/main/frontend/worker/sync.cljs
• /Users/rcmerci/gh-repos/logseq/src/main/frontend/worker/sync/client_op.cljs