fix: re-check the picker's target path at the moment it is written

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1xHmkxuxYb1GYvth1BS75
This commit is contained in:
hugocasa
2026-09-03 17:59:24 +02:00
co-authored by Claude Opus 5
parent c15834bd46
commit 3abc1c0402
3 changed files with 38 additions and 11 deletions
+1 -1
View File
@@ -1 +1 @@
64567911809a2feb4cb0383153939d25585d1f7b
37221a36157a91b99d86f96b613c8ecdd119e564
+4 -3
View File
@@ -337,13 +337,14 @@ pub struct GitRepositorySettings {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub auto_pull: Option<AutoPullSettings>,
/// Open a PR when a deploy pushes a `wm_deploy/**` branch of this promotion
/// repo (app-backed only; runs from the deploy callback so it works without
/// repo (needs a credential the server holds — a GitHub App installation or
/// a checked GitLab token; runs from the deploy callback so it works without
/// inbound webhooks). Off by default so upgrades don't change behavior.
#[serde(default, skip_serializing_if = "is_false")]
pub promotion_open_prs: bool,
/// Parent-level: open a PR when a fork of this workspace deploys to its
/// `wm-fork/**` branch (app-backed only; the fork's deploy callback reads
/// this from the parent). Off by default.
/// `wm-fork/**` branch (needs a credential the server holds; the fork's
/// deploy callback reads this from the parent). Off by default.
#[serde(default, skip_serializing_if = "is_false")]
pub fork_open_prs: bool,
/// Server-owned: the last failure opening a PR for a deploy branch of this
@@ -89,7 +89,10 @@
// suggested path can collide with another project's, and the field is free
// text, so the name alone proves nothing: only the repository the stored
// value points at does.
type Occupant = 'free' | 'same-repo' | 'other'
// `checking` exists so a path that has just changed is never treated as the
// previous path's verdict: the answer is asynchronous, and applying against a
// stale one is how an occupied variable gets overwritten anyway.
type Occupant = 'checking' | 'free' | 'same-repo' | 'other'
let occupant: Occupant = $state('free')
$effect(() => {
const path = variablePath
@@ -99,6 +102,7 @@
occupant = 'free'
return
}
occupant = 'checking'
let cancelled = false
VariableService.existsVariable({ workspace, path })
.then(async (exists) => {
@@ -157,17 +161,33 @@
async function apply(close: (_: any) => void) {
// The token is cleared once stored, and re-applying without one would
// overwrite the stored credential with an empty password. A path holding
// another repository's remote is never written: that would repoint every
// resource using it, silently, at this project.
if (!ws || !project || !token || occupant === 'other') return
// overwrite the stored credential with an empty password.
const pathIsWritable = occupant === 'free' || occupant === 'same-repo'
if (!ws || !project || !token || !pathIsWritable) return
applying = true
try {
const value = repositoryUrl(project)
const target = repoIdentity(project.http_url_to_repo)
const exists = await VariableService.existsVariable({
workspace: ws,
path: variablePath
})
// Re-read rather than trust the state the button was enabled from: the
// path can change between the check and the click, and another writer
// can take the path in between. A path holding anything but this same
// repository is never written over — that would repoint every resource
// using it, silently, at this project.
if (exists) {
const current = await VariableService.getVariableValue({
workspace: ws,
path: variablePath
}).catch(() => undefined)
if (!current || !target || repoIdentity(current) !== target) {
occupant = 'other'
sendUserToast(`${variablePath} holds something else. Choose another path.`, true)
return
}
}
if (exists) {
await VariableService.updateVariable({
workspace: ws,
@@ -277,7 +297,9 @@
size="sm"
inputProps={{ oninput: () => (variablePathTouched = true) }}
/>
{#if occupant === 'other'}
{#if occupant === 'checking'}
<div class="text-2xs font-normal text-hint">Checking this path...</div>
{:else if occupant === 'other'}
<div class="text-2xs font-normal text-red-600 dark:text-red-400">
This variable already holds something else. Choose another path, or anything using
it would start pointing at this project.
@@ -292,7 +314,11 @@
<Button
variant="accent"
unifiedSize="sm"
disabled={!project || !variablePath || !token || occupant === 'other' || applying}
disabled={!project ||
!variablePath ||
!token ||
!(occupant === 'free' || occupant === 'same-repo') ||
applying}
startIcon={{
icon: applying ? Loader2 : GitBranch,
classes: applying ? 'animate-spin' : ''