From 30b0d1d4da00584177d2693aceeda445b2421cef Mon Sep 17 00:00:00 2001 From: josh Date: Sat, 12 Sep 2026 23:59:43 -0400 Subject: [PATCH] Fix delete FK exhaustively; unset-scope surfaces as drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deleteScript: previous fix missed beta_servers.prior_version_id, beta_server_scripts.prior_version_id, and deployments.beta_id. Cascade from betas.script_id -> beta_servers/beta_server_scripts left dangling prior_version_id refs, and deletions of cascaded betas dangled deployments.beta_id. Now nulls every NO ACTION reference before the DELETE inside a single transaction. Scope unset no longer touches disk from the API. Instead it detaches inventory rows for scripts/plugins so drift Pass 1 flags the on-disk copies as untracked; configs rely on drift Pass 2's compose-diff, which already handles scope=null naturally. The operator reconciles from /deployments using the existing rescan + reconcile buttons — same visible flow as any other drift, no more silent synchronous file removal. Co-Authored-By: Claude Opus 4.7 --- src/lib/server/fleet/beta.ts | 40 ++++++++++------- src/lib/server/fleet/scripts.ts | 46 +++++++++----------- src/routes/api/scripts/[id]/scope/+server.ts | 25 +++-------- 3 files changed, 51 insertions(+), 60 deletions(-) diff --git a/src/lib/server/fleet/beta.ts b/src/lib/server/fleet/beta.ts index 01ad132..361b154 100644 --- a/src/lib/server/fleet/beta.ts +++ b/src/lib/server/fleet/beta.ts @@ -1020,26 +1020,36 @@ export function latestVersionForScript(script: ScriptRow): ScriptVersionRow | nu } /** - * Un-set a script's scope. When scope is being removed and matching servers - * currently have the script, remove it from them for consistency. Returns the - * server names that had files deleted. + * Un-set a script's scope. Files are left on disk so drift picks up the + * change on the next /deployments rescan and the operator reconciles from + * there like any other drift — the same visible flow as manual drift. + * + * For scripts/plugins, we detach the inventory rows for this artifact's + * versions so drift Pass 1 flags the on-disk copies as untracked. For + * configs/baselines, no detach is needed: drift Pass 2 recomposes the + * expected dedicated_sp.cfg against the current promoted overlays, and + * scope=null naturally drops the overlay from the compose, surfacing the + * disparity. Returns the servers whose inventory was detached (empty for + * configs/baselines). */ -export async function unsetScopeAndCleanup(scriptId: number): Promise { +export function unsetScopeAndDetach(scriptId: number): string[] { const script = getScript(scriptId); if (!script) throw new BetaError(`no such script (id=${scriptId})`, 'no-such-script'); - if (!script.scope_type || !script.scope_value) return []; - const matched = serversMatchingScope(script.scope_type, script.scope_value); - const cleaned: string[] = []; - for (const name of matched) { - const dest = destOfKind(name, script.kind); - const targetPath = join(dest, script.target_rel_path); - if (existsSync(targetPath)) { - await rm(targetPath, { force: true }); - removeScript(name, script.target_rel_path); - cleaned.push(name); + const detached: string[] = []; + if (script.kind === 'script' || script.kind === 'plugin') { + const rows = db() + .prepare( + `SELECT server FROM server_scripts + WHERE rel_path = ? + AND script_version_id IN (SELECT id FROM script_versions WHERE script_id = ?)` + ) + .all(script.target_rel_path, scriptId) as Array<{ server: string }>; + for (const { server } of rows) { + removeScript(server, script.target_rel_path); + detached.push(server); } } db().prepare('UPDATE scripts SET scope_type = NULL, scope_value = NULL WHERE id = ?').run(scriptId); - return cleaned; + return detached; } diff --git a/src/lib/server/fleet/scripts.ts b/src/lib/server/fleet/scripts.ts index 543106c..c8a2ae0 100644 --- a/src/lib/server/fleet/scripts.ts +++ b/src/lib/server/fleet/scripts.ts @@ -338,35 +338,29 @@ export function deleteScript(scriptId: number): void { .get(scriptId) as { id: number } | undefined; if (beta) throw new ScriptError(`beta ${beta.id} is still running against this script`, 'has-active-beta'); - // Null out deployment-history FKs that would otherwise block the cascade. - // deployments.script_id/script_version_id/prior_version_id and - // deployment_scripts.prior_version_id are historical audit references - // with no ON DELETE action; keep the deployment rows intact but drop - // the FK to the script so we can remove it. + // Every FK that references scripts(id) or script_versions(id) with NO + // ACTION (the default) will abort the delete unless we null it first. + // ON DELETE CASCADE / SET NULL fires automatically. The list below covers + // every NO ACTION reference in schema.sql; add to it if new ones appear. + // deployments.script_id, script_version_id, prior_version_id, beta_id + // deployment_scripts.prior_version_id + // beta_servers.prior_version_id + // beta_server_scripts.prior_version_id + // deployments.beta_id needs nulling because betas.script_id ON DELETE + // CASCADE nukes any beta bundled against this script, which would then + // dangle deployments.beta_id. const handle = db(); const tx = handle.transaction((id: number) => { - handle - .prepare( - `UPDATE deployments - SET script_version_id = NULL - WHERE script_version_id IN (SELECT id FROM script_versions WHERE script_id = ?)` - ) - .run(id); - handle - .prepare( - `UPDATE deployments - SET prior_version_id = NULL - WHERE prior_version_id IN (SELECT id FROM script_versions WHERE script_id = ?)` - ) - .run(id); + const inMyVersions = 'IN (SELECT id FROM script_versions WHERE script_id = ?)'; + const inMyBetas = 'IN (SELECT id FROM betas WHERE script_id = ? UNION SELECT beta_id FROM beta_scripts WHERE script_id = ?)'; + + handle.prepare(`UPDATE beta_servers SET prior_version_id = NULL WHERE prior_version_id ${inMyVersions}`).run(id); + handle.prepare(`UPDATE beta_server_scripts SET prior_version_id = NULL WHERE prior_version_id ${inMyVersions}`).run(id); + handle.prepare(`UPDATE deployment_scripts SET prior_version_id = NULL WHERE prior_version_id ${inMyVersions}`).run(id); + handle.prepare(`UPDATE deployments SET prior_version_id = NULL WHERE prior_version_id ${inMyVersions}`).run(id); + handle.prepare(`UPDATE deployments SET script_version_id = NULL WHERE script_version_id ${inMyVersions}`).run(id); + handle.prepare(`UPDATE deployments SET beta_id = NULL WHERE beta_id ${inMyBetas}`).run(id, id); handle.prepare('UPDATE deployments SET script_id = NULL WHERE script_id = ?').run(id); - handle - .prepare( - `UPDATE deployment_scripts - SET prior_version_id = NULL - WHERE prior_version_id IN (SELECT id FROM script_versions WHERE script_id = ?)` - ) - .run(id); handle.prepare('DELETE FROM scripts WHERE id = ?').run(id); }); tx(scriptId); diff --git a/src/routes/api/scripts/[id]/scope/+server.ts b/src/routes/api/scripts/[id]/scope/+server.ts index ec6ccd7..c514490 100644 --- a/src/routes/api/scripts/[id]/scope/+server.ts +++ b/src/routes/api/scripts/[id]/scope/+server.ts @@ -1,28 +1,17 @@ import { json } from '@sveltejs/kit'; import { getScript, setScope, ScriptError, type ScopeType } from '$lib/server/fleet/scripts.js'; -import { unsetScopeAndCleanup } from '$lib/server/fleet/beta.js'; -import { runDriftDeployment } from '$lib/server/fleet/drift.js'; -import { kick } from '$lib/server/pipeline/worker.js'; +import { unsetScopeAndDetach } from '$lib/server/fleet/beta.js'; function parseId(raw: string): number | null { const n = Number.parseInt(raw, 10); return Number.isFinite(n) ? n : null; } -// Fire-and-forget: after a scope mutation, reconcile any drift the change may -// have introduced (orphan files on servers that no longer match, or missing -// files on servers that now do). Runs in the background so the API returns -// promptly; the deployment will surface on /deployments if there's real drift. -function triggerDrift(): void { - void runDriftDeployment({ triggeredBy: 'ui' }) - .then((result) => { if (result.deploymentId) void kick(); }) - .catch(() => { /* surfaced on /deployments if it fails */ }); -} - // POST body: { scopeType: 'game' | 'map' | null, scopeValue?: string } // Setting/changing scope on a script with a current_version_id returns a -// 409 with kind='scope-requires-beta'; unsetting cleans the file off matching -// servers. +// 409 with kind='scope-requires-beta'. Un-scoping detaches inventory so the +// orphaned files show up as drift on the deployments page for the operator +// to reconcile manually — the same flow as any other drift. export async function POST({ params, request }: { params: { id: string }; request: Request }) { const scriptId = parseId(params.id); if (scriptId === null) return json({ ok: false, error: 'bad id' }, { status: 400 }); @@ -39,12 +28,10 @@ export async function POST({ params, request }: { params: { id: string }; reques try { if (scopeType === null) { - const cleaned = await unsetScopeAndCleanup(scriptId); - triggerDrift(); - return json({ ok: true, cleaned }); + const detached = unsetScopeAndDetach(scriptId); + return json({ ok: true, detached }); } const updated = setScope({ scriptId, scopeType, scopeValue }); - triggerDrift(); return json({ ok: true, script: updated }); } catch (err) { if (err instanceof ScriptError) {