diff --git a/backend/__tests__/integration/galleryFilterFeedbackVisibility.test.js b/backend/__tests__/integration/galleryFilterFeedbackVisibility.test.js new file mode 100644 index 00000000..53266fc3 --- /dev/null +++ b/backend/__tests__/integration/galleryFilterFeedbackVisibility.test.js @@ -0,0 +1,221 @@ +/** + * Guest filters must respect show_feedback_to_guests (#1044 follow-up). + * + * Every filter token on /photos is an OR of two halves: what THIS viewer + * marked, and what ANYONE marked. The response fields built from the second + * half — like_count, comment_count, color_label_count — are all gated on + * show_feedback_to_guests. The FILTER was not. + * + * So with the setting off, the numbers were hidden but `?filter=liked` still + * returned exactly the photos other people had liked: the same information as + * a set instead of a count, one token at a time. These tests pin the gate on + * every token, and pin that the viewer's own half is never gated — filtering + * by what you yourself marked is yours to do regardless. + */ + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const jwt = require('jsonwebtoken'); + +const { bootCrmDb, seedMinimal } = require('./helpers/crmDb'); + +process.env.JWT_SECRET = process.env.JWT_SECRET || 'filter-visibility-secret'; + +const SLUG = 'filter-visibility-event'; +const ME = 'guest-me-identifier'; +const SOMEONE_ELSE = 'guest-other-identifier'; + +describe('guest filters and show_feedback_to_guests (#1044)', () => { + let db; + let cleanup; + let app; + let eventId; + let mine; + let theirs; + let myGuestRowId; + + const galleryToken = () => jwt.sign( + { eventId, eventSlug: SLUG, type: 'gallery' }, + process.env.JWT_SECRET, + { expiresIn: '1h', issuer: 'picpeak-auth' } + ); + + const setVisibility = (visible) => db('event_feedback_settings') + .where({ event_id: eventId }) + .update({ show_feedback_to_guests: visible }); + + // A real verified guest, which is how the viewer's own feedback is actually + // identified — NOT the `guest_id` query parameter the frontend invents. + const guestToken = () => jwt.sign( + { type: 'guest', guestId: myGuestRowId, eventId }, + process.env.JWT_SECRET, + { expiresIn: '1h', issuer: 'picpeak-auth' } + ); + + const filter = async (token, { as = 'me', claimGuestId } = {}) => { + const req = request(app) + .get(`/api/gallery/${SLUG}/photos`) + .query({ filter: token, ...(claimGuestId ? { guest_id: claimGuestId } : {}) }) + .set('Authorization', `Bearer ${galleryToken()}`); + if (as === 'me') req.set('x-guest-token', guestToken()); + const res = await req; + expect(res.status).toBe(200); + const photos = Array.isArray(res.body) ? res.body : res.body.photos; + return (photos || []).map((p) => p.id).sort((a, b) => a - b); + }; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + const inserted = await db('events').insert({ + slug: SLUG, + event_type: 'wedding', + event_name: 'Filter Visibility', + event_date: '2026-08-01', + host_email: 'host@example.com', + admin_email: 'admin@example.com', + password_hash: 'x', + share_link: `/gallery/${SLUG}/share`, + share_token: 'filter-visibility-share', + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + created_at: new Date().toISOString(), + }).returning('id'); + eventId = inserted[0]?.id ?? inserted[0]; + + const addPhoto = async (name) => { + const p = await db('photos').insert({ + event_id: eventId, + filename: name, + path: `events/filter/${name}`, + type: 'individual', + uploaded_at: new Date().toISOString(), + }).returning('id'); + return p[0]?.id ?? p[0]; + }; + mine = await addPhoto('mine.jpg'); + theirs = await addPhoto('theirs.jpg'); + + await db('event_feedback_settings').insert({ + event_id: eventId, + feedback_enabled: true, + allow_likes: true, + allow_comments: true, + allow_ratings: true, + allow_favorites: true, + allow_color_labels: true, + moderate_comments: false, + show_feedback_to_guests: true, + }); + + const guestRow = await db('gallery_guests').insert({ + event_id: eventId, + name: 'Me', + identifier: ME, + created_at: new Date().toISOString(), + last_seen_at: new Date().toISOString(), + is_deleted: false, + }).returning('id'); + myGuestRowId = guestRow[0]?.id ?? guestRow[0]; + + const feedback = (photoId, who, type, extra = {}) => db('photo_feedback').insert({ + photo_id: photoId, + event_id: eventId, + guest_identifier: who, + // Submission links to the per-person guest row when one is present, and + // that is the column the viewer's own half resolves through. + guest_id: who === ME ? myGuestRowId : null, + feedback_type: type, + is_approved: true, + is_hidden: false, + created_at: new Date().toISOString(), + ...extra, + }); + + // Everything on `theirs` belongs to somebody else; `mine` is this viewer's. + await feedback(mine, ME, 'like'); + await feedback(theirs, SOMEONE_ELSE, 'like'); + await feedback(theirs, SOMEONE_ELSE, 'favorite'); + await feedback(theirs, SOMEONE_ELSE, 'comment', { comment_text: 'lovely' }); + await feedback(theirs, SOMEONE_ELSE, 'rating', { rating: 5 }); + await feedback(theirs, SOMEONE_ELSE, 'color_label', { color_label: 'green' }); + + // The denormalized counters the aggregate half of the filter reads. + await db('photos').where('id', theirs).update({ + like_count: 1, favorite_count: 1, comment_count: 1, average_rating: 5, color_label_count: 1, + }); + await db('photos').where('id', mine).update({ like_count: 1 }); + + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/gallery', require('../../src/routes/gallery')); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + describe('with feedback visible to guests', () => { + beforeAll(() => setVisibility(true)); + + it('shows other people\'s marks through every token, as before', async () => { + expect(await filter('liked')).toEqual([mine, theirs].sort((a, b) => a - b)); + expect(await filter('favorited')).toEqual([theirs]); + expect(await filter('rated')).toEqual([theirs]); + expect(await filter('commented')).toEqual([theirs]); + expect(await filter('color:green')).toEqual([theirs]); + }); + }); + + describe('with feedback hidden from guests', () => { + beforeAll(() => setVisibility(false)); + + it('stops every token from selecting on other people\'s marks', async () => { + // `theirs` is the photo only other guests marked. It must not come back + // through any token — a filter that selects on hidden feedback reports + // that feedback just as surely as a count would. + expect(await filter('favorited')).toEqual([]); + expect(await filter('rated')).toEqual([]); + expect(await filter('commented')).toEqual([]); + expect(await filter('color:green')).toEqual([]); + }); + + it('still filters by what the viewer marked themselves', async () => { + // The viewer's own half is never gated: this is their own action, and + // hiding it would break "show me the ones I liked" for no privacy gain. + expect(await filter('liked')).toEqual([mine]); + }); + + it('drops the viewer\'s own feedback once an admin hides it', async () => { + // Moderation has to reach the filter too. getPhotoFeedback excludes + // hidden rows for the guest's OWN feedback, so a photo matching here + // would come back with nothing visible on it to explain why. + await db('photo_feedback') + .where({ photo_id: mine, guest_id: myGuestRowId, feedback_type: 'like' }) + .update({ is_hidden: true }); + + expect(await filter('liked')).toEqual([]); + + await db('photo_feedback') + .where({ photo_id: mine, guest_id: myGuestRowId, feedback_type: 'like' }) + .update({ is_hidden: false }); + expect(await filter('liked')).toEqual([mine]); + }); + + it('ignores a guest_id supplied by the caller', async () => { + // The own-half is resolved from the request identity. If it honoured the + // query string instead, anyone holding another guest's identifier could + // read that guest's hidden memberships one token at a time — straight + // back through the gate this file exists to pin. + expect(await filter('favorited', { claimGuestId: SOMEONE_ELSE })).toEqual([]); + expect(await filter('color:green', { claimGuestId: SOMEONE_ELSE })).toEqual([]); + // And an anonymous caller claiming to be me gets nothing of mine. + expect(await filter('liked', { as: 'anon', claimGuestId: ME })).toEqual([]); + }); + }); +}); diff --git a/backend/__tests__/services/photoAdminMarksRowVanishes.test.js b/backend/__tests__/services/photoAdminMarksRowVanishes.test.js new file mode 100644 index 00000000..4ccdc942 --- /dev/null +++ b/backend/__tests__/services/photoAdminMarksRowVanishes.test.js @@ -0,0 +1,142 @@ +/** + * The other direction of the mark race (#1044 follow-up, review of #1137). + * + * setMark reads the row, then writes it. #1137 fixed the case where two calls + * both write — each now writes only the half it was asked about, so neither + * clobbers the other. This is the case where one call CLEARS while another + * SETS: the clear empties the row, the row is deleted for being empty, and the + * setter's update then matches nothing. Its value lands nowhere and the old + * code reported "no mark" — a keystroke lost silently, which is precisely what + * the sibling fix exists to prevent. + * + * The window is inside setMark, between its read and its write, so it cannot + * be driven from outside by ordinary concurrency — natural ordering resolves + * it correctly. The db handle the service captures is wrapped here to delete + * the row at exactly that point, which is the only way to pin the behaviour + * rather than argue about it. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-mark-vanish-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'mark-vanish-secret'; + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +const ADMIN = 11; + +let db; +let cleanup; +let marks; +let eventId; +let photoId; + +/** Deletes the row the next time the service updates it, then lets it proceed. */ +let deleteBeforeNextUpdate = null; + +beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + const inserted = await db('events').insert({ + slug: 'mark-vanish-event', + event_type: 'wedding', + event_name: 'Mark Vanish', + event_date: '2026-07-20', + host_email: 'host@example.com', + admin_email: 'admin@example.com', + password_hash: 'x', + share_link: '/gallery/mark-vanish/share', + share_token: 'mark-vanish-share', + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + created_at: new Date().toISOString(), + }).returning('id'); + eventId = inserted[0]?.id ?? inserted[0]; + + const photo = await db('photos').insert({ + event_id: eventId, + filename: 'vanish.jpg', + path: 'events/vanish/0.jpg', + type: 'individual', + uploaded_at: new Date().toISOString(), + }).returning('id'); + photoId = photo[0]?.id ?? photo[0]; + + // Wrap the handle BEFORE the service is required, since it destructures `db` + // at load time and keeps that reference. + const dbModule = require('../../src/database/db'); + const real = dbModule.db; + dbModule.db = new Proxy(real, { + apply(target, thisArg, args) { + const qb = Reflect.apply(target, thisArg, args); + if (args[0] !== 'photo_admin_marks' || !deleteBeforeNextUpdate) return qb; + const originalUpdate = qb.update.bind(qb); + qb.update = (...updateArgs) => { + if (!deleteBeforeNextUpdate) return originalUpdate(...updateArgs); + const rowId = deleteBeforeNextUpdate; + deleteBeforeNextUpdate = null; + const pending = originalUpdate(...updateArgs); + // The concurrent clear commits here — after the service read the row, + // before its own write runs. + return real('photo_admin_marks').where('id', rowId).delete().then(() => pending); + }; + return qb; + }, + }); + + marks = require('../../src/services/photoAdminMarksService'); +}, 120000); + +afterAll(async () => { + if (cleanup) await cleanup(); +}); + +beforeEach(async () => { + deleteBeforeNextUpdate = null; + await db('photo_admin_marks').where({ photo_id: photoId }).delete(); +}); + +describe('a mark whose row is deleted mid-write', () => { + it('still lands the value instead of reporting no mark', async () => { + await marks.setMark(eventId, photoId, ADMIN, { rating: 3 }); + const row = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: ADMIN }).first(); + + // A concurrent clear will delete this row inside the next setMark's window. + deleteBeforeNextUpdate = row.id; + + const result = await marks.setMark(eventId, photoId, ADMIN, { colorLabel: 'blue' }); + + // Before the fix the update matched zero rows and this came back null, with + // nothing written anywhere — the colour keystroke gone without a trace. + expect(result).toEqual({ rating: null, color_label: 'blue' }); + + const after = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: ADMIN }).first(); + expect(after).toBeTruthy(); + expect(after.color_label).toBe('blue'); + }); + + it('does not resurrect a row when the call was itself a clear', async () => { + await marks.setMark(eventId, photoId, ADMIN, { rating: 3 }); + const row = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: ADMIN }).first(); + + deleteBeforeNextUpdate = row.id; + + // Retrying must not turn "clear it" into "create an empty row". + const result = await marks.setMark(eventId, photoId, ADMIN, { rating: null }); + + expect(result).toBeNull(); + expect(await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: ADMIN }).first()).toBeUndefined(); + }); +}); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 6a0c10fd..4158605d 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -633,7 +633,10 @@ router.get('/:slug/show/:token/state', handleAsync(async (req, res) => { router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) => { try { // Get filter and sort parameters from query - const { filter, guest_id, sort = 'upload_date', order = 'desc' } = req.query; + // `guest_id` is deliberately NOT read from the query string: the viewer's + // own feedback is resolved from the request identity instead (see the + // filter block). The frontend still sends it; it is ignored. + const { filter, sort = 'upload_date', order = 'desc' } = req.query; // Get watermark settings to generate cache-busting version for URLs const watermarkSettings = await watermarkService.getWatermarkSettings(); @@ -692,6 +695,13 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) // Execute the query let photos = hiddenForGuest ? [] : await photosQuery; + // Check if feedback should be visible to guests. Read BEFORE the filter + // block, not after: the filters below consult it, because a filter that + // selects on other people's feedback is a way of reading that feedback. + const feedbackService = require('../services/feedbackService'); + const feedbackSettings = await feedbackService.getEventFeedbackSettings(req.event.id); + const showFeedbackToGuests = isClient || parseBooleanInput(feedbackSettings.show_feedback_to_guests, true); + // Apply filtering if requested (supports global stats + per-guest interactions) if (filter) { const filterTokens = new Set( @@ -721,11 +731,39 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) }); }; + // Whose feedback counts as "mine" for these filters. + // + // Resolved from the REQUEST, the same either/or the per-viewer + // is_liked and my_color_label queries below use — never from the + // `guest_id` query parameter. Two reasons, and both matter now that + // this is the only half left when feedback is hidden: + // + // - It never matched. The frontend's `gallery_guest_id` is a + // localStorage string it invents (`guest__`) and never + // sends when submitting feedback; submissions store + // generateGuestIdentifier(req). So this lookup found nothing, and + // the filters only ever worked through the aggregate half — which + // is exactly the half now gated. + // - It is caller-controlled. Accepting an identifier from the query + // string would let anyone holding someone else's read their hidden + // memberships one token at a time, straight back through the gate. let guestFeedbackByType = null; let guestColorLabels = null; - if (guest_id) { - const guestFeedbackRows = await db('photo_feedback') - .where({ event_id: req.event.id, guest_identifier: guest_id }) + { + // Hidden rows are excluded, matching what the viewer can actually + // SEE: getPhotoFeedback drops is_hidden for the guest's own feedback + // too, so without this a photo could come back under + // `?filter=commented` with no comment visible on it. Unapproved rows + // are NOT excluded — a comment still in the moderation queue is + // still the viewer's own, and the same guest-own read keeps it. + const viewerFeedback = db('photo_feedback') + .where({ event_id: req.event.id, is_hidden: false }); + if (req.guest?.id) { + viewerFeedback.where('guest_id', req.guest.id); + } else { + viewerFeedback.where('guest_identifier', generateGuestIdentifier(req)); + } + const guestFeedbackRows = await viewerFeedback .select('photo_id', 'feedback_type', 'color_label'); guestFeedbackByType = guestFeedbackRows.reduce((acc, row) => { @@ -753,19 +791,33 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) } }; + // Every token below is an OR of two halves: what THIS viewer marked, + // and what ANYONE marked. The second half is other people's feedback, + // so it is gated on show_feedback_to_guests exactly like the counts + // this endpoint returns. + // + // Without the gate the setting only hides the numbers. A guest could + // still send `?filter=liked` and get back precisely the set of photos + // other people liked — the membership, one token at a time, which is + // most of what the counts would have told them. The viewer's own half + // is always theirs to filter by. + const includeAggregate = (predicate) => { + if (showFeedbackToGuests) includeBy(predicate); + }; + if (filterTokens.has('liked')) { includeGuestMatches('like'); - includeBy(photo => (photo.like_count || 0) > 0); + includeAggregate(photo => (photo.like_count || 0) > 0); } if (filterTokens.has('favorited')) { includeGuestMatches('favorite'); - includeBy(photo => (photo.favorite_count || 0) > 0); + includeAggregate(photo => (photo.favorite_count || 0) > 0); } if (filterTokens.has('rated')) { includeGuestMatches('rating'); - includeBy(photo => (photo.average_rating || 0) > 0); + includeAggregate(photo => (photo.average_rating || 0) > 0); } // Colour-label filters (#1044), one token per colour: `color:green`. @@ -778,31 +830,30 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) for (const color of requestedColors) { guestColorLabels?.[color]?.forEach(id => include.add(id)); } - const colorRows = await db('photo_feedback') - .where({ event_id: req.event.id, feedback_type: 'color_label', is_hidden: false }) - .whereIn('color_label', requestedColors) - .select('photo_id'); - colorRows.forEach(row => include.add(row.photo_id)); + if (showFeedbackToGuests) { + const colorRows = await db('photo_feedback') + .where({ event_id: req.event.id, feedback_type: 'color_label', is_hidden: false }) + .whereIn('color_label', requestedColors) + .select('photo_id'); + colorRows.forEach(row => include.add(row.photo_id)); + } } if (filterTokens.has('commented')) { includeGuestMatches('comment'); - const commentedRows = await db('photo_feedback') - .where({ event_id: req.event.id, feedback_type: 'comment', is_approved: true, is_hidden: false }) - .groupBy('photo_id') - .select('photo_id'); - commentedRows.forEach(row => include.add(row.photo_id)); + if (showFeedbackToGuests) { + const commentedRows = await db('photo_feedback') + .where({ event_id: req.event.id, feedback_type: 'comment', is_approved: true, is_hidden: false }) + .groupBy('photo_id') + .select('photo_id'); + commentedRows.forEach(row => include.add(row.photo_id)); + } } photos = photos.filter(photo => include.has(photo.id)); } } - // Check if feedback should be visible to guests - const feedbackService = require('../services/feedbackService'); - const feedbackSettings = await feedbackService.getEventFeedbackSettings(req.event.id); - const showFeedbackToGuests = isClient || parseBooleanInput(feedbackSettings.show_feedback_to_guests, true); - // Then get comment counts separately const commentCounts = await db('photo_feedback') .whereIn('photo_id', photos.map(p => p.id)) diff --git a/backend/src/services/photoAdminMarksService.js b/backend/src/services/photoAdminMarksService.js index 2512945d..5b84eb37 100644 --- a/backend/src/services/photoAdminMarksService.js +++ b/backend/src/services/photoAdminMarksService.js @@ -19,6 +19,9 @@ const { isValidColorLabel } = require('../constants/colorLabels'); * and a reworded string would silently turn a 400 into a 500. */ const INVALID_MARK = 'INVALID_MARK'; + +/** The row we were writing to was deleted by a concurrent call. */ +const ROW_GONE = Symbol('row-gone'); function invalidMark(message) { return Object.assign(new Error(message), { code: INVALID_MARK }); } @@ -66,7 +69,12 @@ async function setMark(eventId, photoId, adminId, { rating, colorLabel } = {}) { if (rating !== undefined) patch.rating = rating === null ? null : Number(rating); if (colorLabel !== undefined) patch.color_label = colorLabel; - await db('photo_admin_marks').where('id', row.id).update(patch); + const updated = await db('photo_admin_marks').where('id', row.id).update(patch); + // Zero rows means a concurrent CLEAR deleted this row between our read and + // this write — the other direction of the same race the patch above + // closes. Our value landed nowhere, so reporting emptiness here would drop + // the keystroke silently. Say so, and let the caller start over. + if (!updated) return ROW_GONE; // A mark with neither half left is deleted, not kept as an empty row — an // empty row would still count as "marked" to anything testing existence. @@ -86,44 +94,59 @@ async function setMark(eventId, photoId, adminId, { rating, colorLabel } = {}) { return { rating: after.rating ?? null, color_label: after.color_label ?? null }; }; - const existing = await db('photo_admin_marks') - .where({ photo_id: photoId, admin_id: adminId }) - .first(); + const insertFresh = async () => { + // No row yet, so there is nothing for a clear-only call to clear. + const fresh = { + rating: rating === undefined || rating === null ? null : Number(rating), + color_label: colorLabel === undefined || colorLabel === null ? null : colorLabel, + }; + if (fresh.rating === null && fresh.color_label === null) return null; - if (existing) return applyToRow(existing); + const now = new Date().toISOString(); + try { + await db('photo_admin_marks').insert({ + photo_id: photoId, + event_id: eventId, + admin_id: adminId, + ...fresh, + created_at: now, + updated_at: now, + }); + } catch (error) { + // The read above and this insert are not atomic, and a triage pass fires + // them back to back — press 4 then 9 on an UNMARKED photo and both + // requests can find no row and both try to insert. The unique index + // stops the duplicate, which would otherwise surface as a 500 and a lost + // keystroke; converge onto the row the winner created, through the same + // write-only-what-you-addressed path as any other update. + const raced = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: adminId }) + .first(); + if (!raced) throw error; + return applyToRow(raced); + } - // No row yet, so there is nothing for a clear-only call to clear. - const fresh = { - rating: rating === undefined || rating === null ? null : Number(rating), - color_label: colorLabel === undefined || colorLabel === null ? null : colorLabel, + return fresh; }; - if (fresh.rating === null && fresh.color_label === null) return null; - const now = new Date().toISOString(); - try { - await db('photo_admin_marks').insert({ - photo_id: photoId, - event_id: eventId, - admin_id: adminId, - ...fresh, - created_at: now, - updated_at: now, - }); - } catch (error) { - // The read above and this insert are not atomic, and a triage pass fires - // them back to back — press 4 then 9 on an UNMARKED photo and both - // requests can find no row and both try to insert. The unique index stops - // the duplicate, which would otherwise surface as a 500 and a lost - // keystroke; converge onto the row the winner created, through the same - // write-only-what-you-addressed path as any other update. - const raced = await db('photo_admin_marks') + // The row can flip between "exists" and "gone" underneath us: a concurrent + // clear deletes it after we read it, or a concurrent set recreates it after + // we find none. Each pass re-reads and acts on what it finds, and only a + // flip sends us round again — so this settles as soon as one write lands. + // Bounded because an unbounded retry on a hot row is a way to hang a + // request, and three flips in one keystroke is not a real sequence. + for (let attempt = 0; attempt < 3; attempt++) { + const existing = await db('photo_admin_marks') .where({ photo_id: photoId, admin_id: adminId }) .first(); - if (!raced) throw error; - return applyToRow(raced); + + const result = existing ? await applyToRow(existing) : await insertFresh(); + if (result !== ROW_GONE) return result; } - return fresh; + // Not silent: losing the keystroke is the failure this whole path exists to + // avoid, so if it somehow cannot land, say so rather than report success. + throw new Error('Could not save mark: the row kept changing underneath'); } /**