Files
windmill/frontend/src/lib/components/UpdateDevWorkspaceModal.svelte
T
GuilhemandClaude Opus 5 616d4fe167 fix: edit-in-dev-workspace dead-ends, wraps, and misses the tree view (#10354)
* fix: stop the homepage edit-in-fork button from wrapping

* fix: show the full edit-in-fork label anywhere on the button

* fix: thread showEditButton through the homepage tree view

* fix: match the edit-in-fork button styling to the normal edit button

* fix: edit in dev workspace dead-ends on items the dev workspace lacks

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: pull the item's folder before copying it into the dev workspace

* fix: speak the compare page's update vocabulary in the dev-workspace prompt

* fix: raw app with no stylesheet was undeployable across workspaces

* fix: drop the raw-app stylesheet workaround now that the backend serves one

The frontend wrapped `getRawAppData` to report a missing `.css` as empty,
because a raw app with no stylesheet stores no css blob and the shared deploy
treats the resulting 404 as fatal. #10364 fixed that at the source: the backend
now serves an empty body for a missing stylesheet, so the wrapper guards a 404
that no longer happens.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: drop the fork icon from the edit-in-dev-workspace affordances

The row button carried both a pen and a fork, and the menu entries and detail
page buttons carried a fork alone — where the menus already used that same icon
for Duplicate/Fork, so the two entries were indistinguishable. The action is an
edit, so it takes the pen everywhere, matching the ordinary Edit button.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: send edit in dev workspace to the item's editor

The affordance landed on the item's page in the dev workspace and left the user
to open the editor from there. It says "Edit", so it goes to the editor:
`/scripts/edit/...?workspace=<dev>` and the equivalent for flows and both app
kinds. `?workspace=` still does the workspace switch, which the logged layout
applies on any route.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat: choose the on-behalf-of user when updating the dev workspace

The prompt deployed the item with no say over the identity it would run under,
so an item that ran on behalf of someone in prod silently became the deploying
user's in the dev workspace. It now offers the same choice the compare page
does, under the same rules: shown only when the source item carries an
on_behalf_of, picking anyone but yourself gated on admin/wm_deployers in the
target, and confirming blocked until a choice is made — including while the
lookup that decides whether one is needed is still in flight.

The prompt also stops offering the compare page inline; the confirm button
still leads there when the user can't deploy into the dev workspace.

Two fixes the reused selector needed to work inside a dialog:
- ConfirmationModal takes `confirmDisabled`, which also blocks the Enter binding.
- The popover's z-index is now overridable, and the user picker is portalled.
  A ConfirmationModal renders above the popover layer, and its card is
  transformed for the open transition, which makes it the containing block for
  the picker's `fixed` positioning — so both opened behind, and the picker was
  laid out inside the card instead of the viewport.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: check deploy rights per item before prompting to update the dev workspace

* fix: read the compare page link before closing the dev-workspace prompt

The link is derived from the request the prompt is answering, so closing first
left an empty string to navigate to: refusing users saw the dialog dismiss and
stay put, with no way through to the compare page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: keep the new-tab promise and speak up when a popup is blocked

Three defects found by successive cold reviews of the click-time resolution
added earlier in this branch, each one only reachable once the previous fix
existed:

- Safari refuses `window.open` from any promise continuation however fast it
  resolves, so the tab the editor dropdown opens after its existence probe
  never appeared there. `claimTab` takes the tab inside the click's own
  transient activation and points it at the answer once it lands, releasing it
  when there is nothing to show.
- That left the two halves of the same action disagreeing: the entry promises
  never to navigate the editor away, but when the item turned out to be missing
  the prompt took over and navigated in place. The request now carries
  `openInNewTab`, and every destination the prompt can reach honours it.
- With popups blocked the fallback called `window.open` without checking, so a
  successful deploy closed the prompt and did nothing, silently. It now names
  what it could not open.

`openEditInFork` also takes the workspace explicitly. The four editor dropdowns
computed their label from `opWorkspace` but resolved the action from the
navigation store, and `prodWorkspaceId` feeds `deployItem({ workspaceFrom })` —
so a session pane would have deployed from the wrong workspace.

`checkPathWritePermission` is exported with an injectable folder probe and
covered by table-driven cases, chiefly to pin its two fail-open branches, which
otherwise read as dead code inviting deletion.

The two unrelated whitespace hunks in ScriptBuilder.svelte are the repo's
format-on-save hook fixing pre-existing violations in a file this touched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: create the dev workspace's missing folder without overwriting it

`ensureFolder` delegated to the shared `deployItem`, which re-probes and
switches to `updateFolder` when the folder turns out to exist. Nobody asked for
that folder to be deployed — it is created only so the item has somewhere to
land — so a folder created between the two probes had its owners, ACL, summary
and labels silently replaced with the source workspace's. Creating is now
create-only, and losing that race counts as success: the folder exists, which is
all the caller needed.

The same delegation dropped `default_permissioned_as` and `labels`, which the
shared folder deploy does not send. A folder copied without its create-time
identity rules applies none, so an item landing inside it with no on_behalf_of
of its own resolves to whoever deployed it rather than to the principal the
source folder would have chosen — the exact substitution the rest of this branch
exists to prevent. Both are now carried across.

Also check `window.open` in the no-dev-workspace branch of `openEditInFork`. The
branch beneath it already reported a blocked popup; this one returned as if it
had opened something.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: translate copied folder identity rules into the target workspace

A `u/<username>` names a workspace-local account, so copying a folder's
`default_permissioned_as` verbatim was wrong in two directions: the same
username in the dev workspace can be a different person, who would then be
granted the item; and a username with no account there at all passes the
folder-create check, which is structural, only to fail every subsequent item
deploy on the existence check, including the retry — the folder now exists, so
`ensureFolder` short-circuits and the deploy fails identically, with no way out
of the prompt.

Rules are now resolved source username -> email -> target username, since email
is the only identifier stable across workspaces, and a rule whose principal has
no account in the target is dropped rather than carried. Dropping one makes the
copied folder less restrictive than its source, which is not something to
discover later from an item running as the wrong user, so it is reported.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: refuse to overwrite a concurrent item, and translate every folder principal

Four findings from CI review, all on the implicit half of this flow — the writes
the user did not explicitly ask for.

The item write is now create-only. The shared `deployItem` re-probes and silently
switches to an update, so the caller that acts on an item being *absent* could
still overwrite whoever landed it between the two probes. Rather than
reimplementing the per-kind deploys, the frontend's own provider refuses exactly
the three writes that branch reaches for — `updateFlow`, `updateApp`/
`updateAppRaw`, and a `createScript` carrying a `parent_hash`, which is what
makes an otherwise identical create an update. A refusal reports `conflict`, and
the prompt opens their version instead of replacing it.

Folder principals are translated rather than copied. `u/<username>` is
workspace-local, so a verbatim copy either names nobody or names a different
account that happens to share the username. Users now resolve source username ->
email -> target username, and the two kinds of unresolvable principal are
separated because they fail differently: an owner or ACL entry is dropped, which
can only narrow the folder and leaves the creator owning it; an identity rule
refuses the copy outright, because dropping it runs the item as the deployer and
keeping it creates a folder the server then rejects every deploy into.

Groups resolve against `listGroups` rather than `listGroupNames`, which unions in
instance groups that folder rules do not resolve against — a same-named instance
group would otherwise let an unusable rule through.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: read every page of workspace groups before judging a folder principal

`listGroups` paginates, and the `perPage: 100` it was called with is narrower
than the server's own default of 1000 — so a group past the first page read as
having no account in the target. Since an unresolvable identity rule now refuses
the whole folder copy, that turned into a refusal naming a group that does
exist, and an owner or ACL entry on a later page was dropped silently. Read
until a page comes back short, with a size check as the backstop for a server
that ignores `page`.

`list_users` is unpaginated, so the user half of the same lookup was never
affected.

Also move `makeProvider`'s doc block back onto `makeProvider`; adding
`DeployConflict` had left it documenting the type instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: reattach principalTranslator's doc block to principalTranslator

Adding `workspaceGroupNames` above it left the block documenting the helper,
the same way adding `DeployConflict` had displaced `makeProvider`'s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 14:21:14 +02:00

349 lines
13 KiB
Svelte

<script lang="ts">
import ConfirmationModal from '$lib/components/common/confirmationModal/ConfirmationModal.svelte'
import { Alert } from '$lib/components/common'
import { base } from '$lib/base'
import { goto } from '$lib/navigation'
import { sendUserToast } from '$lib/toast'
import { updateDevWorkspaceModal } from '$lib/utils/editInForkModal.svelte'
import { claimTab, devWorkspaceEditUrl } from '$lib/utils/editInFork'
import {
checkItemDeployAccess,
checkItemExists,
createFolderIfAbsent,
deployItem,
getOnBehalfOfOrThrow,
type DeployResult,
type DeployTargetAccess
} from '$lib/utils_workspace_deploy'
import { COMPARE_ITEMS_PARAM } from '$lib/components/sessions/modifiedItemsMask'
import OnBehalfOfSelector, {
needsOnBehalfOfSelection,
type OnBehalfOfChoice,
type OnBehalfOfDetails
} from '$lib/components/OnBehalfOfSelector.svelte'
import Tooltip from '$lib/components/Tooltip.svelte'
const pending = $derived(updateDevWorkspaceModal.val)
let updating = $state(false)
/** The user picker stacks above this dialog and owns the keyboard while it's up (`keyListen`). */
let pickerOpen = $state(false)
/**
* A lookup tagged with the request it answers. Requests outlive the prompt that started them
* (no abort signal on the client), so cancelling and reopening leaves two in flight — without
* the tag the last to settle decides this item's permissions and identity. `undefined` means
* "not looked up yet", which is what gates confirming.
*/
type Tagged<T> = { req: NonNullable<typeof pending>; value: T }
function forPending<T>(res: Tagged<T> | undefined): Tagged<T> | undefined {
return res && res.req === pending ? res : undefined
}
// Re-run per opened item: the modal is mounted for the whole session, the rules can change under
// it, and write access is per-path so it can differ between two items in the same workspace.
let accessLookup = $state<Tagged<DeployTargetAccess> | undefined>(undefined)
let sourceLookup = $state<Tagged<{ onBehalfOf?: string; failed?: boolean }> | undefined>(
undefined
)
$effect(() => {
const req = pending
accessLookup = undefined
sourceLookup = undefined
onBehalfOfChoice = undefined
customOnBehalfOf = undefined
if (!req) return
let live = true
void checkItemDeployAccess(req.devWorkspaceId, req.itemPath).then((value) => {
if (live) accessLookup = { req, value }
})
// `failed` rather than `undefined`: an unreadable source is not one with no identity, and
// conflating them would quietly hand the copy to the deploying user.
void getOnBehalfOfOrThrow(req.itemType, req.itemPath, req.prodWorkspaceId).then(
(onBehalfOf) => {
if (live) sourceLookup = { req, value: { onBehalfOf } }
},
() => {
if (live) sourceLookup = { req, value: { failed: true } }
}
)
return () => {
live = false
}
})
const access = $derived(forPending(accessLookup))
const permission = $derived(access?.value.permission)
// The compare page's update direction (prod -> dev) with this one item preselected. Where the
// confirm button leads when the user can't deploy here, so the request still has somewhere to go.
const compareHref = $derived(
pending
? `${base}/forks/compare?workspace_id=${encodeURIComponent(pending.devWorkspaceId)}` +
`&mode=fork&dir=update` +
`&${COMPARE_ITEMS_PARAM}=${encodeURIComponent(`${pending.itemType}:${pending.itemPath}`)}`
: ''
)
// Falls open while the check is in flight so the modal doesn't flash a refusal it may retract;
// confirming stays blocked until it lands (see `confirmBlocked`).
const canDeploy = $derived(permission?.ok !== false)
// Identity the item will run under once it lands in the dev workspace. Offered only when the
// prod item has an on_behalf_of of its own — otherwise there is no identity to carry over and
// the deploying user is the only sensible answer (`needsOnBehalfOfSelection`).
const sourceOnBehalfOf = $derived(forPending(sourceLookup))
const showOnBehalfOf = $derived(
!!pending && needsOnBehalfOfSelection(pending.itemType, sourceOnBehalfOf?.value.onBehalfOf)
)
// Left unset until the user picks, and confirming is blocked meanwhile. The selector's own
// "preserve the target's value" default can't apply: the item is absent from the dev workspace.
let onBehalfOfChoice = $state<OnBehalfOfChoice>(undefined)
let customOnBehalfOf = $state<OnBehalfOfDetails | undefined>(undefined)
// 'me' is sent explicitly rather than left blank. Sending nothing means "no preference", which
// lets the target folder's `default_permissioned_as` claim the item — so the option labelled
// "me" would deploy it as somebody else. No choice at all (selector hidden) still defers to it.
const chosenIdentity = $derived(
onBehalfOfChoice === 'custom'
? customOnBehalfOf
: onBehalfOfChoice === 'me'
? access?.value.me
: undefined
)
const onBehalfOfUnset = $derived(showOnBehalfOf && onBehalfOfChoice === undefined)
// Blocked until both lookups land *for this item* — Enter is bound to confirm, so a fast one
// would otherwise deploy past the permission check and skip a required choice. Not blocked once
// refused: the button leads to the compare page then.
const sourceOnBehalfOfFailed = $derived(!!sourceOnBehalfOf?.value.failed)
// An identity has to be picked but we don't know who "me" is there, so no choice can be honoured:
// sending nothing would hand the item to the folder default instead.
const targetIdentityUnknown = $derived(showOnBehalfOf && !!access && !access.value.me)
const confirmBlocked = $derived(
canDeploy &&
(!access ||
!sourceOnBehalfOf ||
sourceOnBehalfOfFailed ||
targetIdentityUnknown ||
onBehalfOfUnset)
)
function close() {
updateDevWorkspaceModal.val = undefined
}
/**
* Deploying `f/<folder>/<name>` into a workspace with no `<folder>` succeeds but orphans the
* item — it lands with no folder to carry its permissions. Anything else it needs (resources,
* variables, resource types) stays the compare page's job, exactly as it is there.
*/
async function ensureFolder(req: NonNullable<typeof pending>): Promise<DeployResult> {
const folder = req.itemPath.match(/^f\/([^/]+)\//)?.[1]
if (!folder) return { success: true }
const folderPath = `f/${folder}`
try {
if (await checkItemExists('folder', folderPath, req.devWorkspaceId)) return { success: true }
} catch (e) {
// The one probe that must not fail open: deploying while the folder is in fact missing is
// the orphaning above, and nothing downstream would catch it.
return { success: false, error: `could not check whether ${folderPath} exists (${e})` }
}
// Create-only: nobody asked for this folder to be deployed, so it must never overwrite one
// that appeared meanwhile. See `createFolderIfAbsent`.
const result = await createFolderIfAbsent(folder, req.prodWorkspaceId, req.devWorkspaceId)
if (result.droppedAccess?.length) {
// Narrower than the source rather than wider, so it doesn't block the deploy — but it is
// still not what the folder looked like where it came from.
sendUserToast(
`${folderPath} was created without access for ${result.droppedAccess.join(', ')} — ` +
`no such user or group in ${req.devWorkspaceName}`
)
}
return result
}
async function presenceInDev(
req: NonNullable<typeof pending>
): Promise<'present' | 'absent' | 'unknown'> {
try {
return (await checkItemExists(req.itemType, req.itemPath, req.devWorkspaceId))
? 'present'
: 'absent'
} catch {
return 'unknown'
}
}
async function confirm() {
const req = pending
if (!req || updating) return
// Claimed before the first `await`, for the same reason the dropdown entry claims one: a tab
// opened from a promise continuation never appears on Safari. Released on every path that
// leaves the prompt up, so a retry starts from a clean slate.
const tab = req.openInNewTab ? claimTab() : undefined
async function leaveTo(url: string, destination: string) {
if (tab) tab.show(url)
else if (req!.openInNewTab) {
// The claim was blocked, and so is this. Every caller has already closed the prompt and
// may have deployed, so staying silent would read as the confirm having done nothing.
if (!window.open(url)) sendUserToast(`Allow popups to open ${destination}`, true)
} else await goto(url)
}
const itemInDev = `${req.itemPath} in ${req.devWorkspaceName}`
if (!canDeploy) {
// Read before closing: the href is derived from the request being answered, so clearing it
// first leaves nothing to navigate to.
const href = compareHref
close()
await leaveTo(href, 'the compare page')
return
}
updating = true
// Folders carry no on_behalf_of, so only the item itself takes one.
let result = await ensureFolder(req)
if (result.success) {
// The prompt is only up because the item was absent, so this asks once more before writing
// and the write itself refuses to become an update (`createOnly` below). Between them,
// whoever landed it meanwhile is opened rather than overwritten.
const presence = await presenceInDev(req)
if (presence === 'present') {
updating = false
close()
sendUserToast(`${req.itemPath} is already in ${req.devWorkspaceName}, opening it`)
await leaveTo(
devWorkspaceEditUrl(req.itemType, req.itemPath, req.devWorkspaceId),
itemInDev
)
return
}
if (presence === 'unknown') {
// Someone may have landed it meanwhile and writing would overwrite them, so this probe
// can't fail open either. Prompt stays up so a retry is one click away.
updating = false
tab?.discard()
sendUserToast(
`Could not check whether ${req.itemPath} is already in ${req.devWorkspaceName}`,
true
)
return
}
const deployed = await deployItem({
kind: req.itemType,
path: req.itemPath,
workspaceFrom: req.prodWorkspaceId,
workspaceTo: req.devWorkspaceId,
onBehalfOf: chosenIdentity?.email,
onBehalfOfPrincipal: chosenIdentity?.permissionedAs,
createOnly: true
})
if (deployed.conflict) {
// Landed between the probe above and the write, and `createOnly` refused rather than
// replacing it. Their version stands; open it, as the probe's own branch does.
updating = false
close()
sendUserToast(`${req.itemPath} is already in ${req.devWorkspaceName}, opening it`)
await leaveTo(
devWorkspaceEditUrl(req.itemType, req.itemPath, req.devWorkspaceId),
itemInDev
)
return
}
result = deployed
}
updating = false
if (!result.success) {
// Kept open so the failure is attached to the item it happened on — a lone item can
// fail to stand on its own (missing resource, resource type...).
tab?.discard()
sendUserToast(
`Could not update ${req.devWorkspaceName} with ${req.itemPath}: ${result.error}`,
true
)
return
}
close()
await leaveTo(devWorkspaceEditUrl(req.itemType, req.itemPath, req.devWorkspaceId), itemInDev)
}
</script>
<ConfirmationModal
open={!!pending}
type="info"
title="{pending?.devWorkspaceName} is behind on this item"
confirmationText={canDeploy ? 'Update and edit' : 'Open compare page'}
loading={updating}
keyListen={!pickerOpen}
confirmDisabled={confirmBlocked}
onConfirmed={confirm}
onCanceled={close}
>
{#if pending}
<p>
<span class="font-mono">{pending.itemPath}</span>
exists in <b>{pending.prodWorkspaceId}</b> but not in its dev workspace
<b>{pending.devWorkspaceName}</b>.
</p>
{#if canDeploy}
<p class="mt-2">
Update <b>{pending.devWorkspaceName}</b> with it to edit it there.
</p>
{#if sourceOnBehalfOfFailed}
<div class="mt-2">
<Alert type="error" size="xs" title="Could not read {pending.itemPath}">
Its "run on behalf of" user is unknown, so updating could silently reassign the item to
you. Retry from the compare page.
</Alert>
</div>
{:else if targetIdentityUnknown}
<div class="mt-2">
<Alert
type="error"
size="xs"
title="Could not read your account in {pending.devWorkspaceName}"
>
This item needs a "run on behalf of" user and none can be applied without it. Retry from
the compare page.
</Alert>
</div>
{:else if showOnBehalfOf}
<div class="mt-3 flex items-center gap-2">
<span class="text-xs text-secondary">Runs on behalf of</span>
<OnBehalfOfSelector
targetWorkspace={pending.devWorkspaceId}
targetValue={undefined}
selected={onBehalfOfChoice}
onSelect={(choice, details) => {
onBehalfOfChoice = choice
if (details) customOnBehalfOf = details
}}
kind={pending.itemType}
canPreserve={access?.value.canPreserveOnBehalfOf ?? false}
customValue={customOnBehalfOf?.permissionedAs}
aboveConfirmationModal
onPickerOpenChange={(open) => (pickerOpen = open)}
myPermissionedAs={access?.value.me?.permissionedAs}
/>
</div>
{#if onBehalfOfUnset}
<span class="text-xs text-yellow-600">
You must set the "on behalf of" user before updating
<Tooltip class="text-yellow-600">
The "run on behalf of" field defines which user's permissions will be applied during
execution. Make sure this is set to an appropriate user before updating.
</Tooltip>
</span>
{/if}
{/if}
{:else if permission}
<div class="mt-2">
<Alert type="warning" size="xs" title="You can't update {pending.devWorkspaceName}">
{permission.reason}
</Alert>
</div>
{/if}
{/if}
</ConfirmationModal>