[nwparker]

What

Three independent data-loss/data-leak defects, each in code added by an earlier fix:

1. Mobile notification loss on a failed catch-up (mobile/src/notifications/mobile-notifications.ts:123 on main, from fix(mobile): deliver the notifications a reconnect missed, and never persist a watermark past them #10816). fetchMissed collapsed an RPC failure into an empty batch (.catch(() => [])), so an abandoned catch-up looked identical to "nothing missed". Live delivery kept advancing and persisting lastDeliveredSeq, and because the desktop cuts by seq > lastSeenSeq, the next catch-up asked from above the abandoned range — those notifications were never replayed. A per-session quarantine now freezes the persisted watermark at the seq the failed catch-up asked from until some later catch-up actually drains. Review then surfaced a second half of the same loss: seen was marked before delivery, so a rejected showLocalNotification left the key behind and every later replay of that event was dropped as a duplicate — the quarantine held the range, but the notification inside it could never be re-shown. seen is now marked after the show lands, on the same rule as the watermark.
2. Jira attachment cache repopulated with credentialed bytes after a disconnect (src/main/jira/attachment-image-cache.ts:26 on main, from fix(jira): render issue description/comment images with lightbox #8938). The staleness guard compared siteEpoch + cacheEpoch, so distinct clear states could sum to the same number (site at 1 before a global clear equals global 1 after it). An in-flight download that spanned a per-site clear followed by "Disconnect all" passed the guard and re-inserted image bytes fetched with a now-revoked token. Replaced with one monotonic ticker read via Math.max, so distinct clear states can never be equal.
3. Web-client copy pasting the terminal selection instead of the requested text (src/renderer/src/web/web-clipboard-copy-fallback.ts:18 on main, from fix(terminal): make copy work in the HTTP web client #10534). The execCommand fallback registered its copy listener in the capture phase; xterm's own handler is bubble-phase on terminal.element and overwrote text/plain with the terminal selection afterwards (preventDefault does not stop propagation). Switched to bubble phase plus stopImmediatePropagation().

ELI5

Three separate things could quietly hand you the wrong data. On your phone, if the app lost its connection at the exact moment it was asking the desktop "what did I miss?", it would give up on that batch but still move its bookmark forward — so those notifications were skipped for good and you never learned about them. In the Jira panel, an image that was still downloading when you disconnected your account could land in the cache anyway, meaning pictures fetched with credentials you had just revoked stayed in memory and could be shown again. And in the browser version of Orca, clicking something like "Copy Path" while text was highlighted in a terminal would copy the highlighted terminal text instead — the app said "Copied!", but pasting gave you whatever was selected in the terminal. There was also a subtler version of the phone problem: the app ticked a notification off its "already handled" list the instant it started showing it, so if showing it then failed, the app spent forever re-asking for a notification it would keep throwing away as a repeat. Now the phone keeps asking from the point it actually stopped at until it truly catches up, only ticks a notification off once it has really appeared, the disconnected account's images are dropped, and copy copies what you asked for.

Proof of fix

1. Mobile notification loss (mobile/, vitest)

Source files reverted to origin/main, new test kept at HEAD:

$ git checkout origin/main -- mobile/src/notifications/mobile-notifications.ts \
    mobile/src/notifications/notification-reconnect-catchup.ts
$ cd mobile && npx vitest run --reporter=verbose \
    src/notifications/notification-catchup-failure-quarantine.test.ts

 FAIL  ... > keeps asking from the abandoned range until a catch-up actually succeeds
AssertionError: expected 11 to be 5 // Object.is equality
 ❯ src/notifications/notification-catchup-failure-quarantine.test.ts:143:28

 FAIL  ... > rolls back a watermark a live event stored while the catch-up was in flight
AssertionError: expected 11 to be 5 // Object.is equality
 ❯ src/notifications/notification-catchup-failure-quarantine.test.ts:191:28

 FAIL  ... > quarantines at the last replayed seq when a teardown cuts the batch short
AssertionError: expected 20 to be 6 // Object.is equality

 FAIL  ... > re-shows a replay whose show threw, instead of dropping it as already seen
AssertionError: expected [ 'm6', 'm7' ] to deeply equal [ 'm6', 'm6', 'm7' ]

 FAIL  ... > re-shows a live event whose show threw, instead of dropping it as already seen
AssertionError: expected [ 'm6' ] to deeply equal [ 'm6', 'm6' ]

 Test Files  1 failed (1)
      Tests  5 failed (5)
     Errors  2 errors

The two Errors are the unhandled rejections the escaping show produced (vitest exits 1 on them);
both queue boundaries now contain it.

With the fix:

 ✓ ... > keeps asking from the abandoned range until a catch-up actually succeeds 68ms
 ✓ ... > rolls back a watermark a live event stored while the catch-up was in flight 33ms
 ✓ ... > quarantines at the last replayed seq when a teardown cuts the batch short 33ms
 ✓ ... > re-shows a replay whose show threw, instead of dropping it as already seen 23ms
 ✓ ... > re-shows a live event whose show threw, instead of dropping it as already seen 34ms

 Test Files  1 passed (1)
      Tests  5 passed (5)

2 & 3. Jira cache and web clipboard (desktop vitest)

Source files reverted to origin/main, new tests kept at HEAD:

$ git checkout origin/main -- src/main/jira/attachment-image-cache.ts \
    src/renderer/src/web/web-clipboard-copy-fallback.ts
$ npx vitest run --config config/vitest.config.ts --reporter=verbose \
    src/main/jira/attachment-image-cache.test.ts \
    src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts

 FAIL  src/main/jira/attachment-image-cache.test.ts > attachment image cache >
       does not repopulate after "disconnect all" when the site was cleared before
AssertionError: expected 'data:image/png;base64,SECRET==' to be null
 ❯ src/main/jira/attachment-image-cache.test.ts:84:55

 FAIL  src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts >
       copies the requested text even when the selection anchor is inside a terminal
AssertionError: expected 'rm -rf ./secret-dir' to be '/Users/me/repo/src/index.ts'
 ❯ src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts:74:49

 FAIL  src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts >
       wins over a copy handler that still runs after the document
AssertionError: expected 'later window handler' to be 'pane-42'
 ❯ src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts:92:51

 Test Files  2 failed (2)
      Tests  3 failed | 3 passed (6)

With the fix:

 Test Files  2 passed (2)
      Tests  6 passed (6)

Clipboard: visual before/after

Read these two shots with the caveat below. They are real captures, but of a purpose-built page, not the shipped Orca web client. What is genuine: real Chromium driven by Playwright, real @xterm/xterm from this repo's node_modules, a genuinely insecure origin (served over plain HTTP on a LAN IP, with navigator.clipboard === undefined rendered on the page — 127.0.0.1 would be a secure context and would not exercise this path), a real mouse-drag selection via CDP, the unmodified copyClipboardTextViaExecCommand imported from src/, and a real execCommand('copy') followed by a real paste. The two runs differ only by git stash / git stash pop of web-clipboard-copy-fallback.ts: same viewport, same URL, same drag coordinates, same click sequence, one shared capture script. The diagnostics block is identical in both shots (xterm hasSelection: true, dom selection anchor: xterm-helpers, dom selection inside terminal: true, copy reported: true), so the only delta is the pasted text. The harness was deleted before committing and is not in this diff.

Proof of no regression

New regression tests

• mobile/src/notifications/notification-catchup-failure-quarantine.test.ts — 5 cases. Fails on parent (5 failed + 2 unhandled rejections), passes with the fix (5 passed, 0 errors); output above. The last two are the review finding: one drives a rejecting show inside a replay batch, the other a rejecting show on the pure live path with no catch-up failure at all. It drives the real subscribeToDesktopNotifications across two subscription lifetimes (matching the app/index.tsx teardown/resubscribe cycle) with a mocked RpcClient and AsyncStorage, and asserts the persisted watermark directly.
• src/main/jira/attachment-image-cache.test.ts — added does not repopulate after "disconnect all" when the site was cleared before (the defect) plus still caches a load that spans no clear at all, which guards against the degenerate "never cache anything" fix.
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts — 3 cases. Corrected at the merge gate: the original 2 cases did not pin either half of the fix as this section previously claimed. Re-measured against all four phase/stop combinations, both passed with the listener still in the capture phase, and both passed with plain stopPropagation — the window-level clobber sits on a different target, which stopPropagation also suppresses. Added wins over a later document-level copy handler, which registers the clobber on the same target (document) ordered after the fallback's own listener: it fails with stopPropagation (expected 'later document handler' to be 'pane-42') and passes only with stopImmediatePropagation. All 3 fail against the unfixed origin/main source. The bubble-phase change itself remains unpinned by any test — it is load-bearing in the real DOM (xterm's handler is on terminal.element, a descendant the capture phase reaches before), but the happy-dom harness dispatches from that same element, so capture and bubble are indistinguishable there. Stated rather than claimed as covered.

Existing tests over the touched area (real commands, real results, at HEAD):

$ npx vitest run --config config/vitest.config.ts src/renderer/src/web/ src/main/jira
 Test Files  15 passed | 1 skipped (16)
      Tests  232 passed | 3 skipped (235)

$ cd mobile && npx vitest run src/notifications src/transport
 Test Files  62 passed (62)
      Tests  376 passed | 2 skipped (378)

src/transport is included because host-removal-lifecycle.test.ts asserts on HostNotificationSession.lastDeliveredSeq.

Typecheck and lint

$ pnpm run typecheck:node   # exit 0
$ pnpm run typecheck:web    # exit 0
$ cd mobile && npx tsc --noEmit   # exit 0
$ pnpm run check:code-quality:changed
code quality: 0 new finding(s) across 9 changed file(s).
type-aware code quality: 0 new finding(s) across 9 changed file(s).
React Doctor: 0 new finding(s) across 9 changed file(s).
Changed-code quality gate passed since 747b241145cf.

Deliberately not changed

• pruneExpired in the Jira cache still has no TTL sweep timer, so entries only expire when Jira activity touches the cache. That is why the leak was permanent, but it is not the defect this PR fixes; adding a timer is a separate change. The "no TTL sweep without activity" property is unchanged.
• The catch-up RPC stays outside enqueueHostDelivery. Moving it in would serialize a request that waits up to 30s ahead of live delivery, and it would not fix the watermark problem — the bug is which seq gets persisted, not ordering.
• session.lastDeliveredSeq still advances on live delivery; only the persisted value is clamped. The quarantine changes what reaches AsyncStorage, not in-memory dedup.
• Mark-after-success, not rollback-on-failure, for the seen key. Rollback needs a "did I add this key or was it already here" distinction to avoid deleting a key an earlier successful delivery owns, and it leaves the key present during the await — the exact window a concurrent replay of the same id consults. Marking after the await gives one rule that already governs the watermark two lines below: both advance only once the event reached the user. In-batch pre-marking is not needed, since the batch is a single queue entry and no second replay of a key can interleave within it.
• The dismiss path needed no separate treatment: it flows through the same deliverLive tail. dismissLocalNotification swallows its own OS error, so a rejecting dismiss is not reachable through a path a test can drive — that branch is fixed by construction rather than by a dedicated case.
• Two pre-existing hand-rolled fake copy events (in web-clipboard-copy-fallback.test.ts and the shared document helper in web-preload-api.test.ts) gained a stopImmediatePropagation stub — a property every real ClipboardEvent has. No original assertion was removed or weakened.

Known gaps, stated plainly

• No device-level proof for the notification fix. No paired phone, no real desktop host, no real socket drop. Evidence is the unit test above, not an end-to-end offline/reconnect run.
• Restart while a quarantine is open re-fetches. Because the persisted watermark is rolled back to the quarantined seq, a process death during an open gap resumes from the lower seq and may re-show notifications already delivered once the in-memory seen-set is gone. That is the at-least-once trade-off the existing saveWatermark comment already documents, and it is preferred over silent loss — but it is a real behavior change and there is no test pinning that duplicate-on-restart case.
• One notification edge left unchased. contiguousSeq advances after each replayed event returns, including events skipped as already-seen or already-queued. If a skipped event preceded a later failing one in the same batch, the quarantine point could sit above it. Now that seen is only marked after a show lands, an already-seen skip provably means the event reached the user — but the already-queued skip still rests on the claim being released only after its own delivery settles, and that composition is not pinned by a test.
• A retry needs a later catch-up to arrive. Marking seen after the show makes the failed event re-deliverable, but nothing re-fetches on its own — recovery still waits for the next ready. On a stable connection that may be a long time. Reducing that latency (an explicit retry) is deliberately out of scope.
• No UI proof for the Jira cache fix, and none is possible — the defect is retained bytes in the main process, not anything rendered. Staging it also needs two real Jira sites; it was reproduced only at the unit level.
• Third-party copy listeners were not exhaustively audited. All app-level copy listeners in src/ were checked (there are none besides this one), and Monaco's sole copy listener is scoped to its own textarea and does not stop propagation. Libraries beyond xterm and Monaco were not swept for document-level copy listeners that stopImmediatePropagation might now pre-empt.
• Review note: the notification fix initially clamped only watermark writes made after the failure, leaving an already-too-high value in storage that a restart would read back — loss reached through a restart instead of a reconnect. That was caught in review and fixed in ab2e89be3c; the rolls back a watermark a live event stored while the catch-up was in flight test covers it.
• Review note: CodeRabbit's final comment asked for a same-target clobber regression, since the existing case only installed the later handler on window and would also pass under stopPropagation. It landed after the previous head commit; verified correct and addressed in c075397c47.
• Review note: CodeRabbit then found that the quarantine held the range but not the event — seen was marked before delivery, so a rejected show could never be re-shown. Confirmed and fixed in 5b33a6cd1c. The suggested diff (drop the pre-mark in deliverMissedEvent) was not sufficient on its own: deliverLive marked the key itself before its await, so the identical hole existed on the live path with no catch-up involved. Both are now covered. The same commit contains the escaping rejection at both queue boundaries — the catch-up batch CodeRabbit flagged out-of-band, and queueDelivery, where the live-path rejection escapes identically (pre-existing on main).

Rebase note

Rebased onto 747b241145 (was branched at b41e813cb5). No conflicts, and no hunk was dropped: git log b41e813cb5..origin/main -- over mobile/src/notifications/, src/main/jira/attachment-image-cache.ts and src/renderer/src/web/web-clipboard-copy-fallback.ts is empty, so no upstream commit in that range touches any file this PR changes and none of these three defects was fixed independently.

Made with Orca 🐋

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add quarantine-aware notification catch-up bookkeeping, including clamped watermark requests, replay deduplication, failure handling, and tests for retries, rollback, delivery failures, and interrupted draining. Attachment image caching now uses global and per-site epochs to prevent cleared in-flight loads from repopulating the cache. Clipboard fallback copying now stops immediate propagation, with updated event mocks and tests covering terminal and window-level copy handlers.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The description is detailed, but it does not use the required template sections like Summary, Screenshots, Testing, AI Review Report, Security Audit, and Notes.	Rewrite the PR description to follow the repository template and add the missing required sections, including screenshots/no visual change, testing, review report, security audit, and notes.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the three main fixes in the changeset and is concise.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

mobile/src/notifications/mobile-notifications.ts (1)

172-197: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

A throwing replay rejects fetchMissed into an uncaught promise.

deliverMissedEvent can reject (a failing showLocalNotification). The finally quarantines correctly, but the rejection propagates out of the queued task and through await enqueueHostDelivery(...), so fetchMissed() rejects. Its only caller is the un-caught void (async () => { ... await fetchMissed() })() at Line 240, producing an unhandled rejection on every failed replay.

🛡️ Proposed fix: contain the replay failure at the queue boundary

-    await enqueueHostDelivery(session, async () => {
+    await enqueueHostDelivery(session, async () => {
       // Advances only past events this batch settled, so a teardown or a failing show
       // quarantines the true contiguous point instead of the range it never reached.
       let contiguousSeq = askFrom
       let drained = false
       try {
         for (const raw of missed) {
           // Re-checked per event: the batch can start before a teardown and still be
           // draining after it, and a torn-down host must stop pushing.
           if (disposed) {
             return
           }
           const event = raw as NotificationEvent | DismissNotificationEvent
           await deliverMissedEvent(event)
           contiguousSeq = event.notificationSeq ?? contiguousSeq
         }
         drained = true
       } finally {
         if (drained) {
           resolveCatchUpQuarantine(session, hostId)
         } else {
           quarantineCatchUpWatermark(session, hostId, contiguousSeq)
         }
       }
-    })
+    }).catch(() => {
+      // Quarantine already recorded the contiguous point; the failure must not escape
+      // into the un-awaited 'ready' continuation.
+    })

🧹 Nitpick comments (4)

src/main/jira/attachment-image-cache.ts (1)

18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid repeating the epoch walkthrough in comments.

• src/main/jira/attachment-image-cache.ts#L18-L20: reduce this to a one-line rationale for the monotonic ticker.
• src/main/jira/attachment-image-cache.test.ts#L63-L66: remove the detailed collision narrative; the test name and setup already communicate the regression.

As per coding guidelines, “Comments must be concise, limited to non-obvious information, and preferably one line; do not explain obvious code or walk through implementation details.”

Source: Coding guidelines

src/renderer/src/web/web-clipboard-copy-fallback.ts (1)

14-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Condense the propagation comments. Keep only the non-obvious rationale in a short single-line comment.

• src/renderer/src/web/web-clipboard-copy-fallback.ts#L14-L17: reduce to one concise explanation of the required bubble-phase ordering.
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts#L5-L6: shorten the real-DOM rationale.
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts#L25-L28: replace the multi-line JSDoc with a concise helper description.
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts#L40-L43: replace the multi-line JSDoc with a concise helper description.
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts#L64-L68: reduce the historical implementation walkthrough to the test’s essential setup rationale.
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts#L78-L79: shorten the propagation-order explanation.

Source: Coding guidelines

mobile/src/notifications/notification-catchup-failure-quarantine.test.ts (2)

159-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider an exact assertion on the replayed titles.

arrayContaining also passes if m6/m7 are shown twice, which is the adjacent double-push regression the seen guard exists to prevent. toEqual(['m6', 'm7']) pins both presence and dedup.

♻️ Tighter assertion

-    expect(titles).toEqual(expect.arrayContaining(['m6', 'm7']))
+    expect(titles).toEqual(['m6', 'm7'])

---

222-236: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for a rejecting show, and assert the recovered titles.

The teardown case aborts before deliverMissedEvent(7) runs, so seqs 7-8 are never marked seen and replay cleanly. The sibling case — showLocalNotification rejecting mid-batch — takes a different path through the seen bookkeeping flagged in mobile-notifications.ts (Line 113-129) and is untested. Also assert m7/m8 were actually shown here, not just that the watermark reached 20.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 47a8b82e-fad7-4e4d-9b0a-187de3278f06

📥 Commits

Reviewing files that changed from the base of the PR and between 930ff96 and 3dc5149cbfd236bca78a58d699fc4028d5039d05.

📒 Files selected for processing (9)

• mobile/src/notifications/mobile-notifications.ts
• mobile/src/notifications/notification-catchup-failure-quarantine.test.ts
• mobile/src/notifications/notification-reconnect-catchup.ts
• src/main/jira/attachment-image-cache.test.ts
• src/main/jira/attachment-image-cache.ts
• src/renderer/src/web/web-clipboard-copy-fallback.test.ts
• src/renderer/src/web/web-clipboard-copy-fallback.ts
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts
• src/renderer/src/web/web-preload-api.test.ts

[greptile-apps]

Greptile Summary

This PR fixes three independent data-loss and data-leak defects introduced by earlier patches: mobile notification loss when a catch-up RPC fails, Jira attachment cache re-population with post-revocation credentials, and the web clipboard copy fallback being overwritten by the xterm terminal selection.

• Mobile notification quarantine (mobile/): fetchMissed now returns null on failure, triggering a per-session quarantine that pins the persisted watermark at the last contiguously-delivered seq until a successful catch-up lifts it; seen is also moved to mark-after-success so a rejected showLocalNotification cannot silently block future replays of the same event.
• Jira attachment epoch guard (src/main/jira/attachment-image-cache.ts): replaces the additive siteEpoch + cacheEpoch check with a single monotonic ticker via Math.max, eliminating the integer-collision where a per-site clear at value 1 summed identically to a subsequent global clear.
• Clipboard bubble-phase fix (src/renderer/src/web/web-clipboard-copy-fallback.ts): switches the onCopy listener from capture to bubble phase and adds stopImmediatePropagation(), so the document-level handler now wins over xterm's bubble handler and any subsequent window-level listeners.

Confidence Score: 5/5

All three changes are narrowly scoped to the defective paths, backed by regression tests that fail on the original code, and leave the surrounding serialization and dedup logic intact.

Each fix addresses a clearly diagnosed, well-tested data-loss or data-leak path. The quarantine logic in the mobile notification path is the most involved change, but it is well-commented, all edge cases are covered by the new test suite, and the known gaps are plainly documented.

Files Needing Attention: No files require special attention; the most complex logic is in mobile-notifications.ts and notification-reconnect-catchup.ts, both of which are thoroughly tested by the new quarantine test file.

Important Files Changed

Filename	Overview
mobile/src/notifications/mobile-notifications.ts	Core notification delivery reworked: deliverMissedEvent extracted, seen now marked after show succeeds, batch loop tracks contiguousSeq for fine-grained quarantine, and fetchMissed returns null on failure instead of silently returning an empty array
mobile/src/notifications/notification-reconnect-catchup.ts	Adds catchUpQuarantineSeq to HostNotificationSession plus three new functions: quarantineCatchUpWatermark, resolveCatchUpQuarantine, and catchUpWatermarkSeq; adoptNotificationEpoch also resets the quarantine when the desktop counter changes
src/main/jira/attachment-image-cache.ts	Replaces the summed siteEpoch + cacheEpoch staleness guard with a single monotonic ticker via Math.max, preventing the sum-collision where a per-site clear plus a global clear produced the same integer
src/renderer/src/web/web-clipboard-copy-fallback.ts	Switches onCopy from capture to bubble phase and adds stopImmediatePropagation(), so the document-level handler wins over xterm's bubble handler and any subsequent window-level listeners
mobile/src/notifications/notification-catchup-failure-quarantine.test.ts	New test file with 5 cases covering quarantine across multiple failed catch-ups, watermark rollback after an in-flight live write, teardown-induced partial quarantine, and mark-after-success for both replay and live paths
src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts	New test file using a real happy-dom environment with genuine capture/bubble propagation; covers the xterm overwrite scenario and the stopImmediatePropagation window-handler scenario
src/main/jira/attachment-image-cache.test.ts	Two new test cases: the sum-collision defect and a regression guard ensuring loads with no intervening clear are still cached
src/renderer/src/web/web-clipboard-copy-fallback.test.ts	Adds stopImmediatePropagation stub to the fake ClipboardEvent so existing tests do not throw; no assertions changed
src/renderer/src/web/web-preload-api.test.ts	Same stopImmediatePropagation stub added to the shared document-helper fake event; minimal change, no assertions removed

Sequence Diagram

sequenceDiagram
    participant Desktop
    participant Session
    participant Queue
    participant Storage

    Note over Session: Quarantine: null, lastDeliveredSeq: 5

    Desktop->>Session: ready (epoch-1)
    Session->>Desktop: getMissedSince(lastSeenSeq:5)
    Desktop-->>Session: FAIL (socket closed)
    Session->>Session: "quarantineCatchUpWatermark(askFrom=5)"
    Note over Session: Quarantine: 5
    Session->>Storage: saveWatermark(seq:5) rollback

    Desktop->>Queue: live event seq:11
    Queue->>Storage: saveWatermark(seq:5) clamped by quarantine

    Desktop->>Session: ready (epoch-1)
    Session->>Desktop: getMissedSince(lastSeenSeq:5)
    Desktop-->>Session: [seq:6, seq:7]
    Session->>Queue: enqueueHostDelivery(batch)
    Queue->>Queue: deliverMissedEvent(seq:6) show succeeds, seen.add
    Queue->>Queue: deliverMissedEvent(seq:7) show succeeds, seen.add
    Queue->>Session: resolveCatchUpQuarantine()
    Note over Session: Quarantine: null
    Session->>Storage: saveWatermark(seq:11) full watermark restored

Loading

_{Reviews (3): Last reviewed commit: "test(web-clipboard): pin stopImmediatePr..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

mobile/src/notifications/notification-catchup-failure-quarantine.test.ts (1)

21-22: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Import the watermark key builder instead of duplicating the key.

WATERMARK_STORAGE_KEY_PREFIX and watermarkStorageKey() live in notification-reconnect-catchup.ts; exporting the builder from there would avoid this test hard-coding an internal key format.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f13a6d8-cf52-4477-bd1c-942bd852290d

📥 Commits

Reviewing files that changed from the base of the PR and between 3dc5149cbfd236bca78a58d699fc4028d5039d05 and 5b33a6c.

📒 Files selected for processing (9)

• mobile/src/notifications/mobile-notifications.ts
• mobile/src/notifications/notification-catchup-failure-quarantine.test.ts
• mobile/src/notifications/notification-reconnect-catchup.ts
• src/main/jira/attachment-image-cache.test.ts
• src/main/jira/attachment-image-cache.ts
• src/renderer/src/web/web-clipboard-copy-fallback.test.ts
• src/renderer/src/web/web-clipboard-copy-fallback.ts
• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts
• src/renderer/src/web/web-preload-api.test.ts

🚧 Files skipped from review as they are similar to previous changes (7)

• src/renderer/src/web/web-clipboard-copy-fallback.test.ts
• src/renderer/src/web/web-clipboard-copy-fallback.ts
• src/main/jira/attachment-image-cache.test.ts
• mobile/src/notifications/notification-reconnect-catchup.ts
• src/main/jira/attachment-image-cache.ts
• src/renderer/src/web/web-preload-api.test.ts
• mobile/src/notifications/mobile-notifications.ts

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts (1)

43-61: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Restore the document.execCommand stub after each test.

stubExecCommand replaces the global method, but the current cleanup only clears document.body. Restore the original implementation to prevent order-dependent tests.

Suggested cleanup

+const originalExecCommand = document.execCommand
+
 afterEach(() => {
   document.body.innerHTML = ''
+  document.execCommand = originalExecCommand
 })

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 421036d0-b77e-425d-bcda-9a722eab9dac

📥 Commits

Reviewing files that changed from the base of the PR and between 5b33a6c and c075397.

📒 Files selected for processing (1)

• src/renderer/src/web/web-clipboard-copy-terminal-selection.test.ts