fix(editor): give self-move stamps per-registration expiries + retract tokens

The refcount model used a single shared expiry scalar per role that only
grew via max() and reset at refs=0, so releasing the max-contributing
registration left survivors inheriting an over-extended window (and an
expired registration could be resurrected by a later stamp on a key the
opposite role kept resident). Both over-suppress genuine changes.

Model each stamp as an independent registration carrying its own expiry
(a list per role). recordSelfMove returns a ticket; clearSelfMove
retracts exactly that registration. A role is live while any of its
registrations is unexpired, so concurrent stamps, failed-move clears, and
expiry are all precise. Wrapper/helper thread the tickets through.

Adds regressions for the over-extension and resurrection cases.

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/components/editor/editor-self-move-registry.test.ts

@@ -71,8 +71,8 @@ describe('editor-self-move-registry', () => {
   })
 
   it('clears the roles a move stamped when the rename fails', () => {
-    recordSelfMove('/repo/old.md', '/repo/new.md')
-    clearSelfMove('/repo/old.md', '/repo/new.md')
+    const ticket = recordSelfMove('/repo/old.md', '/repo/new.md')
+    clearSelfMove(ticket)
 
     expect(isRecentSelfMoveSource('/repo/old.md')).toBe(false)
     expect(isRecentSelfMoveTarget('/repo/new.md')).toBe(false)
@@ -84,9 +84,9 @@ describe('editor-self-move-registry', () => {
     // second fails (destination now exists) and clears. The successful move's
     // target stamp must survive so its delayed echo is still recognized.
     recordSelfMove('/one/report.md', '/dest/report.md') // move A
-    recordSelfMove('/two/report.md', '/dest/report.md') // move B (will fail)
+    const ticketB = recordSelfMove('/two/report.md', '/dest/report.md') // move B (fails)
 
-    clearSelfMove('/two/report.md', '/dest/report.md') // B's failure clear
+    clearSelfMove(ticketB)
 
     expect(isRecentSelfMoveTarget('/dest/report.md')).toBe(true)
     // B's own source is gone; A's source is untouched.
@@ -94,12 +94,41 @@ describe('editor-self-move-registry', () => {
     expect(isRecentSelfMoveSource('/one/report.md')).toBe(true)
   })
 
-  it('clear fully releases a target once every registration is cleared', () => {
-    recordSelfMove('/one/report.md', '/dest/report.md')
-    recordSelfMove('/two/report.md', '/dest/report.md')
+  it('clearing a stamp does not over-extend a concurrent stamp’s expiry', () => {
+    // Move A stamps /dest at t=0 (expires 750). Move B stamps the same target at
+    // t=700 (expires 1450) and fails at t=710. Clearing B must not leave /dest
+    // live past A's own 750ms promise — a single shared scalar expiry would.
+    recordSelfMove('/one/report.md', '/dest/report.md', undefined, 750) // A @ t=0
+    vi.advanceTimersByTime(700)
+    const ticketB = recordSelfMove('/two/report.md', '/dest/report.md', undefined, 750) // B @ t=700
+    vi.advanceTimersByTime(10)
+    clearSelfMove(ticketB) // B fails @ t=710
 
-    clearSelfMove('/one/report.md', '/dest/report.md')
-    clearSelfMove('/two/report.md', '/dest/report.md')
+    vi.advanceTimersByTime(50) // t=760, past A's 750 promise
+    expect(isRecentSelfMoveTarget('/dest/report.md')).toBe(false)
+  })
+
+  it('does not resurrect an expired registration when a later stamp reuses the key', () => {
+    // A→P (target) expires, but a reverse move keeps P's map entry resident as a
+    // source. A later failed move on P must not revive the expired target refs.
+    recordSelfMove('/x.md', '/p.md', undefined, 750) // P target, expires 750
+    recordSelfMove('/p.md', '/q.md', undefined, 3000) // P source, keeps entry alive
+    vi.advanceTimersByTime(1000) // P's target registration has expired
+
+    const failed = recordSelfMove('/y.md', '/p.md', undefined, 750) // P target again
+    clearSelfMove(failed)
+
+    expect(isRecentSelfMoveTarget('/p.md')).toBe(false)
+    // The still-live reverse-move source is untouched.
+    expect(isRecentSelfMoveSource('/p.md')).toBe(true)
+  })
+
+  it('clear fully releases a target once every registration is cleared', () => {
+    const ticketA = recordSelfMove('/one/report.md', '/dest/report.md')
+    const ticketB = recordSelfMove('/two/report.md', '/dest/report.md')
+
+    clearSelfMove(ticketA)
+    clearSelfMove(ticketB)
 
     expect(isRecentSelfMoveTarget('/dest/report.md')).toBe(false)
   })
@@ -108,9 +137,9 @@ describe('editor-self-move-registry', () => {
     // Move A → B, then C → A. Clearing the failed C → A must not drop A's source
     // role from the successful A → B move.
     recordSelfMove('/repo/A.md', '/repo/B.md')
-    recordSelfMove('/repo/C.md', '/repo/A.md')
+    const ticketC = recordSelfMove('/repo/C.md', '/repo/A.md')
 
-    clearSelfMove('/repo/C.md', '/repo/A.md')
+    clearSelfMove(ticketC)
 
     expect(isRecentSelfMoveSource('/repo/A.md')).toBe(true)
     expect(isRecentSelfMoveTarget('/repo/A.md')).toBe(false)

src/renderer/src/components/editor/editor-self-move-registry.ts

@@ -23,22 +23,28 @@ export const SELF_MOVE_REMOTE_TTL_MS = 3000
 // valve — TTL pruning is the real bound, since stamps self-expire within seconds.
 const SELF_MOVE_MAX_STAMPS = 1024
 
-// Why: a path can hold BOTH roles at once — e.g. move A→B then immediately undo
-// B→A leaves A a live source (of the first move, whose delayed watcher echo may
-// still be in flight) and a live target (of the undo) — and it can hold the SAME
-// role from two concurrent moves (drag two files named report.md into one dir →
-// both stamp that dir's report.md as a target). So each role is reference
-// counted: a role is live while any un-cleared registration remains and its
-// expiry is in the future. Refcounting lets a FAILED move's clear drop only its
-// own registration instead of erasing a concurrent successful move's stamp.
-type RoleState = {
-  refs: number
-  expiresAt: number
+// Why: each stamp is one independent registration carrying its own expiry, held
+// as a list per role. This keeps concurrent registrations of the same path+role
+// (drag two files named report.md into one dir → both stamp that dir's
+// report.md as a target) fully separate, so:
+//  - a role is live while ANY of its registrations is unexpired,
+//  - `clearSelfMove` retracts EXACTLY the registration a failed move added (by
+//    its ticket's expiry), never a concurrent move's,
+//  - an expired registration can't be "resurrected" by a later stamp on the
+//    same key, because liveness is computed per registration, not from a shared
+//    scalar. (A single shared expiry would over-extend the suppression window
+//    when the max-contributing registration is cleared.)
+type SelfMoveStamp = {
+  source: number[]
+  target: number[]
 }
 
-type SelfMoveStamp = {
-  source: RoleState
-  target: RoleState
+/** A retraction handle for one recorded move — see {@link clearSelfMove}. */
+export type SelfMoveTicket = {
+  fromPath: string
+  toPath: string
+  runtimeEnvironmentId: string | null
+  expiresAt: number
 }
 
 const stamps = new Map<string, SelfMoveStamp>()
@@ -47,15 +53,19 @@ function selfMoveKey(absolutePath: string, runtimeEnvironmentId?: string | null)
   return `${runtimeEnvironmentId?.trim() || 'client'}::${normalizeAbsolutePathForComparison(absolutePath)}`
 }
 
-function isRoleLive(state: RoleState, now: number): boolean {
-  return state.refs > 0 && now <= state.expiresAt
+function dropExpiredAndMaybeDelete(key: string, stamp: SelfMoveStamp, now: number): boolean {
+  stamp.source = stamp.source.filter((expiresAt) => now <= expiresAt)
+  stamp.target = stamp.target.filter((expiresAt) => now <= expiresAt)
+  if (stamp.source.length === 0 && stamp.target.length === 0) {
+    stamps.delete(key)
+    return true
+  }
+  return false
 }
 
 function pruneExpiredSelfMoves(now: number): void {
   for (const [key, stamp] of stamps) {
-    if (!isRoleLive(stamp.source, now) && !isRoleLive(stamp.target, now)) {
-      stamps.delete(key)
-    }
+    dropExpiredAndMaybeDelete(key, stamp, now)
   }
 }
 
@@ -69,25 +79,39 @@ function enforceSelfMoveStampLimit(): void {
   }
 }
 
-function emptyStamp(): SelfMoveStamp {
-  return { source: { refs: 0, expiresAt: 0 }, target: { refs: 0, expiresAt: 0 } }
-}
-
-function stampRole(
+function pushRegistration(
   absolutePath: string,
   role: 'source' | 'target',
   runtimeEnvironmentId: string | null | undefined,
   expiresAt: number
 ): void {
   const key = selfMoveKey(absolutePath, runtimeEnvironmentId)
-  const next = stamps.get(key) ?? emptyStamp()
-  const state = next[role]
-  state.refs += 1
-  state.expiresAt = Math.max(state.expiresAt, expiresAt)
+  const stamp = stamps.get(key) ?? { source: [], target: [] }
+  stamp[role].push(expiresAt)
   // Why: delete+set keeps insertion recency fresh so the cap evicts genuinely
   // old stamps first.
   stamps.delete(key)
-  stamps.set(key, next)
+  stamps.set(key, stamp)
+}
+
+function removeRegistration(
+  absolutePath: string,
+  role: 'source' | 'target',
+  runtimeEnvironmentId: string | null | undefined,
+  expiresAt: number
+): void {
+  const key = selfMoveKey(absolutePath, runtimeEnvironmentId)
+  const stamp = stamps.get(key)
+  if (!stamp) {
+    return
+  }
+  const index = stamp[role].indexOf(expiresAt)
+  if (index >= 0) {
+    stamp[role].splice(index, 1)
+  }
+  if (stamp.source.length === 0 && stamp.target.length === 0) {
+    stamps.delete(key)
+  }
 }
 
 /**
@@ -95,20 +119,33 @@ function stampRole(
  * delete(old)+create(new) echo as self-initiated instead of an external change.
  * Both endpoints are stamped: the source so the delete does not tombstone the
  * tab, the target so the create does not raise a changed-on-disk banner on the
- * (re-homed, still-dirty) tab.
+ * (re-homed, still-dirty) tab. Returns a ticket to retract this exact stamp with
+ * {@link clearSelfMove} if the move turns out not to happen.
  */
 export function recordSelfMove(
   fromPath: string,
   toPath: string,
   runtimeEnvironmentId?: string | null,
   ttlMs: number = SELF_MOVE_TTL_MS
-): void {
+): SelfMoveTicket {
   const now = Date.now()
   pruneExpiredSelfMoves(now)
   const expiresAt = now + ttlMs
-  stampRole(fromPath, 'source', runtimeEnvironmentId, expiresAt)
-  stampRole(toPath, 'target', runtimeEnvironmentId, expiresAt)
+  pushRegistration(fromPath, 'source', runtimeEnvironmentId, expiresAt)
+  pushRegistration(toPath, 'target', runtimeEnvironmentId, expiresAt)
   enforceSelfMoveStampLimit()
+  return { fromPath, toPath, runtimeEnvironmentId: runtimeEnvironmentId ?? null, expiresAt }
+}
+
+/**
+ * Retracts exactly the registration a {@link recordSelfMove} added, so a rename
+ * that FAILED after stamping does not keep suppressing genuine watcher events
+ * for the paths it never moved. A concurrent move's registration on the same
+ * path+role is untouched.
+ */
+export function clearSelfMove(ticket: SelfMoveTicket): void {
+  removeRegistration(ticket.fromPath, 'source', ticket.runtimeEnvironmentId, ticket.expiresAt)
+  removeRegistration(ticket.toPath, 'target', ticket.runtimeEnvironmentId, ticket.expiresAt)
 }
 
 function hasLiveRole(
@@ -122,12 +159,10 @@ function hasLiveRole(
     return false
   }
   const now = Date.now()
-  const live = isRoleLive(stamp[role], now)
-  // Why: drop the whole entry once neither role is live so expired stamps don't
-  // linger until the next write's prune.
-  if (!isRoleLive(stamp.source, now) && !isRoleLive(stamp.target, now)) {
-    stamps.delete(key)
-  }
+  const live = stamp[role].some((expiresAt) => now <= expiresAt)
+  // Why: drop expired registrations on read so they can't be counted live and
+  // don't linger until the next write's prune.
+  dropExpiredAndMaybeDelete(key, stamp, now)
   return live
 }
 
@@ -147,44 +182,6 @@ export function isRecentSelfMoveSource(
   return hasLiveRole(absolutePath, 'source', runtimeEnvironmentId)
 }
 
-function clearRole(
-  absolutePath: string,
-  role: 'source' | 'target',
-  runtimeEnvironmentId: string | null | undefined
-): void {
-  const key = selfMoveKey(absolutePath, runtimeEnvironmentId)
-  const stamp = stamps.get(key)
-  if (!stamp) {
-    return
-  }
-  const state = stamp[role]
-  // Why: drop only THIS move's registration (one ref). A concurrent move that
-  // stamped the same role keeps the role live via its own ref.
-  state.refs = Math.max(0, state.refs - 1)
-  if (state.refs === 0) {
-    state.expiresAt = 0
-  }
-  const now = Date.now()
-  if (!isRoleLive(stamp.source, now) && !isRoleLive(stamp.target, now)) {
-    stamps.delete(key)
-  }
-}
-
-/**
- * Drops one registration of the roles a move stamped, so a rename that FAILED
- * after stamping does not keep suppressing genuine watcher events for the paths
- * it never actually moved. Reference counted, so a concurrent move that stamped
- * the same role keeps that role live.
- */
-export function clearSelfMove(
-  fromPath: string,
-  toPath: string,
-  runtimeEnvironmentId?: string | null
-): void {
-  clearRole(fromPath, 'source', runtimeEnvironmentId)
-  clearRole(toPath, 'target', runtimeEnvironmentId)
-}
-
 export function __clearSelfMoveRegistryForTests(): void {
   stamps.clear()
 }

src/renderer/src/lib/record-self-move-for-open-tabs.test.ts

@@ -75,8 +75,8 @@ describe('recordSelfMoveForOpenTabs', () => {
       { suppressActiveRuntimeFallback: true }
     )
 
-    recordSelfMoveForOpenTabs('/repo/docs/readme.md', '/repo/notes/readme.md')
-    clearSelfMoveForOpenTabs('/repo/docs/readme.md', '/repo/notes/readme.md')
+    const tickets = recordSelfMoveForOpenTabs('/repo/docs/readme.md', '/repo/notes/readme.md')
+    clearSelfMoveForOpenTabs(tickets)
 
     expect(isRecentSelfMoveSource('/repo/docs/readme.md')).toBe(false)
     expect(isRecentSelfMoveTarget('/repo/notes/readme.md')).toBe(false)

src/renderer/src/lib/record-self-move-for-open-tabs.ts

@@ -3,7 +3,8 @@ import { getConnectionIdForFile } from '@/lib/connection-context'
 import {
   clearSelfMove,
   recordSelfMove,
-  SELF_MOVE_REMOTE_TTL_MS
+  SELF_MOVE_REMOTE_TTL_MS,
+  type SelfMoveTicket
 } from '@/components/editor/editor-self-move-registry'
 import { isPathInsideOrEqual } from './remap-open-editor-tabs-for-path-change'
 import type { OpenFile } from '@/store/slices/editor'
@@ -49,15 +50,19 @@ function collectOpenTabMoves(fromPath: string, toPath: string): OpenTabMove[] {
  * before/after stamps and the failure clear. This is exported for that wrapper
  * (and tests); call it directly only when you own the rename lifecycle.
  */
-export function recordSelfMoveForOpenTabs(fromPath: string, toPath: string): void {
-  for (const move of collectOpenTabMoves(fromPath, toPath)) {
+export function recordSelfMoveForOpenTabs(fromPath: string, toPath: string): SelfMoveTicket[] {
+  return collectOpenTabMoves(fromPath, toPath).map((move) =>
     recordSelfMove(move.file.filePath, move.newFilePath, move.runtimeOwner, move.ttlMs)
-  }
+  )
 }
 
-/** Undoes {@link recordSelfMoveForOpenTabs} for a move that did not happen. */
-export function clearSelfMoveForOpenTabs(fromPath: string, toPath: string): void {
-  for (const move of collectOpenTabMoves(fromPath, toPath)) {
-    clearSelfMove(move.file.filePath, move.newFilePath, move.runtimeOwner)
+/**
+ * Retracts the exact registrations from {@link recordSelfMoveForOpenTabs} for a
+ * move that did not happen. Takes the tickets that call returned so a concurrent
+ * move's stamp on a shared path is never dropped.
+ */
+export function clearSelfMoveForOpenTabs(tickets: SelfMoveTicket[]): void {
+  for (const ticket of tickets) {
+    clearSelfMove(ticket)
   }
 }

src/renderer/src/lib/rename-open-editor-tabs-path.ts

@@ -23,11 +23,11 @@ export async function renameOpenTabsPathOnDisk(
   fromPath: string,
   toPath: string
 ): Promise<void> {
-  recordSelfMoveForOpenTabs(fromPath, toPath)
+  const pendingTickets = recordSelfMoveForOpenTabs(fromPath, toPath)
   try {
     await renameRuntimePath(context, fromPath, toPath)
   } catch (err) {
-    clearSelfMoveForOpenTabs(fromPath, toPath)
+    clearSelfMoveForOpenTabs(pendingTickets)
     throw err
   }
   recordSelfMoveForOpenTabs(fromPath, toPath)