mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 14:30:13 +00:00
fix(web): fence child components' post-await sends against identity change (BUG-3095) (#1400)
* wip(BUG-3095): fence rows 1-8 + driven leg (pre-counterfactual checkpoint) * wip(BUG-3095): population guard + gap list (pre-mutation checkpoint) * fix(web): fence child components' post-await requests against identity change (BUG-3095) TimelineVersionCard and ChildItems issue their own requests after awaiting a parent-provided callback or a previous iteration of their own write loop. None of the population held an identity fence: every guard present compares the workspace/item slugs, which do not move when the signed-in USER changes on the same workspace and item. Fixes the eight await-then-send paths in those two files, adds a driven leg for the known member, and lands an AST guard over the whole child population with the remaining six paths enumerated as known gaps. Claude-Session: https://claude.ai/code/session_01JoHaYR8C9wtRnb589CQGC8 * test(web): resolve one-hop local-helper sends in the population guard (BUG-3095) Three independent hand audits of the same population found await-then-send paths this guard reported as having no send at all: the commonest post-await send here is not an api.* call but a call to the component's own loader (loadTimeline, loadChildren). Matching api.* syntactically missed every one -- a false negative, the direction that stays silent. Adds one hop of local-helper resolution, which surfaces eight further paths (ItemTimeline's six comment/reaction handlers, CopyItemDialog.runPreflight, PushToAgentDialog.handleSend); all are enumerated in KNOWN_UNFENCED. Declares depth, commits and non-await suspension as gaps (c)(d)(e). Claude-Session: https://claude.ai/code/session_01JoHaYR8C9wtRnb589CQGC8 * fix(web): fence the attachment delete confirmation against identity change (BUG-3095 row 9) Claude-Session: https://claude.ai/code/session_01JoHaYR8C9wtRnb589CQGC8 --------- Co-authored-by: xarmian <dave@houseofvoi.com>
This commit is contained in:
xarmian
@@ -6,6 +6,7 @@
|
||||
import { syncService } from '$lib/services/sync.svelte';
|
||||
import { collectionStore } from '$lib/stores/collections.svelte';
|
||||
import { workspaceStore } from '$lib/stores/workspace.svelte';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import type { Item, Collection, PaneTarget } from '$lib/types';
|
||||
import { parseFields, parseSchema, formatItemRef } from '$lib/types';
|
||||
@@ -171,13 +172,31 @@
|
||||
.filter((i: any) => !i[SHADOW_ITEM_MARKER_PROPERTY_NAME])
|
||||
.map((item, index) => ({ id: item.id, sort_order: index }));
|
||||
|
||||
// IDENTITY fence (BUG-3095) + the workspace this reorder is about,
|
||||
// both captured before the loop. `wsSlug` was read LIVE inside the loop
|
||||
// before this change, so a workspace switch mid-loop addressed the
|
||||
// remaining PATCHes at the new workspace.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
const reqWs = wsSlug;
|
||||
try {
|
||||
for (const { id, sort_order } of updates) {
|
||||
// HT-2176 Option A (TASK-2172): NO per-PATCH freeze recheck. The
|
||||
// reorder was INITIATED before peeking (the top guard blocks a NEW
|
||||
// one); breaking mid-loop would persist it only partially, leaving
|
||||
// inconsistent sort_orders. Let the initiated reorder finish.
|
||||
await api.items.update(wsSlug, id, { sort_order });
|
||||
//
|
||||
// IDENTITY is checked per iteration even though FREEZE is not
|
||||
// (BUG-3095), and the two are not in tension. Option A's argument
|
||||
// is that a half-applied reorder is worse than a fully-applied one
|
||||
// the user no longer has permission to make — true while the
|
||||
// SENDER is the same person throughout. It says nothing about a
|
||||
// DIFFERENT user: every iteration after the first sends after the
|
||||
// previous iteration's await, so a sign-out mid-loop would issue
|
||||
// the remaining writes on the next user's cookie. A partial
|
||||
// reorder is the lesser harm there, and it is the only outcome
|
||||
// that keeps the writes attributable to whoever started them.
|
||||
if (!isSameIdentity()) return;
|
||||
await api.items.update(reqWs, id, { sort_order });
|
||||
}
|
||||
} catch (e) {
|
||||
console.error('Failed to persist reorder:', e);
|
||||
@@ -204,11 +223,18 @@
|
||||
groupData[status] = reordered.map((it, idx) => ({ ...it, sort_order: idx }));
|
||||
|
||||
const updates = reorderGroup(grp, child.id, dir);
|
||||
// IDENTITY fence + captured workspace (BUG-3095) — see handleFinalize
|
||||
// for why the loop needs a per-iteration check and why that does not
|
||||
// reopen Option A's partial-reorder argument.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
const reqWs = wsSlug;
|
||||
try {
|
||||
for (const u of updates) {
|
||||
// Option A (TASK-2172): no per-PATCH freeze recheck — a reorder
|
||||
// initiated pre-pane finishes fully (see handleFinalize).
|
||||
await api.items.update(wsSlug, u.item.id, { sort_order: u.sort_order });
|
||||
// initiated pre-pane finishes fully (see handleFinalize). Identity
|
||||
// IS checked per iteration even though freeze is not (BUG-3095).
|
||||
if (!isSameIdentity()) return;
|
||||
await api.items.update(reqWs, u.item.id, { sort_order: u.sort_order });
|
||||
}
|
||||
} catch (e) {
|
||||
console.error('Failed to persist reorder:', e);
|
||||
@@ -225,18 +251,30 @@
|
||||
let loadSeq = 0;
|
||||
|
||||
async function loadChildren() {
|
||||
// Capture the request identity (item + workspace) + sequence BEFORE the
|
||||
// await. ItemDetail reuses this panel across a no-{#key} item switch (its
|
||||
// `itemSlug` prop just changes), so a slower A load must NOT overwrite
|
||||
// B's children — nor fire onChildrenChange with A's data into the
|
||||
// parent's childItemIds / progress overrides (PLAN-2105 / TASK-2112).
|
||||
// NAVIGATION fence: capture the REQUEST identity (item + workspace) +
|
||||
// sequence BEFORE the await. ItemDetail reuses this panel across a
|
||||
// no-{#key} item switch (its `itemSlug` prop just changes), so a slower A
|
||||
// load must NOT overwrite B's children — nor fire onChildrenChange with
|
||||
// A's data into the parent's childItemIds / progress overrides
|
||||
// (PLAN-2105 / TASK-2112).
|
||||
const reqSlug = itemSlug;
|
||||
const reqWs = wsSlug;
|
||||
const seq = ++loadSeq;
|
||||
// IDENTITY fence (BUG-3095): a different SIGNED-IN USER on the same
|
||||
// workspace and item moves none of the three values above, so without
|
||||
// this the commit below paints one identity's children into the next
|
||||
// identity's pane and pushes them into the parent through
|
||||
// onChildrenChange.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
// `destroyed` guards the {#key itemSlug} teardown → same-slug remount
|
||||
// case: a late load from the old instance must not push stale children
|
||||
// through onChildrenChange into the freshly-mounted parent (Codex).
|
||||
const stale = () => destroyed || seq !== loadSeq || reqSlug !== itemSlug || reqWs !== wsSlug;
|
||||
const stale = () =>
|
||||
destroyed ||
|
||||
!isSameIdentity() ||
|
||||
seq !== loadSeq ||
|
||||
reqSlug !== itemSlug ||
|
||||
reqWs !== wsSlug;
|
||||
// BUG-2871: only tear the list down when there is nothing valid to show
|
||||
// for THIS item — a first load or an item switch. A same-item REFRESH
|
||||
// keeps the rendered rows mounted, whether it succeeds or fails.
|
||||
@@ -547,12 +585,23 @@
|
||||
const title = createTitle.trim();
|
||||
const collSlug = createCollSlug;
|
||||
if (!title || !collSlug || creating) return;
|
||||
// DR-6b: capture identity BEFORE the await; bail after it if the
|
||||
// instance was destroyed (item switch) or the identity moved.
|
||||
// DR-6b: capture the REQUEST identity (workspace + item) BEFORE the
|
||||
// await; bail after it if the instance was destroyed or the pane
|
||||
// navigated. This is a NAVIGATION fence — it was described as an
|
||||
// identity fence, which it is not (BUG-3095).
|
||||
const reqWs = wsSlug;
|
||||
const reqSlug = itemSlug;
|
||||
const reqId = itemId;
|
||||
const stale = () => destroyed || reqWs !== wsSlug || reqSlug !== itemSlug || reqId !== itemId;
|
||||
// IDENTITY fence (BUG-3095): the signed-in USER, which none of the three
|
||||
// captures above can see. Guards the post-await `loadChildren()` refresh,
|
||||
// which is a fresh request issued after the create settles.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
const stale = () =>
|
||||
destroyed ||
|
||||
!isSameIdentity() ||
|
||||
reqWs !== wsSlug ||
|
||||
reqSlug !== itemSlug ||
|
||||
reqId !== itemId;
|
||||
creating = true;
|
||||
try {
|
||||
// DR-1: parent goes ONLY through the `fields` JSON `parent` key.
|
||||
@@ -625,13 +674,22 @@
|
||||
linkSearching = false;
|
||||
return;
|
||||
}
|
||||
// DR-6b: per-query sequence + identity fence.
|
||||
// DR-6b: per-query sequence + NAVIGATION fence (workspace + item). Was
|
||||
// described as an identity fence; it is not one (BUG-3095).
|
||||
const seq = ++searchSeq;
|
||||
const reqWs = wsSlug;
|
||||
const reqSlug = itemSlug;
|
||||
const reqId = itemId;
|
||||
// IDENTITY fence (BUG-3095): search results are user-scoped, so a swap
|
||||
// mid-query would render the previous identity's matches to the next one.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
const stale = () =>
|
||||
destroyed || seq !== searchSeq || reqWs !== wsSlug || reqSlug !== itemSlug || reqId !== itemId;
|
||||
destroyed ||
|
||||
!isSameIdentity() ||
|
||||
seq !== searchSeq ||
|
||||
reqWs !== wsSlug ||
|
||||
reqSlug !== itemSlug ||
|
||||
reqId !== itemId;
|
||||
linkSearching = true;
|
||||
try {
|
||||
const res = await api.search(q, { workspace: reqWs });
|
||||
@@ -656,11 +714,20 @@
|
||||
const cand = confirmCandidate;
|
||||
// Freeze guard (TASK-2172): mirror submitCreate — no reparent while frozen.
|
||||
if (!cand || linking || frozen) return;
|
||||
// DR-6b: capture identity BEFORE the await.
|
||||
// DR-6b: capture the REQUEST identity (workspace + item) BEFORE the
|
||||
// await — a NAVIGATION fence, not an identity one (BUG-3095).
|
||||
const reqWs = wsSlug;
|
||||
const reqSlug = itemSlug;
|
||||
const reqId = itemId;
|
||||
const stale = () => destroyed || reqWs !== wsSlug || reqSlug !== itemSlug || reqId !== itemId;
|
||||
// IDENTITY fence (BUG-3095): guards the post-await `loadChildren()`
|
||||
// refresh, a fresh request issued after the link write settles.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
const stale = () =>
|
||||
destroyed ||
|
||||
!isSameIdentity() ||
|
||||
reqWs !== wsSlug ||
|
||||
reqSlug !== itemSlug ||
|
||||
reqId !== itemId;
|
||||
linking = true;
|
||||
try {
|
||||
// SOURCE = candidate, TARGET = current item id. Address the source
|
||||
|
||||
@@ -40,6 +40,7 @@
|
||||
*/
|
||||
import { onDestroy, untrack } from 'svelte';
|
||||
import { api, PadApiError } from '$lib/api/client';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import type { AttachmentListItem } from '$lib/types';
|
||||
import {
|
||||
iconForAttachment,
|
||||
@@ -217,6 +218,13 @@
|
||||
att: StripAttachment;
|
||||
anchor: HTMLElement | null;
|
||||
prompt: string;
|
||||
/**
|
||||
* IDENTITY fence (BUG-3095 row 9), captured when the menu OPENED and
|
||||
* checked at the point that sends. It rides on the pending record rather
|
||||
* than on a component field because it belongs to THIS confirmation: a
|
||||
* second `×` click replaces the record and must capture afresh.
|
||||
*/
|
||||
isSameIdentity: () => boolean;
|
||||
} | null>(null);
|
||||
|
||||
const uid = $props.id();
|
||||
@@ -1060,6 +1068,10 @@
|
||||
att,
|
||||
anchor,
|
||||
prompt: attachmentDeletePrompt(att.filename, referencedIds().has(att.id)),
|
||||
// Captured HERE, when the menu opens, so the check at confirm time
|
||||
// compares against the identity that asked for the deletion — not
|
||||
// against whoever is signed in when the button is pressed.
|
||||
isSameIdentity: authStore.identityFence(),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1091,12 +1103,26 @@
|
||||
* returned; an in-app confirmation does NOT block, and the user can switch
|
||||
* item or workspace while it is up. So the fence — and the permission — are
|
||||
* re-checked HERE, at the point that actually sends the request.
|
||||
*
|
||||
* THREE checks, answering three different questions (BUG-3095 row 9 added
|
||||
* the third). `canDelete` is the PERMISSION. `paint.isCurrent()` is the
|
||||
* NAVIGATION fence: is the strip still showing the workspace and item this
|
||||
* tile was painted for. `pending.isSameIdentity()` is the IDENTITY fence: is
|
||||
* the same USER still signed in.
|
||||
*
|
||||
* The third is not implied by the other two, and this is the site that shows
|
||||
* why most sharply. A logout and a different login while the confirmation
|
||||
* menu is open leaves the workspace and item unchanged, so the navigation
|
||||
* fence passes — and the DELETE below would go out on the NEW user's cookie,
|
||||
* for a deletion the previous user asked for. There is no `await` anywhere in
|
||||
* that window, so no post-await guard anywhere in this file could have caught
|
||||
* it; the suspension is the user reading a prompt.
|
||||
*/
|
||||
function confirmDelete() {
|
||||
const pending = pendingDelete;
|
||||
pendingDelete = null;
|
||||
if (!pending) return;
|
||||
if (!canDelete || !paint.isCurrent()) return;
|
||||
if (!canDelete || !paint.isCurrent() || !pending.isSameIdentity()) return;
|
||||
void performDelete(pending.att);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,243 @@
|
||||
/**
|
||||
* BUG-3095 row 9 — the DRIVEN leg for `ItemAttachmentStrip`'s delete
|
||||
* confirmation.
|
||||
*
|
||||
* WHY THIS ONE NEEDED ITS OWN LEG, and why the population guard could never
|
||||
* have found it. Every other member of this surface suspends on an `await`.
|
||||
* This one suspends on a HUMAN: `requestDelete` opens a non-blocking in-app
|
||||
* confirmation menu and returns, and `confirmDelete` sends the DELETE when the
|
||||
* user presses the destructive row. There is no `await` anywhere between those
|
||||
* two points, so no post-await analysis — the guard's whole model — applies.
|
||||
*
|
||||
* The site already understood the shape and got the quantity wrong. Its comment
|
||||
* reasons it out correctly ("`window.confirm` blocked the thread … so the fence
|
||||
* — and the permission — are re-checked HERE, at the point that actually sends
|
||||
* the request"), but the re-check was `paint.isCurrent()`, which compares
|
||||
* `{ws, item}`. A logout and a different login while the menu is open moves
|
||||
* NEITHER, so the navigation fence passes and the DELETE goes out on the new
|
||||
* user's cookie — a destructive call, for a deletion the previous user asked
|
||||
* for.
|
||||
*
|
||||
* The auth double is local to this file rather than the family's shared one,
|
||||
* because the sibling suite `ItemAttachmentStrip.svelte.test.ts` deliberately
|
||||
* does NOT mock `authStore`: it uses the real store, whose epoch never moves
|
||||
* under test, so `identityFence()` is inert there and every existing assertion
|
||||
* is unaffected. Mocking it here keeps that true.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
import type { AttachmentListResponse } from '$lib/types';
|
||||
|
||||
const listMock =
|
||||
vi.fn<(ws: string, filters: Record<string, unknown>) => Promise<AttachmentListResponse>>();
|
||||
const deleteMock = vi.fn<(ws: string, id: string) => Promise<void>>();
|
||||
|
||||
// Declared via `vi.hoisted` rather than as a top-level class: `vi.mock`'s
|
||||
// factory is hoisted above every module-scope binding, so a plain `class`
|
||||
// referenced from inside it throws `Cannot access 'FakeApiError' before
|
||||
// initialization` at mock time.
|
||||
const { FakeApiError } = vi.hoisted(() => ({
|
||||
FakeApiError: class extends Error {
|
||||
code: string;
|
||||
constructor(code: string) {
|
||||
super(code);
|
||||
this.code = code;
|
||||
}
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('$lib/api/client', () => ({
|
||||
PadApiError: FakeApiError,
|
||||
api: {
|
||||
attachments: {
|
||||
list: (ws: string, filters: Record<string, unknown>) => listMock(ws, filters),
|
||||
downloadUrl: (ws: string, id: string, variant?: string) =>
|
||||
`/api/v1/workspaces/${ws}/attachments/${id}${variant ? `?variant=${variant}` : ''}`,
|
||||
delete: (ws: string, id: string) => deleteMock(ws, id),
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
const auth = vi.hoisted(() => {
|
||||
let epoch = 0;
|
||||
return {
|
||||
get identityEpoch() {
|
||||
return epoch;
|
||||
},
|
||||
get userId() {
|
||||
return 'u1';
|
||||
},
|
||||
get user() {
|
||||
return { id: 'u1', name: 'A' };
|
||||
},
|
||||
get authenticated() {
|
||||
return true;
|
||||
},
|
||||
/** What the real `notifyIdentityChange` does to the epoch. */
|
||||
moveIdentity() {
|
||||
epoch += 1;
|
||||
},
|
||||
reset() {
|
||||
epoch = 0;
|
||||
},
|
||||
identityFence() {
|
||||
const captured = epoch;
|
||||
return () => epoch === captured;
|
||||
},
|
||||
/**
|
||||
* Required because `toast.svelte.ts` subscribes at MODULE LOAD, and the
|
||||
* strip imports it. A double missing this throws before any test runs —
|
||||
* so the double has to model the store's subscription surface, not just
|
||||
* the one method under test.
|
||||
*/
|
||||
onIdentityChange(_fn: (previousUserId: string) => void) {
|
||||
return () => {};
|
||||
},
|
||||
};
|
||||
});
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({ authStore: auth }));
|
||||
|
||||
import ItemAttachmentStrip from './ItemAttachmentStrip.svelte';
|
||||
|
||||
const ATT = {
|
||||
id: 'att-1',
|
||||
filename: 'notes.pdf',
|
||||
mime_type: 'application/pdf',
|
||||
size_bytes: 1024,
|
||||
created_at: '2026-07-20T00:00:00Z',
|
||||
item_id: 'item-1',
|
||||
};
|
||||
|
||||
let target: HTMLElement;
|
||||
let instance: ReturnType<typeof mount> | null = null;
|
||||
|
||||
function deleteButtons(): HTMLButtonElement[] {
|
||||
return Array.from(target.querySelectorAll<HTMLButtonElement>('.att-delete'));
|
||||
}
|
||||
function confirmRow(label: string): HTMLElement | undefined {
|
||||
return Array.from(document.querySelectorAll<HTMLElement>('[role="menu"] [role="menuitem"]')).find(
|
||||
(el) => el.querySelector('.mi-label')?.textContent?.trim() === label
|
||||
);
|
||||
}
|
||||
|
||||
async function settle() {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
flushSync();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
listMock.mockReset();
|
||||
deleteMock.mockReset();
|
||||
deleteMock.mockResolvedValue(undefined);
|
||||
auth.reset();
|
||||
listMock.mockResolvedValue({
|
||||
attachments: [ATT],
|
||||
total: 1,
|
||||
} as unknown as AttachmentListResponse);
|
||||
target = document.body.appendChild(document.createElement('div'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (instance) unmount(instance);
|
||||
instance = null;
|
||||
target.remove();
|
||||
document.querySelectorAll('[role="menu"]').forEach((n) => n.remove());
|
||||
});
|
||||
|
||||
async function mountAndOpenConfirm() {
|
||||
instance = mount(ItemAttachmentStrip, {
|
||||
target,
|
||||
props: {
|
||||
wsSlug: 'ws',
|
||||
username: 'dave',
|
||||
itemId: 'item-1',
|
||||
canDelete: true,
|
||||
itemContent: null,
|
||||
liveContent: null,
|
||||
hostToken: 'host-1',
|
||||
parentArchived: false,
|
||||
mutationsEnabled: true,
|
||||
} as Record<string, unknown>,
|
||||
});
|
||||
flushSync();
|
||||
await settle();
|
||||
const btns = deleteButtons();
|
||||
// PRECONDITION for every absence assertion below: the delete control is
|
||||
// actually reachable. Without this, a strip that rendered no tile at all
|
||||
// would make "no DELETE was sent" pass against the defect.
|
||||
expect(btns.length).toBeGreaterThan(0);
|
||||
btns[0].click();
|
||||
flushSync();
|
||||
expect(confirmRow('Delete file')).toBeTruthy();
|
||||
}
|
||||
|
||||
describe('ItemAttachmentStrip delete confirmation identity fence (BUG-3095 row 9)', () => {
|
||||
it('CONTROL: confirming with the identity unchanged DOES send the DELETE', async () => {
|
||||
await mountAndOpenConfirm();
|
||||
confirmRow('Delete file')!.click();
|
||||
flushSync();
|
||||
await settle();
|
||||
expect(deleteMock).toHaveBeenCalledWith('ws', 'att-1');
|
||||
});
|
||||
|
||||
it('an identity change while the confirmation is OPEN refuses the DELETE', async () => {
|
||||
await mountAndOpenConfirm();
|
||||
|
||||
// The window: the menu is up, the user is replaced, and then the
|
||||
// destructive row is pressed.
|
||||
auth.moveIdentity();
|
||||
flushSync();
|
||||
|
||||
confirmRow('Delete file')!.click();
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
expect(deleteMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('the refusal is the IDENTITY fence, not the navigation fence', async () => {
|
||||
// Neither the workspace nor the item moves in this test, so
|
||||
// `paint.isCurrent()` is true throughout. If the DELETE is still refused,
|
||||
// the only check that could have refused it is the identity one. Without
|
||||
// this leg, a green above would be consistent with the pre-existing
|
||||
// navigation fence having done the work — which would leave the real bug
|
||||
// open, since that fence provably cannot see a user swap.
|
||||
await mountAndOpenConfirm();
|
||||
|
||||
const epochBefore = auth.identityEpoch;
|
||||
auth.moveIdentity();
|
||||
flushSync();
|
||||
expect(auth.identityEpoch).not.toBe(epochBefore);
|
||||
|
||||
confirmRow('Delete file')!.click();
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
expect(deleteMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('the fence is captured per confirmation, not once per component', async () => {
|
||||
// A second `×` click replaces the pending record and must capture afresh.
|
||||
// If the predicate were a component-level field captured at mount, an
|
||||
// identity change would poison every later confirmation for the life of
|
||||
// the strip — including legitimate ones by the new user.
|
||||
await mountAndOpenConfirm();
|
||||
auth.moveIdentity();
|
||||
flushSync();
|
||||
confirmRow('Delete file')!.click();
|
||||
flushSync();
|
||||
await settle();
|
||||
expect(deleteMock).not.toHaveBeenCalled();
|
||||
|
||||
// Now the NEW identity opens its own confirmation and presses it.
|
||||
const btns = deleteButtons();
|
||||
expect(btns.length).toBeGreaterThan(0);
|
||||
btns[0].click();
|
||||
flushSync();
|
||||
confirmRow('Delete file')!.click();
|
||||
flushSync();
|
||||
await settle();
|
||||
expect(deleteMock).toHaveBeenCalledWith('ws', 'att-1');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,575 @@
|
||||
/**
|
||||
* BUG-3095 — the CHILD POPULATION guard (surface 8 of the BUG-3084 family).
|
||||
*
|
||||
* ItemDetail's own fences are gated by `itemDetailIdentityFenceAst.test.ts`.
|
||||
* That guard stops at ItemDetail's file boundary, which is the whole reason
|
||||
* this surface exists: ItemDetail hands callbacks down, and a child that awaits
|
||||
* one and then issues its OWN request is invisible from inside the parent.
|
||||
*
|
||||
* WHAT THIS GUARD ASSERTS, in two independent halves.
|
||||
*
|
||||
* 1. THE POPULATION IS THE ONE WE REVIEWED. `POPULATION` below is checked
|
||||
* against the components ItemDetail actually imports. This half exists
|
||||
* because the hand-written population table on BUG-3095 was probed at
|
||||
* `ea0d0848` and had silently drifted by the time it was worked: six
|
||||
* children had been added to ItemDetail that the table did not list, and one
|
||||
* row named a component ItemDetail no longer mounts. A table nobody can tell
|
||||
* is stale is worse than no table, because it is read as coverage. A child
|
||||
* added to ItemDetail now fails HERE, naming itself, before anyone has to
|
||||
* notice.
|
||||
*
|
||||
* 2. EVERY AWAIT-THEN-REQUEST PATH HOLDS AN IDENTITY FENCE. For each population
|
||||
* file, every async function (and every `.then` continuation) in which a
|
||||
* REQUEST is reachable after an `await` must contain a call to a predicate
|
||||
* bound from `authStore.identityFence()`, positioned between them.
|
||||
*
|
||||
* WHY A SECOND, SIMPLER ANALYZER RATHER THAN REUSING THE AID MODULE.
|
||||
* `src/test/identityFenceAst.ts` is a 1500-line flow analysis tuned to
|
||||
* ItemDetail's specific vocabulary — its generation counters, its capture
|
||||
* helpers, its fence booleans. Pointing it at fifteen other components would
|
||||
* mean teaching it fifteen more vocabularies, and the last six windows on this
|
||||
* family all went the same way: the review rounds are spent on the GUARD's
|
||||
* defects, not the component's. So this one is deliberately small and answers a
|
||||
* narrower question, and the rest is declared below rather than discovered in
|
||||
* review.
|
||||
*
|
||||
* WHAT IT CANNOT SEE — the declared gaps, stated up front (lead ruling on
|
||||
* checkpoint 1b). These are not suspicions; each is a construct this guard
|
||||
* provably does not model:
|
||||
*
|
||||
* (a) PROVENANCE. It matches a CALL to a name bound from
|
||||
* `authStore.identityFence()` in the same function. A predicate arriving
|
||||
* as a prop, or returned by a helper module, reads as unfenced. Accepted
|
||||
* as blind: closing it means tracking values across module boundaries,
|
||||
* which is the aid module's job and its size.
|
||||
* (b) BRANCH OUTCOME. It checks that the predicate is CALLED between the
|
||||
* await and the send, not that the branch it guards actually returns. A
|
||||
* fence whose body is empty passes here. The driven legs are what cover
|
||||
* that — `timelineVersionCardIdentityFence.svelte.test.ts` presses the
|
||||
* real component through the real window.
|
||||
* (c) DEPTH. A send reached through a LOCAL helper is modelled to exactly ONE
|
||||
* hop (`loadTimeline()`, `loadChildren()` — see `requestingHelperNames`),
|
||||
* and a fence reached through a local helper likewise (`stale()`). A
|
||||
* second hop in either direction is not modelled. For sends that is
|
||||
* fail-OPEN, which is why it is stated here rather than implied.
|
||||
* A send inside a callback passed to a helper that is not itself awaited
|
||||
* here also reads as having no await before it.
|
||||
* (d) COMMITS. This guard covers REQUESTS (`api.*`, `fetch`, and one-hop
|
||||
* local helpers around them). A post-await assignment to component state
|
||||
* is equally capable of showing one identity's data to the next, and is
|
||||
* NOT modelled. The two commit rows fixed in this unit
|
||||
* (`TimelineVersionCard.ensureResolved`, `ChildItems.loadChildren`) are
|
||||
* covered by their own fences and by the driven legs, not by this guard.
|
||||
* `ItemPicker.invokeCreate` and `ItemAttachmentStrip`'s post-confirmation
|
||||
* delete are known members of this gap, found by hand.
|
||||
* (e) NON-AWAIT SUSPENSION. `ItemAttachmentStrip.confirmDelete` re-checks on
|
||||
* the far side of a NON-BLOCKING in-app confirmation menu — structurally
|
||||
* the right move, on the wrong quantity (`paint.isCurrent()` compares
|
||||
* `{ws, item}`). There is no `await` there at all, so nothing in this
|
||||
* guard's model applies: a logout and different login while that menu is
|
||||
* open passes the check and the DELETE goes out as the new user. Found by
|
||||
* hand, not by this guard, and it is the clearest evidence that this
|
||||
* guard is a floor and not a ceiling.
|
||||
*
|
||||
* FAIL-CLOSED, which is the property that makes the gaps survivable: anything
|
||||
* this guard cannot classify is a FAILURE, never a skip. A file it cannot parse
|
||||
* fails. A send whose enclosing function it cannot resolve fails. It never
|
||||
* concludes "safe" from not having understood something — so the way this guard
|
||||
* goes wrong is by asking to be taught, which is visible, rather than by going
|
||||
* quiet, which is not.
|
||||
*
|
||||
* KNOWN GAPS ARE ENUMERATED, NOT TOLERATED IN BULK (lead ruling). The paths
|
||||
* this unit did not fix are listed in `KNOWN_UNFENCED` by exact
|
||||
* `file::function` path. Each listed path is EXPECTED to be unfenced: a path
|
||||
* that gets fixed without being removed from the list ALSO fails, so the list
|
||||
* cannot rot into a permanent exemption. Anything unfenced and unlisted fails.
|
||||
* That is what makes this guard mergeable green while the follow-up unit is
|
||||
* still open.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { parseComponent, walk, type AstSource, type Node } from '../../../test/identityFenceAst';
|
||||
|
||||
const ROOT = new URL('../../../', import.meta.url).pathname; // src/
|
||||
|
||||
/**
|
||||
* The child components ItemDetail mounts. Half 1 checks this against the real
|
||||
* import list, so this array cannot silently drift from the component.
|
||||
*
|
||||
* `BottomSheet` is deliberately absent: ItemDetail imports it but mounts it
|
||||
* nowhere (zero `<BottomSheet` occurrences), so it is a dead import rather than
|
||||
* a child. Reported on BUG-3095, not fixed here.
|
||||
*/
|
||||
const POPULATION: string[] = [
|
||||
'lib/components/editor/Editor.svelte',
|
||||
'lib/components/editor/EditorBubbleMenu.svelte',
|
||||
'lib/components/editor/EditorLinkPopover.svelte',
|
||||
'lib/components/editor/RawMarkdownEditor.svelte',
|
||||
'lib/components/fields/FieldEditor.svelte',
|
||||
'lib/components/fields/TagInput.svelte',
|
||||
'lib/components/timeline/ItemTimeline.svelte',
|
||||
'lib/components/timeline/TimelineEntryList.svelte',
|
||||
'lib/components/timeline/TimelineVersionCard.svelte',
|
||||
'lib/components/ChildItems.svelte',
|
||||
'lib/components/BacklinksPanel.svelte',
|
||||
'lib/components/RelationBacklinksPanel.svelte',
|
||||
'lib/components/items/ItemPicker.svelte',
|
||||
'lib/components/items/ItemAttachmentStrip.svelte',
|
||||
'lib/components/attachments/AttachmentSurfaceHost.svelte',
|
||||
'lib/components/common/QuickActionsMenu.svelte',
|
||||
'lib/components/common/Menu.svelte',
|
||||
'lib/components/common/MenuItem.svelte',
|
||||
'lib/components/common/ContentSkeleton.svelte',
|
||||
'lib/components/common/ContentError.svelte',
|
||||
'lib/components/collections/EditCollectionModal.svelte',
|
||||
'lib/components/ShareDialog.svelte',
|
||||
'lib/components/items/CopyItemDialog.svelte',
|
||||
'lib/components/items/PushToAgentDialog.svelte',
|
||||
];
|
||||
|
||||
/**
|
||||
* `TimelineVersionCard` is reached through `ItemTimeline`/`TimelineEntryList`
|
||||
* rather than mounted by ItemDetail directly, so the import check below must
|
||||
* not expect it in ItemDetail's import list. It is in the POPULATION because it
|
||||
* is where the known member lives.
|
||||
*/
|
||||
const NOT_DIRECTLY_IMPORTED = new Set(['lib/components/timeline/TimelineVersionCard.svelte']);
|
||||
|
||||
/**
|
||||
* Paths this unit did not fix, each EXPECTED to be unfenced. Format:
|
||||
* `<population path>::<function name>`. The follow-up unit's contract is
|
||||
* exactly this list; removing an entry without fixing it, or fixing one without
|
||||
* removing it, fails.
|
||||
*/
|
||||
const KNOWN_UNFENCED: string[] = [
|
||||
// ItemTimeline.loadMore: paginates the timeline; the second page request is
|
||||
// issued after the first page's await resolves.
|
||||
'lib/components/timeline/ItemTimeline.svelte::loadMore',
|
||||
// QuickActionsMenu: both send after awaiting an earlier request in the same
|
||||
// handler; handleSaveNewAction is a read-modify-write across three calls.
|
||||
'lib/components/common/QuickActionsMenu.svelte::handleAction',
|
||||
'lib/components/common/QuickActionsMenu.svelte::handleSaveNewAction',
|
||||
// EditCollectionModal.handleArchive: delete → list → delete, each after the
|
||||
// previous await. The worst-shaped of the six: two destructive calls.
|
||||
'lib/components/collections/EditCollectionModal.svelte::handleArchive',
|
||||
// CopyItemDialog.handleConfirm: re-runs the preflight after an earlier await.
|
||||
// Note the copy call itself is NOT retried (no idempotency key) — the fence
|
||||
// here must refuse, never re-send.
|
||||
'lib/components/items/CopyItemDialog.svelte::handleConfirm',
|
||||
// ItemTimeline's six comment/reaction handlers: each writes, then calls
|
||||
// `loadTimeline()` — a local helper that issues `api.timeline.list`. The
|
||||
// write goes out as user A; if the identity moves during that round trip,
|
||||
// the refresh GET goes out on the next user's cookie. These are only
|
||||
// visible through the one-hop helper resolution in `requestingHelperNames`;
|
||||
// a guard matching `api.*` syntactically reports them as having no send.
|
||||
'lib/components/timeline/ItemTimeline.svelte::submitComment',
|
||||
'lib/components/timeline/ItemTimeline.svelte::handleReply',
|
||||
'lib/components/timeline/ItemTimeline.svelte::handleEdit',
|
||||
'lib/components/timeline/ItemTimeline.svelte::handleDelete',
|
||||
'lib/components/timeline/ItemTimeline.svelte::handleReaction',
|
||||
'lib/components/timeline/ItemTimeline.svelte::handleRemoveReaction',
|
||||
// CopyItemDialog.runPreflight: the trailing re-dispatch in its `finally`
|
||||
// re-enters the function, issuing a preflight after the previous await with
|
||||
// no guard at all — deliberately, per its own comment, because the trailing
|
||||
// run re-reads the CURRENT destination. That argument is about which
|
||||
// destination, not about who is asking.
|
||||
'lib/components/items/CopyItemDialog.svelte::runPreflight',
|
||||
// PushToAgentDialog.handleSend: pushes, then refreshes presence after the
|
||||
// await. Guarded by `!destroyed && gen === presenceGen` — instance liveness
|
||||
// plus an opening generation, neither of which sees a user change.
|
||||
'lib/components/items/PushToAgentDialog.svelte::handleSend',
|
||||
// Editor's onMount callback: `api.server.capabilities()` fire-and-forget.
|
||||
// FLAGGED BUT PROBABLY NOT A DEFECT — the endpoint is public and carries no
|
||||
// user-scoped data (its own comment says it works pre-login on shared-item
|
||||
// preview surfaces), so a cross-identity issue leaks nothing. Listed rather
|
||||
// than silently excluded because "this endpoint is public" is a judgement
|
||||
// this guard cannot make, and the follow-up should record the decision
|
||||
// instead of re-deriving it.
|
||||
'lib/components/editor/Editor.svelte::<callback of onMount>',
|
||||
];
|
||||
|
||||
|
||||
function calleeText(src: AstSource, n: Node): string {
|
||||
return src.text(n.callee).replace(/\s+/g, '').replace(/\?\./g, '.');
|
||||
}
|
||||
|
||||
/** A DIRECT REQUEST: an `api.*` call or a bare `fetch(`. */
|
||||
function isDirectRequest(src: AstSource, n: Node): boolean {
|
||||
if (n.type !== 'CallExpression') return false;
|
||||
const c = calleeText(src, n);
|
||||
return c === 'fetch' || c.startsWith('api.') || c.startsWith('apiClient.');
|
||||
}
|
||||
|
||||
/**
|
||||
* Script-level function names whose body issues a direct request — so a CALL to
|
||||
* one of them is itself a send.
|
||||
*
|
||||
* WHY THIS EXISTS, and it was a genuine hole rather than a refinement. The
|
||||
* commonest post-await send in this codebase is not an `api.*` call; it is a
|
||||
* call to the component's own loader: `await api.comments.create(…)` then
|
||||
* `await loadTimeline()`, `await api.items.create(…)` then `loadChildren()`.
|
||||
* Matching only `api.*` syntactically inside the handler misses every one of
|
||||
* them, and the miss is a FALSE NEGATIVE — the guard reports such a path as
|
||||
* having no send at all and stays silent.
|
||||
*
|
||||
* Found by three independent readers auditing the same population by hand while
|
||||
* this guard was already green; `ItemTimeline` alone has six such handlers, none
|
||||
* of which this guard saw. Recorded here rather than quietly fixed because the
|
||||
* lesson is the guard's, not the codebase's: a green source scanner had not been
|
||||
* shown able to go red for the shape that dominates the population.
|
||||
*
|
||||
* ONE hop, matching `fenceHelperNames`: a helper calling a helper that requests
|
||||
* is not modelled and reads as no-send, which is the fail-open direction and is
|
||||
* therefore declared as gap (c) in the header rather than left implicit.
|
||||
*/
|
||||
function requestingHelperNames(src: AstSource): Set<string> {
|
||||
const names = new Set<string>();
|
||||
walk(src.script, (n) => {
|
||||
let fn: Node | null = null;
|
||||
let name = '';
|
||||
if (n.type === 'FunctionDeclaration' && n.id?.type === 'Identifier') {
|
||||
fn = n;
|
||||
name = n.id.name;
|
||||
} else if (n.type === 'VariableDeclarator' && isFnNode(n.init) && n.id?.type === 'Identifier') {
|
||||
fn = n.init;
|
||||
name = n.id.name;
|
||||
}
|
||||
if (!fn || !name) return;
|
||||
let requests = false;
|
||||
walk(fn, (m) => {
|
||||
if (isDirectRequest(src, m)) requests = true;
|
||||
});
|
||||
if (requests) names.add(name);
|
||||
});
|
||||
return names;
|
||||
}
|
||||
|
||||
const isFnNode = (n: Node | undefined | null): n is Node =>
|
||||
!!n &&
|
||||
(n.type === 'ArrowFunctionExpression' ||
|
||||
n.type === 'FunctionExpression' ||
|
||||
n.type === 'FunctionDeclaration');
|
||||
|
||||
/**
|
||||
* A STABLE name for a function, used as the key in `KNOWN_UNFENCED`.
|
||||
*
|
||||
* The fallback deliberately avoids byte offsets. An offset-keyed entry changes
|
||||
* whenever anything ABOVE it in the file is edited, so the gap list would fail
|
||||
* on edits that have nothing to do with it — noise that trains a reader to
|
||||
* re-paste the key without re-reading the path, which is the one thing a
|
||||
* known-gaps list must not teach. Naming an anonymous callback after the call
|
||||
* it is an argument to (`onMount`, `$effect`) is stable under unrelated edits
|
||||
* and still unique in practice; a genuine collision between two anonymous
|
||||
* callbacks of the same callee in one file would merge their entries, which
|
||||
* fails closed (both must be fixed to clear one).
|
||||
*/
|
||||
function fnName(src: AstSource, n: Node, ancestors: Node[]): string {
|
||||
if (n.id?.name) return n.id.name;
|
||||
const parent = ancestors[ancestors.length - 1];
|
||||
if (parent?.type === 'VariableDeclarator' && parent.id?.type === 'Identifier') return parent.id.name;
|
||||
if (parent?.type === 'Property' && parent.key?.type === 'Identifier') return parent.key.name;
|
||||
if (parent?.type === 'CallExpression' && parent.callee) {
|
||||
return `<callback of ${calleeText(src, parent)}>`;
|
||||
}
|
||||
for (let i = ancestors.length - 1; i >= 0; i--) {
|
||||
const a = ancestors[i];
|
||||
if (a.type === 'CallExpression' && a.callee) return `<nested in ${calleeText(src, a)}>`;
|
||||
if (isFnNode(a) && a.id?.name) return `<nested in ${a.id.name}>`;
|
||||
}
|
||||
return '<anonymous>';
|
||||
}
|
||||
|
||||
/**
|
||||
* Names bound from `authStore.identityFence()` inside this function.
|
||||
* Provenance gap (a): only a direct call to that member expression counts.
|
||||
*/
|
||||
function fenceNamesIn(src: AstSource, fn: Node): Set<string> {
|
||||
const names = new Set<string>();
|
||||
walk(fn, (n) => {
|
||||
if (n.type !== 'VariableDeclarator' || !n.init) return;
|
||||
if (n.init.type !== 'CallExpression') return;
|
||||
if (calleeText(src, n.init) !== 'authStore.identityFence') return;
|
||||
if (n.id?.type === 'Identifier') names.add(n.id.name);
|
||||
});
|
||||
return names;
|
||||
}
|
||||
|
||||
/**
|
||||
* Names of local helpers whose body CALLS one of `names` — the `stale()` shape.
|
||||
*
|
||||
* ChildItems does not call its predicate at the send; it folds it into a
|
||||
* `const stale = () => destroyed || !isSameIdentity() || …` declared in the
|
||||
* same function, and calls `stale()` after each await. Without this hop the
|
||||
* guard reports every one of those paths as unfenced, which is how it first
|
||||
* behaved — it flagged the six paths this unit had just fixed.
|
||||
*
|
||||
* ONE hop only, and that bound is deliberate rather than lazy: a helper calling
|
||||
* a helper calling the predicate is not modelled and reads as unfenced, which
|
||||
* fails closed. Widening it is how a source scanner acquires an unbounded tail.
|
||||
*/
|
||||
function fenceHelperNames(src: AstSource, fn: Node, names: Set<string>): Set<string> {
|
||||
const helpers = new Set<string>();
|
||||
if (names.size === 0) return helpers;
|
||||
walk(fn, (n) => {
|
||||
if (n.type !== 'VariableDeclarator' || !isFnNode(n.init)) return;
|
||||
if (n.id?.type !== 'Identifier') return;
|
||||
let calls = false;
|
||||
walk(n.init, (m) => {
|
||||
if (m.type === 'CallExpression' && names.has(calleeText(src, m))) calls = true;
|
||||
});
|
||||
if (calls) helpers.add(n.id.name);
|
||||
});
|
||||
return helpers;
|
||||
}
|
||||
|
||||
/** Offsets at which the predicate — or a one-hop helper around it — is CALLED. */
|
||||
function fenceCallOffsets(src: AstSource, fn: Node, names: Set<string>): number[] {
|
||||
const out: number[] = [];
|
||||
if (names.size === 0) return out;
|
||||
const helpers = fenceHelperNames(src, fn, names);
|
||||
walk(fn, (n) => {
|
||||
if (n.type !== 'CallExpression') return;
|
||||
const c = calleeText(src, n);
|
||||
if (names.has(c) || helpers.has(c)) out.push(n.start);
|
||||
});
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Enclosing loop whose body contains an `await`, if any.
|
||||
*
|
||||
* THE BACK-EDGE CASE, encoded deliberately. `for (const x of xs) { await
|
||||
* send(x); }` has its send TEXTUALLY BEFORE every await in the function, so a
|
||||
* naive "is there an await above this send" reading calls it safe. It is not:
|
||||
* every iteration after the first runs after the previous iteration's await.
|
||||
* ChildItems' two reorder loops are exactly this shape and were the least
|
||||
* guarded paths in the population, so a guard that reads them as fine would
|
||||
* have certified the worst rows.
|
||||
*/
|
||||
/**
|
||||
* True when `a` and `b` sit in DIFFERENT branches of the same `if` or ternary,
|
||||
* and therefore never both run.
|
||||
*
|
||||
* Without this the guard reports a false positive on the common shape
|
||||
* `const res = itemId ? await api.listItem(…) : await api.listCollection(…)`:
|
||||
* the first branch's await is textually before the second branch's send, so a
|
||||
* position-only reading calls the second "after an await". It is not — the two
|
||||
* are alternatives.
|
||||
*
|
||||
* A false positive is fail-closed and would be survivable on its own. It is not
|
||||
* survivable HERE: this guard's findings are the follow-up unit's contract, and
|
||||
* an entry naming a path that is not a defect sends the next reader to fence
|
||||
* something that cannot race. A wrong gap list is worse than a short one.
|
||||
*
|
||||
* `try`/`catch` is deliberately NOT treated as exclusive: a catch block really
|
||||
* does run after the try block's await, which is a genuine await-then-send.
|
||||
*/
|
||||
function branchExclusive(fn: Node, a: Node, b: Node): boolean {
|
||||
const contains = (n: Node | null | undefined, x: Node) => !!n && n.start <= x.start && n.end >= x.end;
|
||||
let exclusive = false;
|
||||
walk(fn, (n) => {
|
||||
if (exclusive) return;
|
||||
if (n.type === 'ConditionalExpression') {
|
||||
if (
|
||||
(contains(n.consequent, a) && contains(n.alternate, b)) ||
|
||||
(contains(n.alternate, a) && contains(n.consequent, b))
|
||||
) {
|
||||
exclusive = true;
|
||||
}
|
||||
} else if (n.type === 'IfStatement') {
|
||||
if (
|
||||
(contains(n.consequent, a) && contains(n.alternate, b)) ||
|
||||
(contains(n.alternate, a) && contains(n.consequent, b))
|
||||
) {
|
||||
exclusive = true;
|
||||
}
|
||||
}
|
||||
});
|
||||
return exclusive;
|
||||
}
|
||||
|
||||
function awaitBearingLoopAncestor(ancestors: Node[]): Node | null {
|
||||
const LOOPS = new Set(['ForStatement', 'ForOfStatement', 'ForInStatement', 'WhileStatement', 'DoWhileStatement']);
|
||||
for (let i = ancestors.length - 1; i >= 0; i--) {
|
||||
const a = ancestors[i];
|
||||
if (!LOOPS.has(a.type)) continue;
|
||||
let hasAwait = false;
|
||||
walk(a.body ?? a, (n) => {
|
||||
if (n.type === 'AwaitExpression') hasAwait = true;
|
||||
});
|
||||
if (hasAwait) return a;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
interface Finding {
|
||||
path: string;
|
||||
fn: string;
|
||||
line: number;
|
||||
callee: string;
|
||||
reason: string;
|
||||
}
|
||||
|
||||
function analyseFile(path: string): { findings: Finding[]; fenced: string[] } {
|
||||
let code: string;
|
||||
try {
|
||||
code = readFileSync(ROOT + path, 'utf8');
|
||||
} catch (e) {
|
||||
// FAIL CLOSED, and cleanly: a POPULATION entry naming a file that is not
|
||||
// there is a finding, not a crash. Raised while mutating this guard — an
|
||||
// ENOENT escaping here reads as a broken test rather than as the guard
|
||||
// reporting a stale population, which is the same confusion the
|
||||
// population check exists to prevent.
|
||||
return {
|
||||
findings: [{ path, fn: '<file>', line: 0, callee: '<read>', reason: `missing file: ${String(e)}` }],
|
||||
fenced: [],
|
||||
};
|
||||
}
|
||||
let src: AstSource;
|
||||
try {
|
||||
src = parseComponent(code);
|
||||
} catch (e) {
|
||||
// FAIL CLOSED: a file we cannot parse is a failure, never a skip.
|
||||
return {
|
||||
findings: [{ path, fn: '<file>', line: 0, callee: '<parse>', reason: `unparseable: ${String(e)}` }],
|
||||
fenced: [],
|
||||
};
|
||||
}
|
||||
|
||||
const findings: Finding[] = [];
|
||||
const fenced: string[] = [];
|
||||
const helperSends = requestingHelperNames(src);
|
||||
const isSend = (n: Node) =>
|
||||
isDirectRequest(src, n) ||
|
||||
(n.type === 'CallExpression' && helperSends.has(calleeText(src, n)));
|
||||
|
||||
walk(src.script, (fn, fnAncestors) => {
|
||||
if (!isFnNode(fn)) return;
|
||||
// Only functions that can suspend are in scope.
|
||||
let hasAwait = false;
|
||||
walk(fn, (n) => {
|
||||
if (n.type === 'AwaitExpression') hasAwait = true;
|
||||
});
|
||||
if (!hasAwait) return;
|
||||
|
||||
const name = fnName(src, fn, fnAncestors);
|
||||
const names = fenceNamesIn(src, fn);
|
||||
const fenceOffsets = fenceCallOffsets(src, fn, names);
|
||||
|
||||
// Every await NODE in this function. Nodes, not offsets, because a send
|
||||
// is usually ITSELF awaited (`const x = await api.foo()`), and that
|
||||
// wrapping AwaitExpression starts before the call it contains. Counting
|
||||
// it as "an await preceding this send" leaves no room for any fence and
|
||||
// reports every awaited request as unfenced — which is exactly how this
|
||||
// guard first behaved, flagging the six paths this unit had just fixed.
|
||||
// A send's own await is not something it can be fenced against.
|
||||
const awaitNodes: Node[] = [];
|
||||
walk(fn, (n) => {
|
||||
if (n.type === 'AwaitExpression') awaitNodes.push(n);
|
||||
});
|
||||
|
||||
walk(fn, (n, ancestors) => {
|
||||
if (!isSend(n)) return;
|
||||
// A nested function's sends belong to that function, not this one.
|
||||
for (const a of ancestors) {
|
||||
if (a !== fn && isFnNode(a)) return;
|
||||
}
|
||||
|
||||
const loop = awaitBearingLoopAncestor([...ancestors, n]);
|
||||
// An await counts as PRECEDING this send only if it does not contain
|
||||
// it — a send's own `await` is not a suspension point it could have
|
||||
// been fenced after.
|
||||
const preceding = awaitNodes.filter(
|
||||
(a) => a.start < n.start && a.end < n.end && !branchExclusive(fn, a, n)
|
||||
);
|
||||
const afterAwait = preceding.length > 0 || loop !== null;
|
||||
if (!afterAwait) return;
|
||||
|
||||
// The fence must be called between the await and the send. In the loop
|
||||
// case there is no "before" in the textual sense — the back edge is the
|
||||
// await — so the requirement is a fence call inside the loop body,
|
||||
// textually above the send.
|
||||
const lowerBound = loop ? loop.start : Math.max(...preceding.map((a) => a.start));
|
||||
const ok = fenceOffsets.some((o) => o > lowerBound && o < n.start);
|
||||
|
||||
const label = `${path}::${name}`;
|
||||
if (ok) {
|
||||
fenced.push(label);
|
||||
} else {
|
||||
findings.push({
|
||||
path,
|
||||
fn: name,
|
||||
line: src.line(n.start),
|
||||
callee: calleeText(src, n),
|
||||
reason:
|
||||
names.size === 0
|
||||
? 'no authStore.identityFence() bound in this function'
|
||||
: 'fence bound but not called between the await and the send',
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
return { findings, fenced };
|
||||
}
|
||||
|
||||
describe('BUG-3095 — child population identity fence', () => {
|
||||
it('the reviewed population is the population ItemDetail actually mounts', () => {
|
||||
const detail = readFileSync(ROOT + 'lib/components/items/ItemDetail.svelte', 'utf8');
|
||||
const imported = new Set<string>();
|
||||
for (const m of detail.matchAll(/import\s+(\w+)\s+from\s+'(\$lib\/[^']+\.svelte)'/g)) {
|
||||
const [, local, spec] = m;
|
||||
// Mounted, not merely imported — a dead import is not a child.
|
||||
if (!new RegExp(`<${local}\\b`).test(detail)) continue;
|
||||
imported.add(spec.replace('$lib/', 'lib/'));
|
||||
}
|
||||
// Relative imports (`./ItemPicker.svelte`) resolve against the items dir.
|
||||
for (const m of detail.matchAll(/import\s+(\w+)\s+from\s+'(\.\/[^']+\.svelte)'/g)) {
|
||||
const [, local, spec] = m;
|
||||
if (!new RegExp(`<${local}\\b`).test(detail)) continue;
|
||||
imported.add('lib/components/items/' + spec.replace('./', ''));
|
||||
}
|
||||
|
||||
const reviewed = new Set(POPULATION.filter((p) => !NOT_DIRECTLY_IMPORTED.has(p)));
|
||||
const missing = [...imported].filter((p) => !reviewed.has(p)).sort();
|
||||
const stale = [...reviewed].filter((p) => !imported.has(p)).sort();
|
||||
|
||||
expect(
|
||||
{ missing, stale },
|
||||
'ItemDetail mounts a child this guard has never reviewed, or POPULATION names one it no longer mounts. ' +
|
||||
'Add it to POPULATION (and review its await-then-request paths) or remove it.'
|
||||
).toEqual({ missing: [], stale: [] });
|
||||
});
|
||||
|
||||
it('every await-then-request path in the population holds an identity fence', () => {
|
||||
const findings: Finding[] = [];
|
||||
for (const path of POPULATION) findings.push(...analyseFile(path).findings);
|
||||
const unlisted = findings
|
||||
.map((f) => ({ ...f, label: `${f.path}::${f.fn}` }))
|
||||
.filter((f) => !KNOWN_UNFENCED.includes(f.label));
|
||||
|
||||
expect(
|
||||
unlisted.map((f) => `${f.label} @${f.line} → ${f.callee} (${f.reason})`),
|
||||
'An await-then-request path with no identity fence. Fence it with ' +
|
||||
'authStore.identityFence(), or — if it belongs to the follow-up unit — ' +
|
||||
'add its exact file::function path to KNOWN_UNFENCED.'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('every KNOWN_UNFENCED entry is still unfenced, so the list cannot rot into an exemption', () => {
|
||||
const findings: Finding[] = [];
|
||||
for (const path of new Set(KNOWN_UNFENCED.map((k) => k.split('::')[0]))) {
|
||||
findings.push(...analyseFile(path).findings);
|
||||
}
|
||||
const stillUnfenced = new Set(findings.map((f) => `${f.path}::${f.fn}`));
|
||||
const fixed = KNOWN_UNFENCED.filter((k) => !stillUnfenced.has(k)).sort();
|
||||
expect(
|
||||
fixed,
|
||||
'These paths are listed as known-unfenced but are now fenced (or gone). ' +
|
||||
'Remove them from KNOWN_UNFENCED — a stale entry is a permanent hole.'
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
/** Exported for the guard's own tests below. */
|
||||
export { analyseFile, POPULATION, KNOWN_UNFENCED };
|
||||
@@ -196,13 +196,27 @@ function codeOf(src: AstSource, fn: Node): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* A separator that cannot occur in the source: the reviewed text is built by
|
||||
* JOINING slices, so the join has to be unambiguous or two different programs
|
||||
* render the same text. Round 10's ASI finding is exactly that — `return`
|
||||
* followed by an expression on the next line is TWO statements whose slices,
|
||||
* joined by a space, are byte-identical to the ONE statement spanning the
|
||||
* same two lines, and `identityHeld` returning `undefined` hashed the same as
|
||||
* `identityHeld` returning the comparison.
|
||||
* A separator that cannot occur at a STATEMENT BOUNDARY — which is the only
|
||||
* place the join ever puts it. The reviewed text is built by JOINING slices,
|
||||
* so the join has to be unambiguous or two different programs render the same
|
||||
* text. Round 10's ASI finding is exactly that — `return` followed by an
|
||||
* expression on the next line is TWO statements whose slices, joined by a
|
||||
* space, are byte-identical to the ONE statement spanning the same two lines,
|
||||
* and `identityHeld` returning `undefined` hashed the same as `identityHeld`
|
||||
* returning the comparison.
|
||||
*
|
||||
* The boundary wording is load-bearing and replaces an earlier claim that this
|
||||
* byte "cannot occur in the source" (BUG-3095). That was false: a NUL is legal
|
||||
* inside a string literal, so it CAN occur in the source. What it cannot do is
|
||||
* appear between two statements, because a statement boundary is not inside a
|
||||
* literal — and a slice join only ever lands there. The weaker, true claim is
|
||||
* the one the collision argument actually needs.
|
||||
*
|
||||
* CAUTION for anyone searching this file: the NUL below makes every byte-
|
||||
* oriented tool classify it as BINARY. The harness `grep` runs with `-I`, so
|
||||
* it returns exit 1 and NO output for patterns that are present here — a
|
||||
* silent false negative indistinguishable from a real absence. Use
|
||||
* `/bin/grep -a … | cat -v` (which renders this byte as `^@`) instead.
|
||||
*/
|
||||
const SEP = ' ';
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
<script lang="ts">
|
||||
import type { Version, Item } from '$lib/types';
|
||||
import { api } from '$lib/api/client';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import DiffView from '$lib/components/versions/DiffView.svelte';
|
||||
import Chip from '$lib/components/common/Chip.svelte';
|
||||
import { relativeTime } from '$lib/utils/markdown';
|
||||
@@ -52,20 +53,30 @@
|
||||
|
||||
async function ensureResolved() {
|
||||
if (!version.is_diff || fetchedContent !== null || resolving) return;
|
||||
// Capture the item identity before the await. This card lives in the
|
||||
// timeline panel that ItemDetail reuses across a no-{#key} item switch
|
||||
// (its itemSlug/wsSlug props change under it), so a lazy version-content
|
||||
// resolve landing after a switch must not write into a stale card
|
||||
// (TASK-2112). `resolving` is a local spinner flag, always cleared.
|
||||
// NAVIGATION fence (TASK-2112): capture the REQUEST identity — the item
|
||||
// and workspace this resolve is about — before the await. This card lives
|
||||
// in the timeline panel that ItemDetail reuses across a no-{#key} item
|
||||
// switch (its itemSlug/wsSlug props change under it), so a lazy
|
||||
// version-content resolve landing after a switch must not write into a
|
||||
// stale card. `resolving` is a local spinner flag, always cleared.
|
||||
const reqSlug = itemSlug;
|
||||
const reqWs = wsSlug;
|
||||
// IDENTITY fence (BUG-3095), a separate question from the one above and
|
||||
// not covered by it: the navigation fence compares slugs, which do not
|
||||
// move when the SIGNED-IN USER changes on the same workspace and item.
|
||||
// The GET is issued before any await here, so it cannot be mis-issued —
|
||||
// what this guards is the COMMIT below, which would otherwise paint one
|
||||
// identity's version text into a card the next identity is reading.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
resolving = true;
|
||||
resolveError = false;
|
||||
try {
|
||||
const full = await api.versions.get(reqWs, reqSlug, version.id);
|
||||
if (!isSameIdentity()) return;
|
||||
if (reqSlug !== itemSlug || reqWs !== wsSlug) return;
|
||||
fetchedContent = full.content;
|
||||
} catch {
|
||||
if (!isSameIdentity()) return;
|
||||
if (reqSlug !== itemSlug || reqWs !== wsSlug) return;
|
||||
resolveError = true;
|
||||
} finally {
|
||||
@@ -94,13 +105,22 @@
|
||||
// Master-freeze guard (TASK-2172): the restore UI is hidden while frozen,
|
||||
// but drop a straggler click so a peeking master never dispatches restore.
|
||||
if (frozen) return;
|
||||
// Capture the item identity before the await so a mid-flight item switch
|
||||
// (rapid j/k / row-click in the split pane) can't fire onRestore with
|
||||
// A's restored item into a parent now showing B — nor flip this card's
|
||||
// confirm state after it's been repurposed (TASK-2112). `restoring` is a
|
||||
// local button flag, always cleared.
|
||||
// NAVIGATION fence (TASK-2112): capture the REQUEST identity — the item
|
||||
// and workspace — before the await, so a mid-flight item switch (rapid
|
||||
// j/k / row-click in the split pane) can't fire onRestore with A's
|
||||
// restored item into a parent now showing B — nor flip this card's
|
||||
// confirm state after it's been repurposed. `restoring` is a local
|
||||
// button flag, always cleared.
|
||||
const reqSlug = itemSlug;
|
||||
const reqWs = wsSlug;
|
||||
// IDENTITY fence (BUG-3095). This is the surface-8 member: the restore
|
||||
// POST below is issued AFTER `flushBeforeRestore`, a parent-provided
|
||||
// await of unbounded duration, so a sign-out or account swap during that
|
||||
// flush sends the restore on the NEXT user's cookie. The navigation
|
||||
// fence above cannot see that — an identity change leaves both slugs
|
||||
// alone — and the pre-existing slug check ran only AFTER the POST had
|
||||
// already gone out, which is too late for a write.
|
||||
const isSameIdentity = authStore.identityFence();
|
||||
restoring = true;
|
||||
try {
|
||||
// BUG-2271: flush the initiating client's live collab editor into
|
||||
@@ -116,7 +136,15 @@
|
||||
} catch {
|
||||
// Swallow — proceed with the restore regardless.
|
||||
}
|
||||
// BUG-3095: BETWEEN the await and the send. A failed flush is
|
||||
// swallowed above, so this is the only thing standing between an
|
||||
// identity change during the flush and a restore issued as the new
|
||||
// user. It must stay on this side of the POST: the slug check below
|
||||
// runs after it, which is the right place to refuse a stale UI
|
||||
// update and the wrong place to refuse a write.
|
||||
if (!isSameIdentity()) return;
|
||||
const updatedItem = await api.versions.restore(reqWs, reqSlug, version.id);
|
||||
if (!isSameIdentity()) return;
|
||||
if (reqSlug !== itemSlug || reqWs !== wsSlug) return;
|
||||
confirming = false;
|
||||
onRestore?.(updatedItem);
|
||||
|
||||
@@ -0,0 +1,243 @@
|
||||
/**
|
||||
* BUG-3095 surface 8 — the DRIVEN leg for the known member.
|
||||
*
|
||||
* `TimelineVersionCard.confirmRestore` awaits `flushBeforeRestore`, a callback
|
||||
* ItemDetail passes down that drains the live collab editor (BUG-2271). That
|
||||
* await is of unbounded duration: it PATCHes the item over the network. If the
|
||||
* signed-in identity changes while it runs, the restore POST issued afterwards
|
||||
* goes out on the NEXT user's cookie.
|
||||
*
|
||||
* WHY A DRIVEN LEG AND NOT ONLY THE AST GUARD. The guard beside this
|
||||
* (`itemDetailIdentityFenceAst.test.ts` and its child-population sibling) proves
|
||||
* a fence is PRESENT in the source. It cannot prove the fence RUNS, that it runs
|
||||
* on the right side of the send, or that its branch actually returns instead of
|
||||
* falling through. This mounts the real card, moves the identity from inside the
|
||||
* flush — the exact window — and asks whether the POST happened.
|
||||
*
|
||||
* WHY THE EXISTING SLUG CHECK IS NOT THIS. `confirmRestore` already captured
|
||||
* `reqWs`/`reqSlug` and compared them after the await. That is a NAVIGATION
|
||||
* fence: neither value moves when a different USER signs in on the same
|
||||
* workspace and item, and it sat AFTER the POST besides, which is the right
|
||||
* place to refuse a stale UI update and the wrong place to refuse a write.
|
||||
*
|
||||
* The auth double is the family's shared reactive one (`identityEpochMock`),
|
||||
* not a local `let epoch = 0` — see that module's header for the defect a
|
||||
* non-reactive double hid through ten source assertions and a 10/10 mutation
|
||||
* matrix.
|
||||
*/
|
||||
import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
import { bindReactiveEpoch, resetEpoch, isEpochReactive } from '../../../test/identityEpochMock.svelte';
|
||||
import type { Version } from '$lib/types';
|
||||
|
||||
const { apiCalls } = vi.hoisted(() => ({ apiCalls: [] as string[] }));
|
||||
|
||||
vi.mock('$lib/api/client', () => ({
|
||||
api: {
|
||||
versions: {
|
||||
restore: vi.fn(async () => {
|
||||
apiCalls.push('restore');
|
||||
return { id: 'i1' };
|
||||
}),
|
||||
get: vi.fn(async () => {
|
||||
apiCalls.push('get');
|
||||
return { content: 'resolved' };
|
||||
}),
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
const auth = vi.hoisted(() => {
|
||||
const hook = { read: null as null | (() => number), write: null as null | ((n: number) => void) };
|
||||
let fallback = 0;
|
||||
const getEpoch = () => (hook.read ? hook.read() : fallback);
|
||||
return {
|
||||
__hook: hook,
|
||||
get identityEpoch() {
|
||||
return getEpoch();
|
||||
},
|
||||
get userId() {
|
||||
return 'u1';
|
||||
},
|
||||
get user() {
|
||||
return { id: 'u1', name: 'A' };
|
||||
},
|
||||
get authenticated() {
|
||||
return true;
|
||||
},
|
||||
/** What the real `notifyIdentityChange` does to the epoch. */
|
||||
moveIdentity() {
|
||||
if (hook.write) hook.write(getEpoch() + 1);
|
||||
else fallback++;
|
||||
},
|
||||
identityFence() {
|
||||
const captured = getEpoch();
|
||||
return () => getEpoch() === captured;
|
||||
},
|
||||
};
|
||||
});
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({ authStore: auth }));
|
||||
|
||||
import TimelineVersionCard from './TimelineVersionCard.svelte';
|
||||
|
||||
const version = {
|
||||
id: 'v1',
|
||||
item_id: 'i1',
|
||||
content: 'hello',
|
||||
is_diff: false,
|
||||
change_summary: 'edited',
|
||||
created_by: 'user',
|
||||
source: 'web',
|
||||
created_at: '2026-07-20T00:00:00Z',
|
||||
} as unknown as Version;
|
||||
|
||||
let root: HTMLElement | null = null;
|
||||
let instance: ReturnType<typeof mount> | null = null;
|
||||
|
||||
/**
|
||||
* Mount the real card, expand it, and press through to the restore.
|
||||
* `flushBeforeRestore` resolves on a promise this returns control of, so the
|
||||
* test decides what happens INSIDE the flush window.
|
||||
*/
|
||||
function renderAndRestore(flush: () => Promise<void>) {
|
||||
root = document.body.appendChild(document.createElement('div'));
|
||||
const restored: unknown[] = [];
|
||||
instance = mount(TimelineVersionCard, {
|
||||
target: root,
|
||||
props: {
|
||||
version,
|
||||
wsSlug: 'ws',
|
||||
itemSlug: 'ITEM-1',
|
||||
currentContent: 'now',
|
||||
onRestore: (item: unknown) => restored.push(item),
|
||||
frozen: false,
|
||||
flushBeforeRestore: flush,
|
||||
},
|
||||
});
|
||||
flushSync();
|
||||
(root.querySelector('.card-header') as HTMLButtonElement).click();
|
||||
flushSync();
|
||||
// The confirm flow is two presses: arm, then confirm.
|
||||
const buttons = () => Array.from(root!.querySelectorAll('button')) as HTMLButtonElement[];
|
||||
const byText = (t: string) =>
|
||||
buttons().find((b) => (b.textContent ?? '').toLowerCase().includes(t));
|
||||
byText('restore')!.click();
|
||||
flushSync();
|
||||
// After arming, the confirm control is the one that is not "cancel".
|
||||
const confirm = buttons().find(
|
||||
(b) =>
|
||||
(b.textContent ?? '').toLowerCase().includes('restore') &&
|
||||
!(b.textContent ?? '').toLowerCase().includes('cancel')
|
||||
);
|
||||
confirm!.click();
|
||||
flushSync();
|
||||
return { restored };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
apiCalls.length = 0;
|
||||
resetEpoch();
|
||||
bindReactiveEpoch(auth.__hook);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (instance) unmount(instance);
|
||||
instance = null;
|
||||
if (root) root.remove();
|
||||
root = null;
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('TimelineVersionCard identity fence (BUG-3095 surface 8)', () => {
|
||||
it('PRECONDITION: the double is reactive, so the legs below measure re-running and not compiling', () => {
|
||||
expect(
|
||||
isEpochReactive(
|
||||
() => auth.identityEpoch,
|
||||
() => auth.moveIdentity()
|
||||
)
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('CONTROL: with the identity unchanged through the flush, the restore POST IS issued', async () => {
|
||||
let release!: () => void;
|
||||
const gate = new Promise<void>((r) => (release = r));
|
||||
renderAndRestore(() => gate);
|
||||
|
||||
// Precondition for the absence assertion in the next test: the restore is
|
||||
// genuinely reachable by this driving. Without this leg, a card that never
|
||||
// restored for an unrelated reason (a selector that matched nothing, a
|
||||
// disabled button) would make that test pass against the defect.
|
||||
expect(apiCalls).not.toContain('restore');
|
||||
|
||||
release();
|
||||
await vi.waitFor(() => expect(apiCalls).toContain('restore'));
|
||||
});
|
||||
|
||||
it('an identity change DURING flushBeforeRestore refuses the restore POST', async () => {
|
||||
let release!: () => void;
|
||||
const gate = new Promise<void>((r) => (release = r));
|
||||
const { restored } = renderAndRestore(() => gate);
|
||||
|
||||
expect(apiCalls).not.toContain('restore');
|
||||
|
||||
// The window this bug is about: the flush is in flight and the user is
|
||||
// replaced under it.
|
||||
auth.moveIdentity();
|
||||
flushSync();
|
||||
release();
|
||||
|
||||
// Give the continuation every chance to run before asserting absence —
|
||||
// an absence assertion made too early passes for the wrong reason.
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
|
||||
expect(apiCalls).not.toContain('restore');
|
||||
// And nothing was reported upward as restored.
|
||||
expect(restored).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('the refusal is the IDENTITY fence, not the pre-existing slug check', async () => {
|
||||
// The slug check compares `reqWs`/`reqSlug` against the live props. This
|
||||
// leg never changes either, so if the restore is still refused, the only
|
||||
// guard that could have refused it is the identity one. Without this, a
|
||||
// green on the test above is consistent with the slug check having done
|
||||
// the work, which would leave the real bug open.
|
||||
let release!: () => void;
|
||||
const gate = new Promise<void>((r) => (release = r));
|
||||
renderAndRestore(() => gate);
|
||||
|
||||
const epochBefore = auth.identityEpoch;
|
||||
auth.moveIdentity();
|
||||
flushSync();
|
||||
expect(auth.identityEpoch).not.toBe(epochBefore);
|
||||
release();
|
||||
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
|
||||
expect(apiCalls).not.toContain('restore');
|
||||
});
|
||||
|
||||
it('a flush that REJECTS still refuses the restore when the identity moved', async () => {
|
||||
// `confirmRestore` swallows a flush failure deliberately (BUG-2271: a
|
||||
// flush failure must not block a user-confirmed restore), so the catch
|
||||
// path reaches the send by a different route than the success path. The
|
||||
// fence has to cover both, and a guard placed inside the `try` around the
|
||||
// flush would cover only one.
|
||||
let reject!: (e: unknown) => void;
|
||||
const gate = new Promise<void>((_r, rj) => (reject = rj));
|
||||
renderAndRestore(() => gate);
|
||||
|
||||
auth.moveIdentity();
|
||||
flushSync();
|
||||
reject(new Error('flush failed'));
|
||||
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
|
||||
expect(apiCalls).not.toContain('restore');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user