fix: free a reopened editor from a resolution the closed one left in flight

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-20 01:20:48 +02:00
co-authored by Claude Opus 5
parent 5bbd77ce0a
commit 23193d8eb8
4 changed files with 93 additions and 38 deletions
@@ -20,6 +20,7 @@
import { useActingUser } from '$lib/actingUser.svelte'
import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte'
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
import { useDraftConflictSession } from '$lib/draftConflictSession.svelte'
import DraftConflictAlert from './DraftConflictAlert.svelte'
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
import { onUserInput } from '$lib/userDraftEditGate'
@@ -266,22 +267,16 @@
)
const anyDirty = $derived(dirtyWorkspaces.length > 0)
/** Which editing session a conflict resolution belongs to. Comparing `selected`/path is not
* enough — this component outlives the drawer and reopening the same resource reuses it with
* those same values — so a resolution carries the session it started in and every step checks
* it is still the current one. */
let resolveGeneration = 0
/** Nothing outstanding speaks for this editor any more. Exported because the drawer, not this
* component, is what knows a session has ended. */
/** This component outlives the drawer and reopening the same resource reuses it with the same
* `selected`/path, so a resolution cannot tell from those whether it still speaks for what is
* on screen. Ending is exported because the drawer, not this component, knows when a session
* is over. */
const conflictSession = useDraftConflictSession()
export function endEditingSession(): void {
resolveGeneration++
conflictSession.end()
}
onDestroy(endEditingSession)
/** The session whose resolution is in flight, or 0. Scoped by generation rather than a plain
* flag: a request left over from a closed session must not leave the next one showing busy
* buttons it can never un-disable, and its `finally` must not clear a newer one's. */
let resolvingFor = $state(0)
const resolvingConflict = $derived(resolvingFor !== 0 && resolvingFor === resolveGeneration)
const resolvingConflict = $derived(conflictSession.busy)
/** The server refused this tab's autosave because the row moved under it: another tab, or the
* AI chat, which writes these drafts too. Nothing typed here reaches the server until the user
* picks a version, and the unsaved-changes banner says the opposite — that the edits are held
@@ -301,9 +296,8 @@
const p = initialPath
if (!ws || !p || resolvingConflict) return
const query = { workspace: ws, itemKind: 'resource' as const, path: p }
const gen = ++resolveGeneration
const stillOurs = () => gen === resolveGeneration && selected === ws
resolvingFor = gen
const token = conflictSession.start()
const stillOurs = () => conflictSession.holds(token) && selected === ws
try {
if (keepMine) {
// Settle the key first: an ordinary autosave still queued would displace the forced
@@ -357,9 +351,7 @@
// resolve again — which is the whole point of reading first.
sendUserToast(`Could not load the other version: ${e}`, true)
} finally {
// Only if it is still ours: a stale one settling later must not clear the busy state of
// the session that replaced it.
if (resolvingFor === gen) resolvingFor = 0
conflictSession.finish(token)
}
}
@@ -25,6 +25,7 @@
import LocalDraftBanner from './LocalDraftBanner.svelte'
import DraftConflictAlert from './DraftConflictAlert.svelte'
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
import { useDraftConflictSession } from '$lib/draftConflictSession.svelte'
import { isEncryptedDraftValue } from '$lib/encryptedDraft'
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
@@ -125,22 +126,15 @@
Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws]))
)
/** Which editing session a conflict resolution belongs to. Comparing `selected`/path is not
* enough — reopening the same variable reuses this component and those same values — so a
* resolution carries the session it started in and every step checks it is still the current
* one. Ended by `endEditingSession`, which every entry point and the teardown go through. */
let resolveGeneration = 0
/** Nothing outstanding speaks for this editor any more: a different variable, a different
* session on the same one, or the component going away. */
/** Ended by every entry point and by the teardown: reopening the same variable reuses this
* component, so `selected`/path cannot tell a resolution whether it still speaks for what is
* on screen. */
const conflictSession = useDraftConflictSession()
function endEditingSession(): void {
resolveGeneration++
conflictSession.end()
}
onDestroy(endEditingSession)
/** The session whose resolution is in flight, or 0. Scoped by generation rather than a plain
* flag: a request left over from a closed session must not leave the next one showing busy
* buttons it can never un-disable, and its `finally` must not clear a newer one's. */
let resolvingFor = $state(0)
const resolvingConflict = $derived(resolvingFor !== 0 && resolvingFor === resolveGeneration)
const resolvingConflict = $derived(conflictSession.busy)
/** The server refused this tab's autosave because the row moved under it: another tab, or the
* AI chat, which writes these drafts too. Nothing typed here reaches the server until the user
* picks a version, and the unsaved-changes banner says the opposite — that the edits are held
@@ -160,9 +154,8 @@
const p = editPath
if (!ws || !p || resolvingConflict) return
const query = { workspace: ws, itemKind: 'variable' as const, path: p }
const gen = ++resolveGeneration
const stillOurs = () => gen === resolveGeneration && selected === ws
resolvingFor = gen
const token = conflictSession.start()
const stillOurs = () => conflictSession.holds(token) && selected === ws
try {
if (keepMine) {
// Settle the key first: an ordinary autosave still queued would displace the forced
@@ -224,9 +217,7 @@
// resolve again — which is the whole point of reading first.
sendUserToast(`Could not load the other version: ${e}`, true)
} finally {
// Only if it is still ours: a stale one settling later must not clear the busy state of
// the session that replaced it.
if (resolvingFor === gen) resolvingFor = 0
conflictSession.finish(token)
}
}
@@ -0,0 +1,41 @@
/**
* Identity for one editing session's conflict resolution.
*
* A drawer editor outlives what it opens: reopening the same item reuses the component with the
* same workspace and path, so an awaited step cannot tell from those alone whether it still
* speaks for what is on screen. Every resolution takes a token and checks it before touching
* shared editor state.
*/
export function useDraftConflictSession() {
let generation = $state(0)
/** The token of the resolution in flight, or 0. A token rather than a flag: one left over from
* a closed session must not clear the busy state of the session that replaced it. */
let inFlight = $state(0)
return {
/** A resolution started in the current session is in flight. Ending a session clears it, so
* a reopened editor never shows buttons that only a stale request settling can re-enable. */
get busy(): boolean {
return inFlight !== 0
},
/** Claim the session for a resolution about to start. */
start(): number {
inFlight = ++generation
return inFlight
},
/** Whether `token` still speaks for the editor. */
holds(token: number): boolean {
return token === generation
},
/** Release `token`'s claim. A stale token is ignored. */
finish(token: number): void {
if (inFlight === token) inFlight = 0
},
/** Nothing outstanding speaks for this editor any more: a different item, a different
* session on the same one, or the component going away. */
end(): void {
generation++
inFlight = 0
}
}
}
@@ -0,0 +1,31 @@
import { describe, it, expect } from 'vitest'
import { useDraftConflictSession } from './draftConflictSession.svelte'
/**
* A drawer editor is reused by the next thing it opens, so a resolution the closed session left
* in flight must neither speak for the new one nor hold its buttons disabled until it settles.
*/
describe('useDraftConflictSession', () => {
it('frees a reopened session from a resolution the closed one left in flight', () => {
const session = useDraftConflictSession()
const stale = session.start()
expect(session.busy).toBe(true)
session.end()
expect(session.busy).toBe(false)
expect(session.holds(stale)).toBe(false)
const fresh = session.start()
expect(session.busy).toBe(true)
expect(session.holds(fresh)).toBe(true)
// The stale request settles last: it must not release the live session's claim.
session.finish(stale)
expect(session.busy).toBe(true)
session.finish(fresh)
expect(session.busy).toBe(false)
})
})