Bookdrop: dispatch metadatacopied on save-success, not optimistically (bookshelf-v3bi0) #1435

Merged

zombor merged 2 commits from bd-bookshelf-v3bi0 into main 2026-08-13 00:22:48 +00:00

Conversation 3 Commits 2 Files changed 2 +167 -14

zombor commented 2026-08-12 19:56:22 +00:00

Owner

Copy link

Summary

Code-review MINOR follow-up on merged PR #1424.

_autoSave in bookdrop_file_editor_controller.js dispatched the
bookdrop:metadatacopied CustomEvent unconditionally right after firing the
save request, so the review controller auto-checked the row's checkbox even
when the underlying _doSave() POST later failed — the row read "selected"
though the copy was never persisted server-side.

Fix: _doSave now accepts a dispatchCopyEvent flag (set by _autoSave,
threaded through the in-flight-retry path) and only fires
bookdrop:metadatacopied from _onSaveSuccess — i.e. only after the save
request actually resolves ok: true. On failure (non-ok response or network
error) the event is never dispatched, so the row is never left auto-checked
for an unpersisted copy.

Test plan

• New AUTO-SAVE: bookdrop:metadatacopied dispatch Vitest describe block:
  • dispatch fires once the auto-save succeeds
  • no dispatch when the save returns a non-ok response
  • no dispatch when the save network-errors
  • a plain debounced (non-copy) auto-save success does not dispatch the event
• Updated the pre-existing METADATA COPIED EVENT specs (bookshelf-j8pvj) to
  mock a resolving fetch and await the save's promise chain, since dispatch
  is no longer synchronous.
• npm run coverage — 4752 tests pass, 100% statements/branches/functions/lines.

Closes bead bookshelf-v3bi0 on merge.

## Summary Code-review MINOR follow-up on merged PR #1424. `_autoSave` in `bookdrop_file_editor_controller.js` dispatched the `bookdrop:metadatacopied` CustomEvent unconditionally right after firing the save request, so the review controller auto-checked the row's checkbox even when the underlying `_doSave()` POST later failed — the row read "selected" though the copy was never persisted server-side. Fix: `_doSave` now accepts a `dispatchCopyEvent` flag (set by `_autoSave`, threaded through the in-flight-retry path) and only fires `bookdrop:metadatacopied` from `_onSaveSuccess` — i.e. only after the save request actually resolves `ok: true`. On failure (non-ok response or network error) the event is never dispatched, so the row is never left auto-checked for an unpersisted copy. ## Test plan - New `AUTO-SAVE: bookdrop:metadatacopied dispatch` Vitest describe block: - dispatch fires once the auto-save succeeds - no dispatch when the save returns a non-ok response - no dispatch when the save network-errors - a plain debounced (non-copy) auto-save success does not dispatch the event - Updated the pre-existing `METADATA COPIED EVENT` specs (bookshelf-j8pvj) to mock a resolving `fetch` and await the save's promise chain, since dispatch is no longer synchronous. - `npm run coverage` — 4752 tests pass, 100% statements/branches/functions/lines. Closes bead bookshelf-v3bi0 on merge.

zombor added 1 commit 2026-08-12 19:56:23 +00:00

Bookdrop: dispatch metadatacopied only on save-success (bookshelf-v3bi0) ... f3fa75ce77

_autoSave previously dispatched "bookdrop:metadatacopied" optimistically,
immediately after firing the save request, so the review controller
auto-checked the row even when the underlying POST failed. Move the
dispatch into _doSave's success path (_onSaveSuccess), gated by a
dispatchCopyEvent flag threaded through copyField/copyAll/copyMissing's
_autoSave call, so the row is only marked as "copied" once the copy is
actually persisted server-side.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016tRKybTpfjQ4SxmNdVFLHi

zombor commented 2026-08-12 20:08:06 +00:00

Author

Owner

Copy link

Adversarial security review of PR #1435 (bookshelf-v3bi0).

Scope: static/js/controllers/bookdrop_file_editor_controller.js + its Vitest spec only — the bookdrop:metadatacopied CustomEvent now dispatches on save success instead of optimistically.

Findings:

[MINOR] static/js/controllers/bookdrop_file_editor_controller.js:433 — _pendingDispatchCopy is a per-controller instance flag, not per-field
When a copy-triggered save collides with an in-flight save and a second _doSave call queues behind it (_savePendingAfterInflight), _pendingDispatchCopy is OR'd across whichever calls queued in that window (ctrl._pendingDispatchCopy || !!dispatchCopyEvent). This is a UI-correctness nit (the review row could auto-check slightly earlier/later than the exact field that succeeded), not a security issue — no cross-user or cross-proposal leak, since proposalIdValue on the dispatched event is scoped to the mounted controller's own row. No fix required before merge; flag only if the review-row auto-check behavior is later found flaky under rapid-copy usage.

Security assessment:

• No XSS / unsafe DOM injection: no innerHTML touched; the event detail.proposalId is echoed from an already-rendered proposalIdValue back into a same-page CustomEvent, never re-inserted into markup.
• No per-user/cross-user leak or ownership bypass: this changes only timing of a UI notification for an already-authorized, already-persisted per-request save; it does not change what data is fetched/written or which proposal it targets.
• No secret/token handling in this diff.
• Change is additive/gating only (fires strictly less often — after success, not optimistically) so it cannot newly expose state that wasn't already server-confirmed.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

Adversarial security review of PR #1435 (bookshelf-v3bi0). Scope: `static/js/controllers/bookdrop_file_editor_controller.js` + its Vitest spec only — the `bookdrop:metadatacopied` CustomEvent now dispatches on save success instead of optimistically. Findings: [MINOR] static/js/controllers/bookdrop_file_editor_controller.js:433 — `_pendingDispatchCopy` is a per-controller instance flag, not per-field When a copy-triggered save collides with an in-flight save and a second `_doSave` call queues behind it (`_savePendingAfterInflight`), `_pendingDispatchCopy` is OR'd across whichever calls queued in that window (`ctrl._pendingDispatchCopy || !!dispatchCopyEvent`). This is a UI-correctness nit (the review row could auto-check slightly earlier/later than the exact field that succeeded), not a security issue — no cross-user or cross-proposal leak, since `proposalIdValue` on the dispatched event is scoped to the mounted controller's own row. No fix required before merge; flag only if the review-row auto-check behavior is later found flaky under rapid-copy usage. Security assessment: - No XSS / unsafe DOM injection: no `innerHTML` touched; the event `detail.proposalId` is echoed from an already-rendered `proposalIdValue` back into a same-page CustomEvent, never re-inserted into markup. - No per-user/cross-user leak or ownership bypass: this changes only *timing* of a UI notification for an already-authorized, already-persisted per-request save; it does not change what data is fetched/written or which proposal it targets. - No secret/token handling in this diff. - Change is additive/gating only (fires strictly less often — after success, not optimistically) so it cannot newly expose state that wasn't already server-confirmed. REVIEW VERDICT: 0 blocker, 0 major, 1 minor

zombor commented 2026-08-12 20:08:47 +00:00

Author

Owner

Copy link

Adversarial review of PR #1435 (bookshelf-v3bi0).

[MAJOR] static/js/controllers/bookdrop_file_editor_controller.js:489-495 — network-error retry path drops/leaks the queued dispatchCopyEvent flag, defeating the PR's own guarantee in a race
The .catch() branch (network error) does not mirror the .then() branch's flag handling. Compare:

.then() (lines 480-484, correct):

if (ctrl._savePendingAfterInflight) {
  var nextDispatchCopy = ctrl._pendingDispatchCopy;
  ctrl._savePendingAfterInflight = false;
  ctrl._pendingDispatchCopy = false;
  ctrl._doSave(nextDispatchCopy);
}

.catch() (lines 489-495, buggy):

.catch(function () {
  ctrl._saveInFlight = false;
  ctrl._onSaveError("Network error. Please try again.");
  if (ctrl._savePendingAfterInflight) {
    ctrl._savePendingAfterInflight = false;
    ctrl._doSave();   // <-- no argument, and _pendingDispatchCopy is never reset
  } else {
    ctrl._drainFlushResolvers(false);
  }
});

Two concrete failure modes from this one line:

1. Lost dispatch (silent regression of the review workflow). Scenario: _doSave(true) (a copy-triggered save) is in flight; while it's pending, another _doSave() gets queued behind it and sets _pendingDispatchCopy = true. If the in-flight request network-errors, the .catch() branch replays the queued save via ctrl._doSave() — passing undefined instead of true. The replayed save can still succeed and persist the copied fields, but _onSaveSuccess(payload, undefined) will never dispatch bookdrop:metadatacopied, so the row is never auto-checked even though the copy was persisted server-side.

2. Stale flag causes a spurious future dispatch (reintroduces the exact bug this PR fixes). Because _pendingDispatchCopy is left true (never reset to false in the catch branch, unlike the then branch), the next unrelated _doSave() call that happens to race with an in-flight save will OR against that stale true (line 433: ctrl._pendingDispatchCopy = ctrl._pendingDispatchCopy || !!dispatchCopyEvent) and inherit it. A later plain field-edit autosave (not a copy action at all) can then end up dispatching bookdrop:metadatacopied on success — silently auto-checking a row for a save that was never a copy. This is precisely the "row reads as selected but wasn't a real copy" failure class bookshelf-v3bi0 was filed to eliminate, reintroduced via the untested retry path.

Neither path is covered by the new tests — all four new specs in bookdrop_file_editor_controller.test.js use a fetch mock that resolves on the very first call with no queued follow-up save, so the in-flight-queueing branch (lines 432-433, 489-495) is never exercised by any add­ed test.

Fix: mirror the .then() branch's handling in .catch():

.catch(function () {
  ctrl._saveInFlight = false;
  ctrl._onSaveError("Network error. Please try again.");
  if (ctrl._savePendingAfterInflight) {
    var nextDispatchCopy = ctrl._pendingDispatchCopy;
    ctrl._savePendingAfterInflight = false;
    ctrl._pendingDispatchCopy = false;
    ctrl._doSave(nextDispatchCopy);
  } else {
    ctrl._drainFlushResolvers(false);
  }
});

Add a Vitest spec that queues a copy-triggered _doSave(true) behind an in-flight save whose fetch rejects, then resolves the replayed save ok:true, and asserts the event fires exactly once (proves fix #1) — plus a spec proving a later unrelated plain save does NOT dispatch after that network-error race (proves fix #2, i.e. _pendingDispatchCopy is reset).

[MINOR] static/js/test/bookdrop_file_editor_controller.test.js:848-892 — new specs only prove "eventually dispatches/doesn't," not "never fires synchronously before the fetch resolves"
The four new specs (and the updated METADATA COPIED EVENT specs) all await Promise.resolve() three times before asserting, so they can't distinguish "dispatch moved into the resolved-fetch .then()" from a hypothetical implementation that dispatches synchronously at click time but the assertion happens to run after. The failure-path specs (fetch resolves ok:false / rejects) are still a legitimate regression test for the original bug (an unconditional/optimistic dispatch would have fired regardless of outcome, failing those two not.toHaveBeenCalled() assertions), so this is not vacuous — but a stronger spec would additionally assert handler has NOT been called synchronously right after the click, before the first await, to positively pin "no dispatch before the request resolves" rather than inferring it only from the failure-path specs. Non-blocking; existing tests are far from tautological/self-validating.

REVIEW VERDICT: 0 blocker, 1 major, 1 minor

Adversarial review of PR #1435 (bookshelf-v3bi0). [MAJOR] static/js/controllers/bookdrop_file_editor_controller.js:489-495 — network-error retry path drops/leaks the queued dispatchCopyEvent flag, defeating the PR's own guarantee in a race The `.catch()` branch (network error) does not mirror the `.then()` branch's flag handling. Compare: `.then()` (lines 480-484, correct): ``` if (ctrl._savePendingAfterInflight) { var nextDispatchCopy = ctrl._pendingDispatchCopy; ctrl._savePendingAfterInflight = false; ctrl._pendingDispatchCopy = false; ctrl._doSave(nextDispatchCopy); } ``` `.catch()` (lines 489-495, buggy): ``` .catch(function () { ctrl._saveInFlight = false; ctrl._onSaveError("Network error. Please try again."); if (ctrl._savePendingAfterInflight) { ctrl._savePendingAfterInflight = false; ctrl._doSave(); // <-- no argument, and _pendingDispatchCopy is never reset } else { ctrl._drainFlushResolvers(false); } }); ``` Two concrete failure modes from this one line: 1. **Lost dispatch (silent regression of the review workflow).** Scenario: `_doSave(true)` (a copy-triggered save) is in flight; while it's pending, another `_doSave()` gets queued behind it and sets `_pendingDispatchCopy = true`. If the in-flight request network-errors, the `.catch()` branch replays the queued save via `ctrl._doSave()` — passing `undefined` instead of `true`. The replayed save can still succeed and persist the copied fields, but `_onSaveSuccess(payload, undefined)` will never dispatch `bookdrop:metadatacopied`, so the row is never auto-checked even though the copy *was* persisted server-side. 2. **Stale flag causes a spurious future dispatch (reintroduces the exact bug this PR fixes).** Because `_pendingDispatchCopy` is left `true` (never reset to `false` in the catch branch, unlike the then branch), the *next* unrelated `_doSave()` call that happens to race with an in-flight save will OR against that stale `true` (line 433: `ctrl._pendingDispatchCopy = ctrl._pendingDispatchCopy || !!dispatchCopyEvent`) and inherit it. A later plain field-edit autosave (not a copy action at all) can then end up dispatching `bookdrop:metadatacopied` on success — silently auto-checking a row for a save that was never a copy. This is precisely the "row reads as selected but wasn't a real copy" failure class bookshelf-v3bi0 was filed to eliminate, reintroduced via the untested retry path. Neither path is covered by the new tests — all four new specs in `bookdrop_file_editor_controller.test.js` use a fetch mock that resolves on the very first call with no queued follow-up save, so the in-flight-queueing branch (lines 432-433, 489-495) is never exercised by any add­ed test. **Fix:** mirror the `.then()` branch's handling in `.catch()`: ```js .catch(function () { ctrl._saveInFlight = false; ctrl._onSaveError("Network error. Please try again."); if (ctrl._savePendingAfterInflight) { var nextDispatchCopy = ctrl._pendingDispatchCopy; ctrl._savePendingAfterInflight = false; ctrl._pendingDispatchCopy = false; ctrl._doSave(nextDispatchCopy); } else { ctrl._drainFlushResolvers(false); } }); ``` Add a Vitest spec that queues a copy-triggered `_doSave(true)` behind an in-flight save whose fetch rejects, then resolves the replayed save `ok:true`, and asserts the event fires exactly once (proves fix #1) — plus a spec proving a later unrelated plain save does NOT dispatch after that network-error race (proves fix #2, i.e. `_pendingDispatchCopy` is reset). [MINOR] static/js/test/bookdrop_file_editor_controller.test.js:848-892 — new specs only prove "eventually dispatches/doesn't," not "never fires synchronously before the fetch resolves" The four new specs (and the updated METADATA COPIED EVENT specs) all `await Promise.resolve()` three times before asserting, so they can't distinguish "dispatch moved into the resolved-fetch `.then()`" from a hypothetical implementation that dispatches synchronously at click time but the assertion happens to run after. The failure-path specs (fetch resolves `ok:false` / rejects) are still a legitimate regression test for the original bug (an unconditional/optimistic dispatch would have fired regardless of outcome, failing those two `not.toHaveBeenCalled()` assertions), so this is not vacuous — but a stronger spec would additionally assert `handler` has NOT been called synchronously right after the click, before the first `await`, to positively pin "no dispatch before the request resolves" rather than inferring it only from the failure-path specs. Non-blocking; existing tests are far from tautological/self-validating. REVIEW VERDICT: 0 blocker, 1 major, 1 minor

zombor commented 2026-08-12 20:09:11 +00:00

Author

Owner

Copy link

Adversarial review of PR #1435 (bookshelf-v3bi0).

[MAJOR] static/js/controllers/bookdrop_file_editor_controller.js:489-495 — network-error retry path drops/leaks the queued dispatchCopyEvent flag, defeating the PR's own guarantee in a race
The .catch() branch (network error) does not mirror the .then() branch's flag handling. Compare:

.then() (lines 480-484, correct):

if (ctrl._savePendingAfterInflight) {
  var nextDispatchCopy = ctrl._pendingDispatchCopy;
  ctrl._savePendingAfterInflight = false;
  ctrl._pendingDispatchCopy = false;
  ctrl._doSave(nextDispatchCopy);
}

.catch() (lines 489-495, buggy):

.catch(function () {
  ctrl._saveInFlight = false;
  ctrl._onSaveError("Network error. Please try again.");
  if (ctrl._savePendingAfterInflight) {
    ctrl._savePendingAfterInflight = false;
    ctrl._doSave();   // <-- no argument, and _pendingDispatchCopy is never reset
  } else {
    ctrl._drainFlushResolvers(false);
  }
});

Two concrete failure modes from this one line:

1. Lost dispatch (silent regression of the review workflow). Scenario: _doSave(true) (a copy-triggered save) is in flight; while it's pending, another _doSave() gets queued behind it and sets _pendingDispatchCopy = true. If the in-flight request network-errors, the .catch() branch replays the queued save via ctrl._doSave() — passing undefined instead of true. The replayed save can still succeed and persist the copied fields, but _onSaveSuccess(payload, undefined) will never dispatch bookdrop:metadatacopied, so the row is never auto-checked even though the copy was persisted server-side.

2. Stale flag causes a spurious future dispatch (reintroduces the exact bug this PR fixes). Because _pendingDispatchCopy is left true (never reset to false in the catch branch, unlike the then branch), the next unrelated _doSave() call that happens to race with an in-flight save will OR against that stale true (line 433: ctrl._pendingDispatchCopy = ctrl._pendingDispatchCopy || !!dispatchCopyEvent) and inherit it. A later plain field-edit autosave (not a copy action at all) can then end up dispatching bookdrop:metadatacopied on success — silently auto-checking a row for a save that was never a copy. This is precisely the "row reads as selected but wasn't a real copy" failure class bookshelf-v3bi0 was filed to eliminate, reintroduced via the untested retry path.

Neither path is covered by the new tests — all four new specs in bookdrop_file_editor_controller.test.js use a fetch mock that resolves on the very first call with no queued follow-up save, so the in-flight-queueing branch (lines 432-433, 489-495) is never exercised by any added test.

Fix: mirror the .then() branch's handling in .catch():

.catch(function () {
  ctrl._saveInFlight = false;
  ctrl._onSaveError("Network error. Please try again.");
  if (ctrl._savePendingAfterInflight) {
    var nextDispatchCopy = ctrl._pendingDispatchCopy;
    ctrl._savePendingAfterInflight = false;
    ctrl._pendingDispatchCopy = false;
    ctrl._doSave(nextDispatchCopy);
  } else {
    ctrl._drainFlushResolvers(false);
  }
});

Add a Vitest spec that queues a copy-triggered _doSave(true) behind an in-flight save whose fetch rejects, then resolves the replayed save ok:true, and asserts the event fires exactly once (proves fix #1) — plus a spec proving a later unrelated plain save does NOT dispatch after that network-error race (proves fix #2, i.e. _pendingDispatchCopy is reset).

[MINOR] static/js/test/bookdrop_file_editor_controller.test.js:848-892 — new specs only prove "eventually dispatches/doesn't," not "never fires synchronously before the fetch resolves"
The four new specs (and the updated METADATA COPIED EVENT specs) all await Promise.resolve() three times before asserting, so they can't distinguish "dispatch moved into the resolved-fetch .then()" from a hypothetical implementation that dispatches synchronously at click time but the assertion happens to run after. The failure-path specs (fetch resolves ok:false / rejects) are still a legitimate regression test for the original bug (an unconditional/optimistic dispatch would have fired regardless of outcome, failing those two not.toHaveBeenCalled() assertions), so this is not vacuous — but a stronger spec would additionally assert handler has NOT been called synchronously right after the click, before the first await, to positively pin "no dispatch before the request resolves" rather than inferring it only from the failure-path specs. Non-blocking; existing tests are far from tautological/self-validating.

REVIEW VERDICT: 0 blocker, 1 major, 1 minor

Adversarial review of PR #1435 (bookshelf-v3bi0). [MAJOR] static/js/controllers/bookdrop_file_editor_controller.js:489-495 — network-error retry path drops/leaks the queued dispatchCopyEvent flag, defeating the PR's own guarantee in a race The `.catch()` branch (network error) does not mirror the `.then()` branch's flag handling. Compare: `.then()` (lines 480-484, correct): ``` if (ctrl._savePendingAfterInflight) { var nextDispatchCopy = ctrl._pendingDispatchCopy; ctrl._savePendingAfterInflight = false; ctrl._pendingDispatchCopy = false; ctrl._doSave(nextDispatchCopy); } ``` `.catch()` (lines 489-495, buggy): ``` .catch(function () { ctrl._saveInFlight = false; ctrl._onSaveError("Network error. Please try again."); if (ctrl._savePendingAfterInflight) { ctrl._savePendingAfterInflight = false; ctrl._doSave(); // <-- no argument, and _pendingDispatchCopy is never reset } else { ctrl._drainFlushResolvers(false); } }); ``` Two concrete failure modes from this one line: 1. **Lost dispatch (silent regression of the review workflow).** Scenario: `_doSave(true)` (a copy-triggered save) is in flight; while it's pending, another `_doSave()` gets queued behind it and sets `_pendingDispatchCopy = true`. If the in-flight request network-errors, the `.catch()` branch replays the queued save via `ctrl._doSave()` — passing `undefined` instead of `true`. The replayed save can still succeed and persist the copied fields, but `_onSaveSuccess(payload, undefined)` will never dispatch `bookdrop:metadatacopied`, so the row is never auto-checked even though the copy *was* persisted server-side. 2. **Stale flag causes a spurious future dispatch (reintroduces the exact bug this PR fixes).** Because `_pendingDispatchCopy` is left `true` (never reset to `false` in the catch branch, unlike the then branch), the *next* unrelated `_doSave()` call that happens to race with an in-flight save will OR against that stale `true` (line 433: `ctrl._pendingDispatchCopy = ctrl._pendingDispatchCopy || !!dispatchCopyEvent`) and inherit it. A later plain field-edit autosave (not a copy action at all) can then end up dispatching `bookdrop:metadatacopied` on success — silently auto-checking a row for a save that was never a copy. This is precisely the "row reads as selected but wasn't a real copy" failure class bookshelf-v3bi0 was filed to eliminate, reintroduced via the untested retry path. Neither path is covered by the new tests — all four new specs in `bookdrop_file_editor_controller.test.js` use a fetch mock that resolves on the very first call with no queued follow-up save, so the in-flight-queueing branch (lines 432-433, 489-495) is never exercised by any added test. **Fix:** mirror the `.then()` branch's handling in `.catch()`: ```js .catch(function () { ctrl._saveInFlight = false; ctrl._onSaveError("Network error. Please try again."); if (ctrl._savePendingAfterInflight) { var nextDispatchCopy = ctrl._pendingDispatchCopy; ctrl._savePendingAfterInflight = false; ctrl._pendingDispatchCopy = false; ctrl._doSave(nextDispatchCopy); } else { ctrl._drainFlushResolvers(false); } }); ``` Add a Vitest spec that queues a copy-triggered `_doSave(true)` behind an in-flight save whose fetch rejects, then resolves the replayed save `ok:true`, and asserts the event fires exactly once (proves fix #1) — plus a spec proving a later unrelated plain save does NOT dispatch after that network-error race (proves fix #2, i.e. `_pendingDispatchCopy` is reset). [MINOR] static/js/test/bookdrop_file_editor_controller.test.js:848-892 — new specs only prove "eventually dispatches/doesn't," not "never fires synchronously before the fetch resolves" The four new specs (and the updated METADATA COPIED EVENT specs) all await Promise.resolve() three times before asserting, so they can't distinguish "dispatch moved into the resolved-fetch .then()" from a hypothetical implementation that dispatches synchronously at click time but the assertion happens to run after. The failure-path specs (fetch resolves ok:false / rejects) are still a legitimate regression test for the original bug (an unconditional/optimistic dispatch would have fired regardless of outcome, failing those two not.toHaveBeenCalled() assertions), so this is not vacuous — but a stronger spec would additionally assert handler has NOT been called synchronously right after the click, before the first await, to positively pin "no dispatch before the request resolves" rather than inferring it only from the failure-path specs. Non-blocking; existing tests are far from tautological/self-validating. REVIEW VERDICT: 0 blocker, 1 major, 1 minor

zombor added 1 commit 2026-08-12 20:11:55 +00:00

fix(review): mirror flag threading in catch-path replay (bookshelf-v3bi0) ... 87b209cf63

The .then() success branch correctly captures _pendingDispatchCopy into a
local before resetting it and replaying a queued save. The .catch() network-
error branch called _doSave() with no argument and never reset
_pendingDispatchCopy, so a queued copy-triggered save replayed after a
network error lost its dispatch intent, and the stale flag could leak into
an unrelated later save. Mirror the .then() branch's handling exactly.

Adds a Vitest spec that queues a copy-triggered save behind an in-flight
save whose first fetch rejects, asserting the replay still dispatches
bookdrop:metadatacopied exactly once and a later unrelated save does not
re-dispatch. Confirmed RED against the unfixed .catch() before applying the
fix. Also pins a synchronous "no dispatch yet" assertion right after the
click, before the first await.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016tRKybTpfjQ4SxmNdVFLHi

zombor referenced this pull request 2026-08-12 20:19:06 +00:00

nnb9: black-box internal/library tests (bookshelf-nnb9.13) #1439

zombor referenced this pull request 2026-08-12 20:26:43 +00:00

nnb9: black-box internal/stats tests (bookshelf-nnb9.29) #1442

zombor referenced this pull request 2026-08-12 20:37:30 +00:00

fix(db): wrap ctx.Err() + soften doc comment (bookshelf-adzpx) #1436

zombor merged commit 13a98cd44c into main 2026-08-13 00:22:48 +00:00

zombor referenced this pull request from a commit 2026-08-13 00:22:54 +00:00

Merge pull request 'Bookdrop: dispatch metadatacopied on save-success, not optimistically (bookshelf-v3bi0)' (#1435) from bd-bookshelf-v3bi0 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1435

No description provided.