lee/Termix
1
0
Fork 0
mirror of https://github.com/Termix-SSH/Termix.git synced 2026-08-23 06:26:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix remote sync stalling after the first pass and never propagating deletions (#1140)

Browse Source 
The incremental cursor never matched. updated_at/deleted_at are TEXT columns
written by CURRENT_TIMESTAMP ("2026-07-29 10:11:21"), while the client sends
an ISO 8601 since ("2026-07-29T10:06:55.172Z"). Both comparisons are lexical
and ' ' sorts below 'T', so a newer row lost at position 10 and every
?since= query came back empty. Pass 1 syncs everything (since is null) and
persists a cursor; every pass after it returns nothing with lastError: null
and reports success. Normalize since into the stored shape on the way in,
leaving an already-normalized value alone -- parsing that would treat it as
local time and, west of UTC, push the cursor past unsynced rows.

POST /sync/tombstones was unreachable. It was registered after
POST /:entityType, and "tombstones" is a valid :entityType, so the wildcard
answered it with 400 "Unknown entity type" and the handler never ran. The
pass has no per-entity error handling, so that 400 also discarded the state
of every entity type already synced in the same pass. Move it ahead of the
wildcards.

The tombstone guard consulted the incremental window. A row deleted on one
side and untouched on the other -- the shape every ordinary deletion takes
once the two sides converge -- is not in that window, so the tombstone was
skipped, and skipped again on each later pass as it slid out of its own
window. The guard cannot just be dropped: recording a tombstone for a row
that was already gone hands the sender a fresh one to push back, and the two
trade the same deletion forever. So only a delete that removed something
records a tombstone, which makes the endpoint idempotent and lets the client
push every tombstone unconditionally.

Deletions missed while the cursor was broken stay missed -- their tombstones
predate the persisted cursor. Ordinary edits do come through, since the
row's updatedAt is still newer than it.

Fixes Termix-SSH/Support#1050
Fixes Termix-SSH/Support#1051
This commit is contained in:
ZacharyZcR
2026-07-31 16:43:11 +08:00
committed by GitHub
parent 975b271a11
commit ca7abf8426
3 changed files with 173 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
electron/remote-sync.cjs
+19 -17
View File
@@ -481,26 +481,28 @@ class RemoteSyncEngine {
}
}
// Apply tombstones to whichever side hasn't already deleted the row.
// Apply every tombstone to the other side. Gating this on the row being
// present in localBySyncId/remoteBySyncId would skip the common case:
// once both sides have converged, the surviving copy of a deleted row
// has not been touched, so it is not in the incremental window at all.
// The receiving end ignores a deletion for a row it no longer has and
// records no tombstone for it, so re-sending one costs a no-op request
// and nothing ping-pongs back.
for (const tombstone of localTombstones) {
if (remoteBySyncId.has(tombstone.syncId)) {
await this.pushTombstone(
remoteBaseUrl,
remoteJwt,
entityType,
tombstone.syncId,
);
}
await this.pushTombstone(
remoteBaseUrl,
remoteJwt,
entityType,
tombstone.syncId,
);
}
for (const tombstone of remoteTombstones) {
if (localBySyncId.has(tombstone.syncId)) {
await this.pushTombstone(
EMBEDDED_BASE_URL,
this.localJwt,
entityType,
tombstone.syncId,
);
}
await this.pushTombstone(
EMBEDDED_BASE_URL,
this.localJwt,
entityType,
tombstone.syncId,
);
}
return { syncedAt };
src/backend/database/routes/sync.ts
+96 -73
View File
@@ -84,6 +84,24 @@ export function isValidEntityType(value: unknown): value is SyncEntityType {
return typeof value === "string" && VALID_ENTITY_TYPES.has(value);
}
// updatedAt/deletedAt are TEXT columns written by CURRENT_TIMESTAMP, so they
// read as "2026-07-29 10:11:21" while clients send an ISO 8601 `since` like
// "2026-07-29T10:06:55.172Z". Both comparisons are lexical, and ' ' (0x20)
// sorts below 'T' (0x54), so a newer row loses the comparison at position 10
// and the incremental window comes back empty forever. Rewrite `since` into
// the stored shape. Dropping the fraction can re-send rows from the same
// second, which is harmless — every apply path upserts by syncId.
export function normalizeSince(value: unknown): string | null {
if (typeof value !== "string" || !value) return null;
// Already stored-shaped. Parsing it would be worse than leaving it alone:
// Date treats a string with no zone as local time, which shifts the cursor
// by the offset and, west of UTC, moves it forward past unsynced rows.
if (/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/.test(value)) return value;
const parsed = new Date(value);
if (Number.isNaN(parsed.getTime())) return null;
return parsed.toISOString().slice(0, 19).replace("T", " ");
}
async function findReferenceSyncId(
context: RepositoryContext,
entityType: SyncReferenceEntity,
@@ -213,6 +231,82 @@ export function stripWritePayload(
* 500:
* description: Failed to fetch rows.
*/
/**
* @openapi
* /sync/tombstones:
* post:
* summary: Report a deletion from the other side of a sync pair
* description: Applies a remote deletion locally (if the row still exists) and records the tombstone so future pulls stay consistent.
* tags:
* - Sync
* responses:
* 200:
* description: Deletion applied (or row already absent).
* 400:
* description: Unknown entity type or missing syncId.
* 500:
* description: Failed to apply deletion.
*/
// Must stay ahead of the "/:entityType" routes below: Express matches in
// registration order and "tombstones" is a valid :entityType value, so a
// wildcard registered first would answer this path with "Unknown entity type".
router.post(
"/tombstones",
authenticateJWT,
async (req: Request, res: Response) => {
const userId = (req as AuthenticatedRequest).userId;
const entityType = req.body?.entityType;
const syncId = req.body?.syncId;
if (
!isValidEntityType(entityType) ||
typeof syncId !== "string" ||
!syncId
) {
return res.status(400).json({ error: "Missing entityType or syncId" });
}
try {
const { table, singleton } = ENTITY_CONFIG[entityType];
const context = createCurrentRepositoryContext();
const match = singleton
? eq(table.userId, userId)
: and(
eq((table as typeof hosts).syncId, syncId),
eq(table.userId, userId),
);
const existing = await context.drizzle
.select()
.from(table as typeof hosts)
.where(match)
.limit(1);
// Only a deletion that actually removed something earns a tombstone.
// Recording one unconditionally would hand the sender a fresh tombstone
// to push back on the next pass, and the two sides would trade the same
// deletion forever.
if (existing.length > 0) {
await context.drizzle.delete(table as typeof hosts).where(match);
await createCurrentSyncTombstoneRepository().record(
userId,
entityType,
syncId,
);
await DatabaseSaveTrigger.forceSave("sync_tombstone_applied");
}
res.json({ success: true });
} catch (err) {
databaseLogger.error("Failed to apply sync tombstone", err, {
operation: "sync_tombstone_apply",
entityType,
userId,
});
res.status(500).json({ error: "Failed to apply deletion" });
}
},
);
router.get(
"/:entityType",
authenticateJWT,
@@ -222,10 +316,7 @@ router.get(
if (!isValidEntityType(entityType)) {
return res.status(400).json({ error: "Unknown entity type" });
}
const since =
typeof req.query.since === "string" && req.query.since
? req.query.since
: null;
const since = normalizeSince(req.query.since);
try {
const { table, singleton } = ENTITY_CONFIG[entityType];
@@ -415,10 +506,7 @@ router.get(
if (!isValidEntityType(entityType)) {
return res.status(400).json({ error: "Unknown entity type" });
}
const since =
typeof req.query.since === "string" && req.query.since
? req.query.since
: null;
const since = normalizeSince(req.query.since);
try {
const tombstones = await createCurrentSyncTombstoneRepository().listSince(
@@ -438,69 +526,4 @@ router.get(
},
);
/**
* @openapi
* /sync/tombstones:
* post:
* summary: Report a deletion from the other side of a sync pair
* description: Applies a remote deletion locally (if the row still exists) and records the tombstone so future pulls stay consistent.
* tags:
* - Sync
* responses:
* 200:
* description: Deletion applied (or row already absent).
* 400:
* description: Unknown entity type or missing syncId.
* 500:
* description: Failed to apply deletion.
*/
router.post(
"/tombstones",
authenticateJWT,
async (req: Request, res: Response) => {
const userId = (req as AuthenticatedRequest).userId;
const entityType = req.body?.entityType;
const syncId = req.body?.syncId;
if (
!isValidEntityType(entityType) ||
typeof syncId !== "string" ||
!syncId
) {
return res.status(400).json({ error: "Missing entityType or syncId" });
}
try {
const { table, singleton } = ENTITY_CONFIG[entityType];
const context = createCurrentRepositoryContext();
await context.drizzle
.delete(table as typeof hosts)
.where(
singleton
? eq(table.userId, userId)
: and(
eq((table as typeof hosts).syncId, syncId),
eq(table.userId, userId),
),
);
await createCurrentSyncTombstoneRepository().record(
userId,
entityType,
syncId,
);
await DatabaseSaveTrigger.forceSave("sync_tombstone_applied");
res.json({ success: true });
} catch (err) {
databaseLogger.error("Failed to apply sync tombstone", err, {
operation: "sync_tombstone_apply",
entityType,
userId,
});
res.status(500).json({ error: "Failed to apply deletion" });
}
},
);
export default router;
src/backend/tests/database/routes/sync.test.ts
+58 -1
View File
@@ -1,9 +1,31 @@
import { describe, expect, it } from "vitest";
import {
import syncRouter, {
isValidEntityType,
normalizeSince,
stripWritePayload,
} from "../../../database/routes/sync.js";
describe("sync route registration order", () => {
// Express matches in registration order and "tombstones" is a perfectly
// good :entityType value, so a POST /:entityType registered first swallows
// POST /sync/tombstones and answers it with 400 "Unknown entity type".
const paths = (
syncRouter as unknown as {
stack: { route?: { path: string; methods: Record<string, boolean> } }[];
}
).stack
.filter((layer) => layer.route?.methods.post)
.map((layer) => layer.route!.path);
it("registers POST /tombstones before the POST /:entityType wildcard", () => {
expect(paths).toContain("/tombstones");
expect(paths).toContain("/:entityType");
expect(paths.indexOf("/tombstones")).toBeLessThan(
paths.indexOf("/:entityType"),
);
});
});
describe("isValidEntityType", () => {
it("accepts every whitelisted sync entity type", () => {
for (const type of [
@@ -29,6 +51,41 @@ describe("isValidEntityType", () => {
});
});
describe("normalizeSince", () => {
it("rewrites an ISO cursor into the shape CURRENT_TIMESTAMP stores", () => {
expect(normalizeSince("2026-07-29T10:06:55.172Z")).toBe(
"2026-07-29 10:06:55",
);
});
it("makes a newer row win the lexical comparison against the cursor", () => {
// The bug: a row written five minutes after the cursor still compared as
// older, because ' ' (0x20) sorts below 'T' (0x54) at position 10.
const storedRow = "2026-07-29 10:11:21";
const rawCursor = "2026-07-29T10:06:55.172Z";
expect(storedRow > rawCursor).toBe(false);
expect(storedRow > normalizeSince(rawCursor)!).toBe(true);
});
it("keeps a Postgres timestamp with fraction and offset above the cursor", () => {
expect(
"2026-07-29 10:06:55.123456+00" >
normalizeSince("2026-07-29T10:06:55.172Z")!,
).toBe(true);
});
it("treats a missing, empty, or unparsable cursor as no filter", () => {
expect(normalizeSince(undefined)).toBeNull();
expect(normalizeSince("")).toBeNull();
expect(normalizeSince("not a date")).toBeNull();
expect(normalizeSince(1753783615172)).toBeNull();
});
it("is idempotent, so an already-normalized cursor survives a round trip", () => {
expect(normalizeSince("2026-07-29 10:06:55")).toBe("2026-07-29 10:06:55");
});
});
describe("stripWritePayload", () => {
it("strips id, userId, and syncId from every entity type", () => {
const payload = {