From ebe31aeeac3d6f6508bcc9af4e6eaad8036849f7 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 15 Jul 2026 10:01:35 +0200 Subject: [PATCH] feat(dev-workspace): reflect existing protection rules in lock toggles (#10093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(dev-workspace): reflect existing protection rules in lock toggles When creating or attaching a dev workspace, the "block direct edits" and "prevent forking" toggles now check the root workspace's current protection rules. If a restriction is already enforced by an existing rule, its toggle is shown on but locked, with a note, instead of offering a fresh default that could misrepresent the effect. The value sent to the backend is derived so it stays consistent with what the locked toggle shows. Co-Authored-By: Claude Opus 4.8 (1M context) * docs: clarify fail-open comment on dev-workspace lock toggles Reword the protection-rule fetch comment so the fallback path isn't misread as dropping protection: a failed fetch falls back to the editable default-on toggle, and any real rule still enforces server-side. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(dev-workspace): lock protection toggles until rules load The lock toggles derived alreadyBlocks* from an async fetch, so during the load window (and the first frame before loading flips) they were editable and the effective value could be false. A user could turn a lock off and submit before an existing rule was detected, omitting the reserved rule and silently leaving prod unprotected once that existing rule was later removed. Treat "rules not yet known" (loading || current === undefined) the same as "already enforced": lock the toggle on and keep the effective value true during that window, so the request can never submit false before the fetch resolves. Submission stays available (a hung fetch degrades to over-protection, not a blocked form). Also fixes the stale-value flash when switching base workspace. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(dev-workspace): honor rule bypasses and guard stale protection fetches Two issues in the protection-rule awareness for the dev-workspace lock toggles: - Bypassable rules became unconditional locks. alreadyBlocks* used isRuleActiveInRulesets, which ignores bypass_users/bypass_groups, and forced the request flag to true. The reserved dev_workspace_lock rule is created with empty bypass lists, so layering it over an existing rule that let specific users through revoked their deploy/forking access. Switch to isRuleUnconditionallyActiveInRulesets so a toggle is only shown as already enforced (locked) when an existing rule has no bypasses; a bypassable rule stays editable, making the lock the user's explicit choice. - A stale protection fetch could apply another base's rules. The generated client can't take an abort signal, so a delayed response for a previous base could overwrite the newly selected one. Tag each result with its workspace and only trust a result matching the current base; also throw AbortError from a superseded fetch so it can't overwrite current. Co-Authored-By: Claude Opus 4.8 (1M context) * docs: condense protection helper comment to four lines Trim the isRuleUnconditionallyActiveInRulesets doc comment to satisfy the AGENTS.md ≤4-line comment rule. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(dev-workspace): align already-enforced note under the toggle label The note used ml-8, landing under the toggle switch rather than aligned with the switch edge or the label, so it read as floating. Bump to ml-11 so it lines up under the label as helper text for that toggle. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../lib/components/DevWorkspaceSetting.svelte | 97 +++++++++++++++++-- .../CreateWorkspaceInner.svelte | 90 +++++++++++++++-- .../lib/workspaceProtectionRules.svelte.ts | 17 ++++ 3 files changed, 185 insertions(+), 19 deletions(-) diff --git a/frontend/src/lib/components/DevWorkspaceSetting.svelte b/frontend/src/lib/components/DevWorkspaceSetting.svelte index e6295d24cd..2537105c72 100644 --- a/frontend/src/lib/components/DevWorkspaceSetting.svelte +++ b/frontend/src/lib/components/DevWorkspaceSetting.svelte @@ -10,7 +10,11 @@ import { base } from '$lib/base' import { findCanonicalDevWorkspace } from '$lib/utils/workspaceHierarchy' import { devBadgeText, devLabelKey, devLabelNoun } from '$lib/utils/devWorkspaceLabel' - import { loadProtectionRules } from '$lib/workspaceProtectionRules.svelte' + import { + loadProtectionRules, + fetchProtectionRulesForWorkspace, + isRuleUnconditionallyActiveInRulesets + } from '$lib/workspaceProtectionRules.svelte' import { GitFork, ExternalLink } from 'lucide-svelte' import { resource } from 'runed' @@ -55,6 +59,48 @@ let busy = $state(false) let labelBusy = $state(false) + // If this workspace already blocks direct deploy / forking through an existing protection rule, keep + // the matching lock toggle on but locked: attaching only manages its own reserved dev-workspace rule, + // so turning it "off" here couldn't lift a separately-defined block. Fetched only while the attach form + // is on screen; a failed fetch falls back to the editable default-on toggle (real rules still enforce). + const rootProtectionRules = resource( + () => (!parentId && !pairedDev ? $workspaceStore : undefined), + async (ws, _prev, { signal }) => { + if (!ws) return undefined + const rules = await fetchProtectionRulesForWorkspace(ws) + // The generated client can't take an abort signal, so drop a superseded response here: a late + // result for a previously selected workspace must not overwrite the current one's rules. + if (signal.aborted) throw new DOMException('superseded', 'AbortError') + return { ws, rules } + } + ) + // Only trust a result that belongs to the current workspace (guards the in-flight window and any + // out-of-order response); undefined means "not known yet" and is treated as locked below. + let rootRules = $derived.by(() => { + const current = rootProtectionRules.current + return current && current.ws === $workspaceStore ? current.rules : undefined + }) + // Only a rule with no bypass users/groups matches the empty-bypass reserved lock we would create; a + // bypassable rule stays editable, otherwise forcing the lock on would revoke the bypassed users' + // direct-deploy / forking access. + let alreadyBlocksDeploy = $derived( + isRuleUnconditionallyActiveInRulesets(rootRules ?? [], 'DisableDirectDeployment') + ) + let alreadyBlocksForking = $derived( + isRuleUnconditionallyActiveInRulesets(rootRules ?? [], 'DisableWorkspaceForking') + ) + // Until the fetch resolves for the current workspace its rules are unknown. Treat each lock as + // engaged during that window so the toggle is locked on and the effective value stays true: + // otherwise a user could turn a lock off and attach before an existing rule is detected, sending + // false and omitting the reserved rule — leaving prod unprotected if that rule is later removed. + let rulesUnknown = $derived(rootProtectionRules.loading || rootRules === undefined) + let deployLocked = $derived(alreadyBlocksDeploy || rulesUnknown) + let forkingLocked = $derived(alreadyBlocksForking || rulesUnknown) + // Sent to the backend: a locked restriction (enforced or not-yet-known) stays on regardless of the + // toggle's raw state, keeping the request consistent with what the locked toggle shows. + let effectiveLockProdDeploy = $derived(deployLocked || lockProdDeploy) + let effectiveLockProdForking = $derived(forkingLocked || lockProdForking) + // A standalone root workspace, or an existing fork of this prod (same family), can be attached. // A fork parented to a different workspace can't (the backend rejects a parent that isn't this // prod), so it's excluded here. @@ -92,8 +138,8 @@ workspace: $workspaceStore, requestBody: { dev_workspace_id: selectedDevId, - lock_prod_deploy: lockProdDeploy, - lock_prod_forking: lockProdForking, + lock_prod_deploy: effectiveLockProdDeploy, + lock_prod_forking: effectiveLockProdForking, dev_workspace_label: attachLabel } }) @@ -217,13 +263,44 @@ Change to {attachLabel === 'staging' ? 'dev' : 'staging'} - - + {#if deployLocked} +
+ + {#if alreadyBlocksDeploy} + Already enforced by an existing protection rule + {/if} +
+ {:else} + + {/if} + {#if forkingLocked} +
+ + {#if alreadyBlocksForking} + Already enforced by an existing protection rule + {/if} +
+ {:else} + + {/if}
- - + {#if deployLocked} +
+ + {#if rootAlreadyBlocksDeploy} + Already enforced by an existing protection rule + {/if} +
+ {:else} + + {/if} + {#if forkingLocked} +
+ + {#if rootAlreadyBlocksForking} + Already enforced by an existing protection rule + {/if} +
+ {:else} + + {/if} {/if} diff --git a/frontend/src/lib/workspaceProtectionRules.svelte.ts b/frontend/src/lib/workspaceProtectionRules.svelte.ts index 1774ed60e2..9bca16005f 100644 --- a/frontend/src/lib/workspaceProtectionRules.svelte.ts +++ b/frontend/src/lib/workspaceProtectionRules.svelte.ts @@ -182,6 +182,23 @@ export function isRuleActiveInRulesets( return rulesets.some((ruleset) => ruleset.rules.includes(ruleKind)) } +/** + * Whether a rule kind is enforced with no bypass users/groups in at least one ruleset, the only case + * that matches the empty-bypass reserved dev-workspace lock. A bypassable rule does not, since adding + * the unconditional lock would revoke those users' access; callers keep such a toggle editable. + */ +export function isRuleUnconditionallyActiveInRulesets( + rulesets: ProtectionRuleset[], + ruleKind: ProtectionRuleKind +): boolean { + return rulesets.some( + (ruleset) => + ruleset.rules.includes(ruleKind) && + ruleset.bypass_users.length === 0 && + ruleset.bypass_groups.length === 0 + ) +} + /** * Checks if user can bypass a rule kind in given rulesets (workspace-agnostic version) * @param rulesets Array of protection rulesets to check