refactor: own the acting-user resolution in one composable

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY
This commit is contained in:
Diego Imbert
2026-09-08 22:27:09 +02:00
co-authored by Claude Opus 5
parent 9166bc06d3
commit b67183d212
7 changed files with 144 additions and 154 deletions
+51
View File
@@ -0,0 +1,51 @@
import { untrack } from 'svelte'
import { fromStore } from 'svelte/store'
import { userStore, workspaceStore, type UserExt } from '$lib/stores'
import { getUserExt } from '$lib/user'
/**
* The user acting in a workspace that is not necessarily the one the top nav points at — an AI
* session or a workspace-specific variant acts on a workspace the nav deliberately is not on.
*
* `$userStore` is loaded for the navigation workspace and answers only for that one, so it is
* returned as-is there and costs no request. Any other workspace is asked once and cached under
* its own id; keying the cache by workspace is what makes a superseded lookup harmless, since a
* late answer can only ever land under the question it was asked.
*
* An unresolved user is `undefined`, and must never fall back to the navigation user, whose
* rights belong to another workspace — `canWrite`/`isOwner` refuse for an unknown user, which is
* the only safe answer. Callers that must not show that refusal as a denial ask `resolved` first.
*/
export function useActingUser(workspace: () => string | undefined) {
const navWorkspace = fromStore(workspaceStore)
const navUser = fromStore(userStore)
// A failed lookup is cached as `undefined` under its key, so it refuses rather than
// retrying on every read.
let others: Record<string, UserExt | undefined> = $state({})
$effect(() => {
const ws = workspace()
if (!ws || ws === navWorkspace.current) return
if (ws in others) return
untrack(() => {
getUserExt(ws).then((u) => (others[ws] = u))
})
})
function userIn(ws: string | undefined): UserExt | undefined {
if (!ws) return undefined
return ws === navWorkspace.current ? navUser.current : others[ws]
}
return {
/** The acting user in `ws`, or `undefined` when it is not known. Only workspaces this
* hook has been pointed at are looked up; the rest read as unknown. */
in: userIn,
/** Whether `ws` has an answer at all — a resolved user, or a lookup that failed. */
resolved: (ws: string | undefined): boolean =>
!!ws && (ws === navWorkspace.current || ws in others),
get current(): UserExt | undefined {
return userIn(workspace())
}
}
}
@@ -9,7 +9,7 @@
} from '$lib/gen'
import { canWrite } from '$lib/utils'
import { createEventDispatcher, onDestroy, untrack } from 'svelte'
import { userStore, workspaceStore } from '$lib/stores'
import { workspaceStore } from '$lib/stores'
import { sendUserToast } from '$lib/toast'
import { clearJsonSchemaResourceCache } from './schema/jsonSchemaResource.svelte'
import ResourceForm from './ResourceForm.svelte'
@@ -17,8 +17,7 @@
import { invalidateWorkspacePaths } from './PathNameAutocomplete.svelte'
import Alert from './common/alert/Alert.svelte'
import { resource } from 'runed'
import { getUserExt } from '$lib/user'
import type { UserExt } from '$lib/stores'
import { useActingUser } from '$lib/actingUser.svelte'
import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte'
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
@@ -91,18 +90,7 @@
let initialStates: Record<string, ResourceState> = $state({})
let existedInitially: Record<string, boolean> = $state({})
let fetchedResources: Record<string, Resource> = $state({})
// The user acting in each loaded workspace other than the navigation one, fetched
// alongside the resource. `undefined` stands for "we don't know" — a lookup still in
// flight or one that failed. Read through `actingUserIn`, never directly.
let perWsUser: Record<string, UserExt | undefined> = $state({})
/** The user acting in `ws`. `$userStore` is loaded for the navigation workspace and
* answers only for that one; anywhere else the lookup above answers, and `undefined`
* must never borrow the navigation user's rights — `canWrite` refuses for it. */
function actingUserIn(ws: string | undefined): UserExt | undefined {
if (!ws) return undefined
return ws === $workspaceStore ? $userStore : perWsUser[ws]
}
const acting = useActingUser(() => selected)
const handlesArray = UserDraft.useMany<ResourceState>(() =>
workspaceSpecs.map((s) => ({
@@ -237,7 +225,7 @@
() => deployedPath,
() => deployedUrl,
() => resource_type,
() => actingUserIn(selected)?.is_admin
() => acting.in(selected)?.is_admin
],
async ([ws, path, _url, type, admin]) =>
ws && path && type === 'git_repository' && admin
@@ -254,14 +242,15 @@
let resourceToEdit: Resource | undefined = $derived(
selected ? fetchedResources[selected] : undefined
)
let can_write = $derived.by(() => {
// A resource that does not exist yet has nobody's permissions on it. In edit mode the
// resource and the acting user land together, so a missing one is also a missing
// other, and neither may read as writable.
// `undefined` until both the resource and the acting user have landed — a pending verdict
// is neither a grant nor the denial the read-only alert announces, so the two must stay
// distinguishable.
let can_write: boolean | undefined = $derived.by(() => {
// A resource that does not exist yet has nobody's permissions on it.
if (!initialPath || !selected) return true
const r = fetchedResources[selected]
if (!r) return false
return canWrite(current?.path ?? initialPath, r.extra_perms ?? {}, actingUserIn(selected))
if (!r || !acting.resolved(selected)) return undefined
return canWrite(current?.path ?? initialPath, r.extra_perms ?? {}, acting.in(selected))
})
const dirtyWorkspaces = $derived(
@@ -298,8 +287,7 @@
dirtyWorkspaces.every((ws) => {
const r = fetchedResources[ws]
return (
!r ||
canWrite(states[ws]?.draft?.path ?? initialPath, r.extra_perms ?? {}, actingUserIn(ws))
!r || canWrite(states[ws]?.draft?.path ?? initialPath, r.extra_perms ?? {}, acting.in(ws))
)
})
)
@@ -322,11 +310,6 @@
ensureHandle(ws, s)
initialStates[ws] = structuredClone(s)
existedInitially[ws] = false
// A resource being created runs no fetch for the acting user to ride along with,
// so a workspace the navigation store cannot answer for is asked here.
if (ws !== $workspaceStore && !(ws in perWsUser)) {
getUserExt(ws).then((u) => (perWsUser[ws] = u))
}
})
})
@@ -336,45 +319,40 @@
if (!ws || !initialPath) return
if (ws in states) return
untrack(() => {
// `actingUserIn` answers from `$userStore` for the navigation workspace, so only
// another one is worth asking.
const needsUser = ws !== $workspaceStore
Promise.all([
ResourceService.getResource({ workspace: ws, path: initialPath, getDraft: true }),
needsUser ? getUserExt(ws) : undefined
]).then(([r, user]) => {
// `.draft` already holds the editor's `ResourceState` shape.
const savedDraftState = (r as any).draft as ResourceState | undefined
fetchedResources[ws] = r
// Deployed baseline as the dirty-check reference, so the banner
// compares draft-vs-deployed and fires immediately when a draft exists.
const deployedState: ResourceState = {
path: r.path,
description: r.description ?? '',
args: (r.value ?? {}) as any,
labels: r.labels ?? undefined,
wsSpecific: r.ws_specific ?? false
ResourceService.getResource({ workspace: ws, path: initialPath, getDraft: true }).then(
(r) => {
// `.draft` already holds the editor's `ResourceState` shape.
const savedDraftState = (r as any).draft as ResourceState | undefined
fetchedResources[ws] = r
// Deployed baseline as the dirty-check reference, so the banner
// compares draft-vs-deployed and fires immediately when a draft exists.
const deployedState: ResourceState = {
path: r.path,
description: r.description ?? '',
args: (r.value ?? {}) as any,
labels: r.labels ?? undefined,
wsSpecific: r.ws_specific ?? false
}
// Open with the saved draft if present, else the deployed.
const s: ResourceState = savedDraftState ?? deployedState
openedOnDraft[ws] = !!savedDraftState
// Gate BEFORE the handle is acquired: `stopSync` queues on a
// not-yet-live entry, and the form can settle before the effect
// above gets a chance to run. Only worth doing when no draft exists
// yet — where one does, there is no phantom to prevent and
// suspending could only drop a write.
if (!savedDraftState) setGated(ws, true)
ensureHandle(ws, s)
initialStates[ws] = structuredClone(deployedState)
// Draft-only paths (`no_deployed`) have no row — saving must
// CREATE, not update (update 404s).
existedInitially[ws] = !(r as any).no_deployed
// Keep resource_type in sync for the base workspace (controls the schema)
if (ws === effectiveWorkspace) {
resource_type = r.resource_type
}
}
// Open with the saved draft if present, else the deployed.
const s: ResourceState = savedDraftState ?? deployedState
openedOnDraft[ws] = !!savedDraftState
// Gate BEFORE the handle is acquired: `stopSync` queues on a
// not-yet-live entry, and the form can settle before the effect
// above gets a chance to run. Only worth doing when no draft exists
// yet — where one does, there is no phantom to prevent and
// suspending could only drop a write.
if (!savedDraftState) setGated(ws, true)
ensureHandle(ws, s)
initialStates[ws] = structuredClone(deployedState)
// Draft-only paths (`no_deployed`) have no row — saving must
// CREATE, not update (update 404s).
existedInitially[ws] = !(r as any).no_deployed
if (needsUser) perWsUser[ws] = user
// Keep resource_type in sync for the base workspace (controls the schema)
if (ws === effectiveWorkspace) {
resource_type = r.resource_type
}
})
)
})
})
@@ -441,7 +419,7 @@
onDraftStateChange?.(!!initialPath && selectedDirty)
})
$effect(() => {
onCanWriteChange?.(can_write)
onCanWriteChange?.(can_write === true)
})
export function localDraftDeployed(): ResourceState | undefined {
@@ -575,7 +553,9 @@
</Alert>
{/if}
{#if current}
<!-- Held back until there is a verdict: rendering the form against a pending `can_write`
would flash read-only controls at someone who can in fact write. -->
{#if current && can_write !== undefined}
{#key current}
<ResourceForm
bind:path={() => current!.path, setPath}
@@ -597,7 +577,7 @@
{resourceToEdit}
onLoadResourceType={() => resourceTypeResource.refetch()}
workspace={selected}
actingUser={actingUserIn(selected)}
actingUser={acting.in(selected) ?? null}
/>
{/key}
{/if}
@@ -5,10 +5,9 @@
import { History, Loader2, Save } from 'lucide-svelte'
import WsSpecificVersions from './WsSpecificVersions.svelte'
import { userStore, workspaceStore } from '$lib/stores'
import { workspaceStore } from '$lib/stores'
import { isOwner } from '$lib/utils'
import { getUserExt } from '$lib/user'
import { resource } from 'runed'
import { useActingUser } from '$lib/actingUser.svelte'
import LocalDraftBanner from './LocalDraftBanner.svelte'
import OpenInSessionButton from './sessions/OpenInSessionButton.svelte'
import {
@@ -62,24 +61,13 @@
// The editor renders whichever workspace-specific variant `selected` points at, so history has
// to follow it too — otherwise a restore would write over the variant the user is not looking at.
let historyWorkspace = $derived(selected ?? effectiveWorkspace)
// `$userStore` is loaded for the navigation workspace and answers only for that one, so
// only history pointed elsewhere costs a lookup — and only once a resource is open, since
// this drawer outlives every resource it opens.
const otherWsUser = resource(
() => (path && historyWorkspace !== $workspaceStore ? historyWorkspace : undefined),
async (ws) => (ws ? await getUserExt(ws) : undefined)
)
const historyUser = $derived.by(() => {
if (historyWorkspace === $workspaceStore) return $userStore
const u = otherWsUser.current
// `resource` keeps the previous result across a refetch, and a superseded lookup can
// still land last, so a user only answers for the workspace they were fetched for.
return u?.workspace_id === historyWorkspace ? u : undefined
})
// Gated on `path`: this drawer outlives every resource it opens, so there is nothing to
// answer about until one is open.
const historyUser = useActingUser(() => (path ? historyWorkspace : undefined))
// Clearing is irreversible and the backend gates it on ownership, not write access, so the
// verdict has to come from the membership `historyWorkspace` knows about. An unresolved
// user gets no Clear button rather than one computed from another workspace's rights.
let canClearSelected = $derived(isOwner(path ?? '', historyUser, historyWorkspace))
let canClearSelected = $derived(isOwner(path ?? '', historyUser.current, historyWorkspace))
// A close reaches `on:close` on a later flush, by which point a caller that closed this drawer to
// open another editor has already anchored the new one. Clearing then would strip that anchor.
@@ -38,7 +38,9 @@
viewJsonSchema: boolean
jsonError: string
deployTo: string | undefined
can_write: boolean
/** `undefined` while the acting user or the resource is still being resolved: neither a
* grant nor the denial the read-only alert announces. */
can_write: boolean | undefined
resource_type: string | undefined
resourceTypeInfo: ResourceType | undefined
resourceSchema: Schema | undefined
@@ -49,9 +51,10 @@
* defaults to the nav workspace. */
workspace?: string | undefined
/** The user acting in `workspace`, resolved by the editor above. `undefined` while
* that lookup is pending or after it failed: every check below then refuses, rather
* than answering with the navigation user's rights in another workspace. */
actingUser: UserExt | undefined
* `null` while that lookup is pending or after it failed: every check below then
* refuses, rather than answering with the navigation user's rights in another
* workspace. */
actingUser: UserExt | null
/** Fired once the GitLab picker has stored the picked project's token, so a
* form that would otherwise file the URL as a secret knows it holds none. */
onCredentialStored?: () => void
@@ -158,7 +161,7 @@
{#if !hidePath}
<div>
{#if !can_write}
{#if can_write === false}
<div class="my-2">
<Alert type="warning" title="Only read access">
You only have read access to this resource and cannot edit it
@@ -168,13 +171,13 @@
<Label label="Path">
<ResourcePathHint />
<Path
disabled={initialPath != '' && !isOwner(initialPath, actingUser, ws)}
disabled={initialPath != '' && !isOwner(initialPath, actingUser ?? undefined, ws)}
bind:path
{initialPath}
namePlaceholder="resource"
kind="resource"
workspaceOverride={workspace}
actingUser={actingUser ?? null}
{actingUser}
/>
</Label>
</div>
@@ -1,7 +1,7 @@
<script lang="ts">
import { VariableService, WorkspaceService } from '$lib/gen'
import { createEventDispatcher, untrack } from 'svelte'
import { userStore, workspaceStore } from '$lib/stores'
import { workspaceStore } from '$lib/stores'
import { Button } from './common'
import Drawer from './common/drawer/Drawer.svelte'
import DrawerContent from './common/drawer/DrawerContent.svelte'
@@ -20,8 +20,7 @@
import { invalidateWorkspacePaths } from './PathNameAutocomplete.svelte'
import WsSpecificVersions from './WsSpecificVersions.svelte'
import { resource } from 'runed'
import { getUserExt } from '$lib/user'
import type { UserExt } from '$lib/stores'
import { useActingUser } from '$lib/actingUser.svelte'
import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte'
import LocalDraftBanner from './LocalDraftBanner.svelte'
import { isEncryptedDraftValue } from '$lib/encryptedDraft'
@@ -55,20 +54,9 @@
let initialStates: Record<string, VariableState> = $state({})
let existedInitially: Record<string, boolean> = $state({})
let extraPerms: Record<string, Record<string, boolean>> = $state({})
// The user acting in each loaded workspace other than the navigation one, fetched
// alongside the variable. `undefined` stands for "we don't know" — a lookup still in
// flight or one that failed. Read through `actingUserIn`, never directly.
let perWsUser: Record<string, UserExt | undefined> = $state({})
/** The user acting in `ws`. `$userStore` is loaded for the navigation workspace and
* answers only for that one; anywhere else the lookup above answers, and `undefined`
* must never borrow the navigation user's rights — `canWrite` refuses for it. */
function actingUserIn(ws: string | undefined): UserExt | undefined {
if (!ws) return undefined
return ws === $workspaceStore ? $userStore : perWsUser[ws]
}
let selected: string | undefined = $state(undefined)
let pathError = $state('')
const acting = useActingUser(() => selected)
const handlesArray = UserDraft.useMany<VariableState>(() =>
workspaceSpecs.map((s) => ({
@@ -125,8 +113,8 @@
const can_write: boolean | undefined = $derived.by(() => {
if (!selected || !edit) return true
const perms = extraPerms[selected]
if (!perms) return undefined
return canWrite(editPath ?? '', perms, actingUserIn(selected))
if (!perms || !acting.resolved(selected)) return undefined
return canWrite(editPath ?? '', perms, acting.in(selected))
})
const dirtyWorkspaces = $derived(
Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws]))
@@ -170,7 +158,7 @@
const dirtyCanWrite = $derived(
dirtyWorkspaces.every((ws) => {
const perms = extraPerms[ws]
return !perms || canWrite(editPath ?? '', perms, actingUserIn(ws))
return !perms || canWrite(editPath ?? '', perms, acting.in(ws))
})
)
@@ -181,18 +169,12 @@
if (!ws || !p) return
if (ws in states) return
untrack(() => {
// `actingUserIn` answers from `$userStore` for the navigation workspace, so only
// another one is worth asking.
const needsUser = ws !== $workspaceStore
Promise.all([
VariableService.getVariable({
workspace: ws,
path: p,
decryptSecret: false,
getDraft: true
}),
needsUser ? getUserExt(ws) : undefined
]).then(([v, user]) => {
VariableService.getVariable({
workspace: ws,
path: p,
decryptSecret: false,
getDraft: true
}).then((v) => {
// `.draft` already holds the editor's `VariableState` shape.
const savedDraftState = (v as any).draft as VariableState | undefined
// Deployed baseline as the dirty-check reference, so the banner
@@ -215,7 +197,6 @@
// CREATE, not update (update 404s).
existedInitially[ws] = !(v as any).no_deployed
extraPerms[ws] = v.extra_perms ?? {}
if (needsUser) perWsUser[ws] = user
})
})
})
@@ -227,7 +208,6 @@
initialStates = {}
existedInitially = {}
extraPerms = {}
perWsUser = {}
pathError = ''
}
@@ -245,9 +225,6 @@
initialStates[ws] = structuredClone(s)
existedInitially[ws] = false
selected = ws
// A variable being created runs no fetch for the acting user to ride along with, so
// a workspace the navigation store cannot answer for is asked here.
if (ws !== $workspaceStore) getUserExt(ws).then((u) => (perWsUser[ws] = u))
drawer?.openDrawer()
}
@@ -363,7 +340,9 @@
</Alert>
{/if}
{#if current}
<!-- Held back until there is a verdict: rendering the form against a pending `can_write`
would flash read-only controls at someone who can in fact write. -->
{#if current && can_write !== undefined}
{#key current}
<VariableForm
bind:this={form}
@@ -378,7 +357,7 @@
{edit}
onLoadSecret={loadSecret}
workspace={selected}
actingUser={actingUserIn(selected)}
actingUser={acting.in(selected) ?? null}
/>
{/key}
{/if}
@@ -35,9 +35,10 @@
/** Workspace the path is validated against; defaults to the nav workspace. */
workspace?: string | undefined
/** The user acting in `workspace`, resolved by the editor above. `undefined` while
* that lookup is pending or after it failed: every check below then refuses, rather
* than answering with the navigation user's rights in another workspace. */
actingUser: UserExt | undefined
* `null` while that lookup is pending or after it failed: every check below then
* refuses, rather than answering with the navigation user's rights in another
* workspace. */
actingUser: UserExt | null
}
let {
@@ -76,14 +77,14 @@
<div class="flex flex-col gap-1">
<label for="path" class="text-xs font-semibold text-emphasis">Path</label>
<Path
disabled={initialPath != '' && !isOwner(initialPath, actingUser, ws)}
disabled={initialPath != '' && !isOwner(initialPath, actingUser ?? undefined, ws)}
bind:error={pathError}
bind:path
{initialPath}
namePlaceholder="variable"
kind="variable"
workspaceOverride={workspace}
actingUser={actingUser ?? null}
{actingUser}
/>
<LabelsInput bind:labels />
</div>
@@ -29,7 +29,7 @@
type Schedule,
type ErrorHandler
} from '$lib/gen'
import { enterpriseLicense, userStore, workspaceStore, type UserExt } from '$lib/stores'
import { enterpriseLicense, workspaceStore } from '$lib/stores'
import { canWrite, emptyString, formatCron, sendUserToast, cronV1toV2 } from '$lib/utils'
import { base } from '$lib/base'
import Section from '$lib/components/Section.svelte'
@@ -50,8 +50,7 @@
import { twMerge } from 'tailwind-merge'
import PermissionedAsLine from '../PermissionedAsLine.svelte'
import { getTriggerWorkspace } from '$lib/components/triggers/triggerWorkspace'
import { getUserExt } from '$lib/user'
import { resource } from 'runed'
import { useActingUser } from '$lib/actingUser.svelte'
let {
useDrawer = true,
@@ -139,21 +138,10 @@
const triggerWs = getTriggerWorkspace()
const wsId = $derived(triggerWs?.() ?? $workspaceStore)
// `$userStore` is loaded for the navigation workspace and only answers for that one, so
// an acting workspace anywhere else has to be asked who is acting in it.
const otherWsUser = resource(
() => (wsId && wsId !== $workspaceStore ? wsId : undefined),
async (ws) => (ws ? await getUserExt(ws) : undefined)
)
// `undefined` while that lookup is in flight or after it failed; the checks below then
// `undefined` while the lookup is in flight or after it failed; the checks below then
// refuse rather than fall back to rights that belong to another workspace.
const actingUser: UserExt | undefined = $derived.by(() => {
if (wsId === $workspaceStore) return $userStore
const u = otherWsUser.current
// `resource` keeps the previous result across a refetch, and a superseded lookup can
// still land last, so a user only answers for the workspace they were fetched for.
return u?.workspace_id === wsId ? u : undefined
})
const acting = useActingUser(() => wsId)
const actingUser = $derived(acting.current)
const can_write = $derived(
permsPath === undefined ? true : canWrite(permsPath, extraPerms, actingUser)
)