Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/app/ui/components/Channel/PinnedPostBanner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ export function PinnedPostBanner({
}
}, [pinnedPostId]);

if (!pinnedPostId || !postQuery.data || isDismissed) {
if (
!pinnedPostId ||
!postQuery.data ||
postQuery.data.isDeleted ||
isDismissed
) {
return null;
}

Expand Down
130 changes: 128 additions & 2 deletions packages/shared/src/db/queries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,7 @@ describe('getPendingPosts', () => {
expect(ids).not.toContain(failedCleared.id);
});

test('includes deleted markPostSent / needs_verification rows so the remount path still has a tombstone source', async () => {
test('includes deleted markPostSent / needs_verification rows while the delete is not settled', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
// `markPostSent` catch-up: server already acknowledged, sequenced
// `addPost` hasn't arrived yet, and the user deleted in the gap.
Expand All @@ -1225,13 +1225,15 @@ describe('getPendingPosts', () => {
deliveryStatus: 'sent',
sequenceNum: 0,
isDeleted: true,
deleteStatus: null,
});
// `needs_verification` may also have reached the server.
const verifyCleared = await seedPost({
id: 'verify-cleared',
deliveryStatus: 'needs_verification',
sequenceNum: 0,
isDeleted: true,
deleteStatus: 'pending',
});
const ids = (await queries.getPendingPosts(channelId)).map((p) => p.id);
expect(ids).toEqual(
Expand All @@ -1246,19 +1248,36 @@ describe('getPendingPosts', () => {
deliveryStatus: 'enqueued',
sequenceNum: 0,
isDeleted: true,
deleteStatus: null,
});
const pendingCleared = await seedPost({
id: 'pending-cleared',
deliveryStatus: 'pending',
sequenceNum: 0,
isDeleted: true,
deleteStatus: 'pending',
});
const ids = (await queries.getPendingPosts(channelId)).map((p) => p.id);
expect(ids).toEqual(
expect.arrayContaining([enqueuedCleared.id, pendingCleared.id])
);
});

test('excludes settled deleted rows without deleting them from storage', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const settled = await seedPost({
id: 'settled-delete',
deliveryStatus: 'sent',
sequenceNum: 0,
isDeleted: true,
deleteStatus: 'sent',
});

const ids = (await queries.getPendingPosts(channelId)).map((p) => p.id);
expect(ids).not.toContain(settled.id);
expect(await queries.getPost({ postId: settled.id })).toBeTruthy();
});

test('excludes confirmed rows (null deliveryStatus) regardless of isDeleted', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const confirmed = await seedPost({
Expand Down Expand Up @@ -1292,6 +1311,54 @@ describe('getPendingPosts', () => {
});
});

describe('deleteUnsequencedAcknowledgedPost', () => {
const channelId = 'conditional-delete-channel';

async function seedPost(id: string, overrides: Partial<Post> = {}) {
const post = {
id,
type: 'chat',
channelId,
authorId: '~zod',
sentAt: Date.now(),
receivedAt: Date.now(),
sequenceNum: 0,
deliveryStatus: 'sent',
content: JSON.stringify([{ inline: ['seed'] }]),
syncedAt: Date.now(),
...overrides,
} as Post;
await queries.insertChannelPosts({ posts: [post] });
return post;
}

test('deletes and returns an acknowledged row while it is still unsequenced', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const post = await seedPost('still-unsequenced');

const deleted = await queries.deleteUnsequencedAcknowledgedPost(post.id);

expect(deleted?.id).toBe(post.id);
expect(await queries.getPost({ postId: post.id })).toBeNull();
});

test('preserves the row when a sequenced echo wins the race', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const post = await seedPost('sequenced-before-delete');
// This update represents the addPost echo landing after the caller chose
// the hard-delete path but before SQLite executes the guarded DELETE.
await queries.updatePost({ id: post.id, sequenceNum: 42 });

const deleted = await queries.deleteUnsequencedAcknowledgedPost(post.id);

expect(deleted).toBeNull();
expect(await queries.getPost({ postId: post.id })).toMatchObject({
id: post.id,
sequenceNum: 42,
});
});
});

// TLON-5606: `undoOptimisticReplyBump` must undo a single optimistic reply
// add without clobbering server-sourced reply summary state. Two regimes:
// complete local cache → full recompute; partial local cache → decrement
Expand Down Expand Up @@ -1572,6 +1639,32 @@ describe('recomputeChannelLastPost', () => {
expect(channel!.lastPostAt).toBe(1000);
});

test('repairs the stale preview shape left by a persisted settled-delete ghost', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
await seedTopLevel('previous-post', 1000);
await seedTopLevel('persisted-ghost', 2000, {
sequenceNum: 0,
deliveryStatus: 'sent',
isDeleted: true,
deleteStatus: 'sent',
});
// The legacy delete path nulled only the id, leaving the old timestamp.
await queries.updateChannel({
id: channelId,
lastPostId: null,
lastPostAt: 2000,
});

await queries.recomputeChannelLastPost({ channelId });

const channel = await queries.getChannel({ id: channelId });
expect(channel!.lastPostId).toBe('previous-post');
expect(channel!.lastPostAt).toBe(1000);
// Read-layer repair is non-destructive; the settled row stays inert in
// storage and remains available for any later sync reconciliation.
expect(await queries.getPost({ postId: 'persisted-ghost' })).toBeTruthy();
});

test('also repairs parent group lastPostId / lastPostAt when the channel belongs to a group', async () => {
const groupId = '~zod/group-with-preview';
await queries.insertGroups({
Expand Down Expand Up @@ -1889,7 +1982,7 @@ describe('getDeliveryPendingPosts', () => {
return base;
}

test('includes enqueued and pending rows regardless of isDeleted', async () => {
test('includes enqueued and pending rows while their delete is not settled', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const enqueuedLive = await seedPost({
id: 'enq-live',
Expand All @@ -1900,6 +1993,7 @@ describe('getDeliveryPendingPosts', () => {
id: 'enq-cleared',
deliveryStatus: 'enqueued',
isDeleted: true,
deleteStatus: null,
});
const pendingLive = await seedPost({
id: 'pend-live',
Expand All @@ -1910,6 +2004,7 @@ describe('getDeliveryPendingPosts', () => {
id: 'pend-cleared',
deliveryStatus: 'pending',
isDeleted: true,
deleteStatus: 'pending',
});

const ids = (await queries.getDeliveryPendingPosts(channelId)).map(
Expand Down Expand Up @@ -1982,6 +2077,37 @@ describe('getDeliveryPendingPosts', () => {
expect(ids).toContain(catchUp.id);
});

test('excludes settled deleted rows across delivery states without deleting them', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const enqueued = await seedPost({
id: 'settled-enqueued',
deliveryStatus: 'enqueued',
isDeleted: true,
deleteStatus: 'sent',
});
const pending = await seedPost({
id: 'settled-pending',
deliveryStatus: 'pending',
isDeleted: true,
deleteStatus: 'sent',
});
const sent = await seedPost({
id: 'settled-sent',
deliveryStatus: 'sent',
sequenceNum: 0,
isDeleted: true,
deleteStatus: 'sent',
});

const ids = (await queries.getDeliveryPendingPosts(channelId)).map(
(p) => p.id
);
expect(ids).not.toContain(enqueued.id);
expect(ids).not.toContain(pending.id);
expect(ids).not.toContain(sent.id);
expect(await queries.getPost({ postId: sent.id })).toBeTruthy();
});

test('excludes sequenced sent rows (the sequenced addPost has already reconciled the row)', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const reconciled = await seedPost({
Expand Down
70 changes: 53 additions & 17 deletions packages/shared/src/db/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4179,6 +4179,31 @@ export const deletePost = createWriteQuery(
['posts']
);

/**
* Delete an acknowledged optimistic top-level post only while it is still
* unsequenced. Keeping the state guard in the DELETE predicate closes the
* race where a sequenced addPost echo updates the row between a read and a
* later unconditional delete.
*/
export const deleteUnsequencedAcknowledgedPost = createWriteQuery(
'deleteUnsequencedAcknowledgedPost',
async (postId: string, ctx: QueryCtx) => {
const deleted = await ctx.db
.delete($posts)
.where(
and(
eq($posts.id, postId),
eq($posts.sequenceNum, 0),
isNull($posts.parentId),
inArray($posts.deliveryStatus, ['sent', 'needs_verification'])
)
)
.returning();
return deleted[0] ?? null;
},
['posts']
);

export const markPostAsDeleted = createWriteQuery(
'markPostAsDeleted',
async (postId: string, ctx: QueryCtx) => {
Expand Down Expand Up @@ -4598,22 +4623,25 @@ export const getPendingPosts = createReadQuery(
eq($posts.channelId, channelId),
isNotNull($posts.deliveryStatus),
not(eq($posts.type, 'reply')),
// Exclude only truly local-only deleted sends. Deleted rows whose
// `deliveryStatus` is `'sent'` or `'needs_verification'` may still
// be server-backed but not yet sequenced; keep them as a DB-backed
// tombstone source so the channel remount path still renders them
// before the sequenced `addPost` arrives. Enqueued / pending rows
// are still in-flight and also remain visible — they either reach
// `'sent'` (stay as tombstone) or `'failed'` (vanish on next tick).
// Exclude deleted sends once they are either known to be local-only
// failures or their server delete has settled. The latter also makes
// old ghost rows inert without a startup cleanup task. Rows whose
// delete is still in flight remain a tombstone source until the
// outcome is known.
//
// SQL NULL handling: `isDeleted` is a nullable boolean. A bare
// `not(and(eq(isDeleted, true), ...))` would evaluate to NULL for
// rows where `isDeleted IS NULL`, which SQL treats as filter-out.
// Enumerate the keep condition explicitly instead.
// SQL NULL handling: `isDeleted` and `deleteStatus` are nullable. A
// negated conjunction would evaluate to NULL for unset values, which
// SQL treats as filter-out, so enumerate the keep cases explicitly.
or(
isNull($posts.isDeleted),
eq($posts.isDeleted, false),
not(eq($posts.deliveryStatus, 'failed'))
and(
not(eq($posts.deliveryStatus, 'failed')),
or(
isNull($posts.deleteStatus),
not(eq($posts.deleteStatus, 'sent'))
Comment thread
jamesacklin marked this conversation as resolved.
)
)
)
),
});
Expand All @@ -4624,11 +4652,11 @@ export const getPendingPosts = createReadQuery(
/**
* Rows that are still in-flight from the sender's perspective, used by
* `syncChannelWithBackoff` to decide whether delivery polling should
* continue. Unlike `getPendingPosts` (UI merge input), this does NOT
* exclude `isDeleted` rows — a user can delete a post mid-flight, and the
* server still needs to acknowledge the original send before anything is
* truly settled. `failed` and `needs_verification` are handled by dedicated
* retry / verification flows, so they are not treated as "still delivering".
* continue. Deleted rows remain eligible while the delete is in flight, but
* drop out once `deleteStatus` is `sent`: the server has settled the delete,
* so there is no original send left for delivery polling to reconcile.
* `failed` and `needs_verification` are handled by dedicated retry /
* verification flows, so they are not treated as "still delivering".
*
* Server-acknowledged but unsequenced rows (`deliveryStatus: 'sent'` while
* still `sequenceNum === 0`) are also included: `markPostSent` sets the
Expand All @@ -4649,6 +4677,14 @@ export const getDeliveryPendingPosts = createReadQuery(
eq($posts.deliveryStatus, 'enqueued'),
eq($posts.deliveryStatus, 'pending'),
and(eq($posts.deliveryStatus, 'sent'), eq($posts.sequenceNum, 0))
),
// Keep nullable values explicit to avoid filtering ordinary rows via
// SQL's three-valued logic.
or(
isNull($posts.isDeleted),
eq($posts.isDeleted, false),
isNull($posts.deleteStatus),
not(eq($posts.deleteStatus, 'sent'))
Comment thread
jamesacklin marked this conversation as resolved.
Outdated
)
),
});
Expand Down
38 changes: 37 additions & 1 deletion packages/shared/src/store/dbHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,49 @@ export const useAllChannels = ({ enabled }: { enabled?: boolean }) => {
export const useCurrentChats = (
queryConfig?: CustomQueryConfig<GroupedChats>
): UseQueryResult<GroupedChats | null> => {
return useQuery({
const query = useQuery({
queryFn: async () => {
return db.getChats();
},
queryKey: ['currentChats', useKeyFromQueryDeps(db.getChats)],
...queryConfig,
});

useEffect(() => {
if (!query.data) return;

// Older settled-delete ghosts can leave `lastPostId` nulled while the old
// `lastPostAt` survives. Repair only that inconsistent shape as it falls
// out of the chat-list read layer; recomputation clears the timestamp or
// installs a real head, so this work is self-limiting and needs no startup
// sweep.
const staleChannelIds = new Set<string>();
const chats = [
...query.data.pinned,
...query.data.pending,
...query.data.unpinned,
];
for (const chat of chats) {
const channels =
chat.type === 'channel' ? [chat.channel] : chat.group.channels ?? [];
for (const channel of channels) {
if (channel.lastPostId === null && channel.lastPostAt !== null) {
staleChannelIds.add(channel.id);
Comment thread
jamesacklin marked this conversation as resolved.
Outdated
}
}
}
if (staleChannelIds.size === 0) return;

void (async () => {
for (const channelId of staleChannelIds) {
await db.recomputeChannelLastPost({ channelId });
}
})().catch((error) => {
console.error('Failed to repair stale channel previews', error);
});
}, [query.data]);

return query;
};

// Probe %notes once to detect whether the notes desk is installed on the
Expand Down
Loading
Loading