mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-12 16:02:56 +00:00
fix: guard session moves against concurrent turns and unsaved edits
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
bdb111c236
commit
c91afd67aa
@@ -645,9 +645,9 @@ pub async fn delete_own_draft_for_path(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Copy drafts from `source` into its fork `target`, returning the `(path, typ)` of each row
|
||||
/// Copy drafts from `source` into its fork `target`, returning the `(kind, path)` of each row
|
||||
/// copied. With `items = None` this is the fork-time clone: every row of `email` plus the legacy
|
||||
/// NULL-email row. With `items`, only `email`'s own rows at those `(path, kind)` pairs.
|
||||
/// NULL-email row. With `items`, only `email`'s own rows at those `(kind, path)` pairs.
|
||||
///
|
||||
/// A plain INSERT: the caller guarantees the target holds none of the copied rows (an empty
|
||||
/// fork, or a conflict check under the same transaction), else `draft_pkey_with_user` aborts it.
|
||||
|
||||
@@ -1305,8 +1305,21 @@ export class AIChatManager implements ChatViewHost {
|
||||
}, 5_000)
|
||||
}
|
||||
|
||||
#autoResumeHolds = 0
|
||||
/** Keep background-job completions from starting a turn until the returned release runs. */
|
||||
holdAutoResume(): () => void {
|
||||
this.#autoResumeHolds++
|
||||
let released = false
|
||||
return () => {
|
||||
if (released) return
|
||||
released = true
|
||||
this.#autoResumeHolds--
|
||||
void this.#maybeAutoResumeFromJobs()
|
||||
}
|
||||
}
|
||||
|
||||
async #maybeAutoResumeFromJobs() {
|
||||
if (this.#autoResuming) return
|
||||
if (this.#autoResuming || this.#autoResumeHolds > 0) return
|
||||
// Global/sessions chat only (the only mode with a jobs tray + preamble).
|
||||
if (this.mode !== AIMode.GLOBAL) return
|
||||
// Mid-turn: the notes will ride that turn's preamble, so don't start another.
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
open,
|
||||
parentName,
|
||||
targetName,
|
||||
newFork = false,
|
||||
items,
|
||||
conflicts = [],
|
||||
error,
|
||||
@@ -20,6 +21,8 @@
|
||||
open: boolean
|
||||
parentName: string
|
||||
targetName: string
|
||||
// A new fork is cloned with every draft of the user, not only this session's.
|
||||
newFork?: boolean
|
||||
items: Item[]
|
||||
conflicts?: DraftItemRef[]
|
||||
error?: string
|
||||
@@ -45,8 +48,13 @@
|
||||
>
|
||||
<div class="flex flex-col gap-3">
|
||||
<p>
|
||||
The session will act on <span class="font-medium text-primary">{targetName}</span>. These
|
||||
drafts are copied there:
|
||||
The session will act on <span class="font-medium text-primary">{targetName}</span>.
|
||||
{#if newFork}
|
||||
The new fork starts with a copy of all your drafts in {parentName}, including these from
|
||||
this session:
|
||||
{:else}
|
||||
These drafts are copied there:
|
||||
{/if}
|
||||
</p>
|
||||
<ul class="flex flex-col gap-0.5 max-h-48 overflow-y-auto text-xs">
|
||||
{#each items as item (`${item.kind}:${item.path}`)}
|
||||
@@ -61,13 +69,13 @@
|
||||
{/each}
|
||||
</ul>
|
||||
<div class="flex items-start gap-2 border rounded-md p-3 bg-surface-secondary">
|
||||
<Toggle size="xs" bind:checked={removeFromParent} />
|
||||
<div class="flex flex-col">
|
||||
<Toggle size="xs" id="move-remove-from-parent" bind:checked={removeFromParent} />
|
||||
<label for="move-remove-from-parent" class="flex flex-col cursor-pointer">
|
||||
<span class="text-xs font-medium text-primary">Remove these drafts from {parentName}</span>
|
||||
<span class="text-3xs text-tertiary"
|
||||
>Leaves {parentName} at its deployed versions. The work then exists only in the fork.</span
|
||||
>
|
||||
</div>
|
||||
</label>
|
||||
</div>
|
||||
{#if conflicts.length > 0}
|
||||
<p class="text-xs text-red-600 dark:text-red-400">
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
import { DraftService, WorkspaceService, type DraftItemRef } from '$lib/gen'
|
||||
import WorkspaceFamilyPicker from './WorkspaceFamilyPicker.svelte'
|
||||
import SessionMoveModal from './SessionMoveModal.svelte'
|
||||
import { moveSessionToFork, type MoveTarget } from './moveSessionToFork'
|
||||
import { flushPendingSaves, moveSessionToFork, type MoveTarget } from './moveSessionToFork'
|
||||
import { maskHasDraftRow } from './modifiedItemsMask'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import Toggle from '$lib/components/Toggle.svelte'
|
||||
@@ -375,6 +375,12 @@
|
||||
const mask = sessionMask()
|
||||
let items: MoveRequest['items']
|
||||
let conflicts: DraftItemRef[] = []
|
||||
// The list comes from the server, so an edit still debounced would be left out of it.
|
||||
const unsaved = await flushPendingSaves(parent)
|
||||
if (unsaved) {
|
||||
sendUserToast(unsaved, true)
|
||||
return
|
||||
}
|
||||
try {
|
||||
// Only the user's own rows move; a legacy workspace-level draft is nobody's.
|
||||
items = (await DraftService.listDrafts({ workspace: parent }))
|
||||
@@ -414,10 +420,12 @@
|
||||
const req = moveRequest
|
||||
if (!session || !req || moving) return
|
||||
moving = true
|
||||
const releaseAutoResume = runtime?.manager.holdAutoResume()
|
||||
try {
|
||||
const res = await moveSessionToFork(session.id, req.parent, req.target, {
|
||||
items: req.items.map(({ kind, path }) => ({ kind, path })),
|
||||
removeFromParent: moveRemoveFromParent
|
||||
removeFromParent: moveRemoveFromParent,
|
||||
isBusy: () => !canMoveToFork
|
||||
})
|
||||
if (!res.ok) {
|
||||
if (!req.open) {
|
||||
@@ -443,6 +451,7 @@
|
||||
sendUserToast(`Session moved to ${req.targetName}`)
|
||||
} finally {
|
||||
moving = false
|
||||
releaseAutoResume?.()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -709,6 +718,7 @@
|
||||
open={moveRequest.open}
|
||||
parentName={moveRequest.parentName}
|
||||
targetName={moveRequest.targetName}
|
||||
newFork={moveRequest.target.kind === 'new'}
|
||||
items={moveRequest.items}
|
||||
conflicts={moveRequest.conflicts}
|
||||
error={moveRequest.error}
|
||||
|
||||
@@ -192,10 +192,16 @@
|
||||
// is premium-only (backend caps it per paid seat). DisableWorkspaceForking on the active workspace
|
||||
// (a locked prod) doesn't apply when there's a dev to fork from instead — the dev isn't locked, and
|
||||
// devOfRoot only resolves when the user is a member of it.
|
||||
// In `forksOf` mode the fork is always based on that workspace, so only its own rule counts.
|
||||
const forksGateOpen = $derived(
|
||||
(!isCloudHosted() || $maybePremium) &&
|
||||
$workspaceStore !== 'admins' &&
|
||||
(canCreateFork($userStore) || !!devOfRoot)
|
||||
(forksOf
|
||||
? forksOf !== 'admins' &&
|
||||
!rootRulesetsResource.loading &&
|
||||
!rootUserInfoResource.loading &&
|
||||
(!isRuleActiveInRulesets(rootRulesets, 'DisableWorkspaceForking') ||
|
||||
canUserBypassRuleKindInRulesets(rootRulesets, 'DisableWorkspaceForking', rootUserInfo))
|
||||
: $workspaceStore !== 'admins' && (canCreateFork($userStore) || !!devOfRoot))
|
||||
)
|
||||
// A fork is a new workspace, so it's subject to the community-edition cap on
|
||||
// the number of non-'admins' workspaces (backend _check_nb_of_workspaces,
|
||||
|
||||
@@ -15,6 +15,26 @@ export type MoveSessionResult =
|
||||
| { ok: true; forkId: string; copied: number; removed: number }
|
||||
| { ok: false; error: string; conflicts?: DraftItemRef[] }
|
||||
|
||||
/**
|
||||
* Send every debounced draft save for `workspace` to the server. Returns an error message
|
||||
* when one of them failed or conflicted.
|
||||
*
|
||||
* A save still debounced in an open editor would otherwise land in the parent after the
|
||||
* move: re-creating a draft just removed there, or missing from the copy. Three debounces
|
||||
* stand between a keystroke and the server, and each hands on to the next through an
|
||||
* effect, hence the ticks.
|
||||
*/
|
||||
export async function flushPendingSaves(workspace: string): Promise<string | undefined> {
|
||||
flushPendingKeystrokes()
|
||||
await tick()
|
||||
flushEditorSaves(workspace)
|
||||
await tick()
|
||||
const unsaved = await UserDraftDbSyncer.flushWorkspace(workspace)
|
||||
if (unsaved.length > 0) {
|
||||
return `Could not save ${unsaved.map((q) => q.path).join(', ')} before moving. Resolve it and retry.`
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Move a session into a fork of its workspace, bringing its drafts along. The session's
|
||||
* workspace changes only once its drafts are where they must be, so a failure at any step
|
||||
@@ -28,23 +48,18 @@ export async function moveSessionToFork(
|
||||
sessionId: string,
|
||||
parent: string,
|
||||
target: MoveTarget,
|
||||
opts: { items: DraftItemRef[]; removeFromParent: boolean }
|
||||
): Promise<MoveSessionResult> {
|
||||
// A save still debounced in an open editor would otherwise land in the parent after the
|
||||
// move: re-creating a draft just removed there, or missing from the copy. Three debounces
|
||||
// stand between a keystroke and the server, and each hands on to the next through an
|
||||
// effect, hence the ticks.
|
||||
flushPendingKeystrokes()
|
||||
await tick()
|
||||
flushEditorSaves(parent)
|
||||
await tick()
|
||||
const unsaved = await UserDraftDbSyncer.flushWorkspace(parent)
|
||||
if (unsaved.length > 0) {
|
||||
return {
|
||||
ok: false,
|
||||
error: `Could not save ${unsaved.map((q) => q.path).join(', ')} before moving. Resolve it and retry.`
|
||||
}
|
||||
opts: {
|
||||
items: DraftItemRef[]
|
||||
removeFromParent: boolean
|
||||
// A turn running in the parent keeps writing there while its drafts move away.
|
||||
isBusy?: () => boolean
|
||||
}
|
||||
): Promise<MoveSessionResult> {
|
||||
const busy = { ok: false as const, error: 'A chat turn is running. Retry once it ends.' }
|
||||
const unsaved = await flushPendingSaves(parent)
|
||||
if (unsaved) return { ok: false, error: unsaved }
|
||||
|
||||
if (opts.isBusy?.()) return busy
|
||||
|
||||
let forkId: string
|
||||
let copied = 0
|
||||
@@ -53,6 +68,7 @@ export async function moveSessionToFork(
|
||||
if (target.kind === 'new') {
|
||||
const created = await materializeFork(target.fork)
|
||||
if (!created) return { ok: false, error: 'Could not create the fork' }
|
||||
if (opts.isBusy?.()) return busy
|
||||
forkId = created
|
||||
copied = opts.items.length
|
||||
if (opts.removeFromParent && opts.items.length > 0) {
|
||||
|
||||
Reference in New Issue
Block a user