diff --git a/backend/.sqlx/query-925359a7b0fe8f4ed17f072f7299c7d503eac045a6d5d13ac3c953a3fbdac0ff.json b/backend/.sqlx/query-925359a7b0fe8f4ed17f072f7299c7d503eac045a6d5d13ac3c953a3fbdac0ff.json new file mode 100644 index 0000000000..59472d78dd --- /dev/null +++ b/backend/.sqlx/query-925359a7b0fe8f4ed17f072f7299c7d503eac045a6d5d13ac3c953a3fbdac0ff.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT versions[array_upper(versions, 1)] FROM app\n WHERE path = $1 AND workspace_id = $2 FOR UPDATE", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "versions", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "925359a7b0fe8f4ed17f072f7299c7d503eac045a6d5d13ac3c953a3fbdac0ff" +} diff --git a/backend/tests/dependency_map.rs b/backend/tests/dependency_map.rs index 0dffcc40b6..4520cae848 100644 --- a/backend/tests/dependency_map.rs +++ b/backend/tests/dependency_map.rs @@ -515,6 +515,7 @@ def main(): preserve_on_behalf_of: None, labels: None, skip_draft_deletion: None, + allow_kind_change: None, }) .send() .await diff --git a/backend/windmill-api-integration-tests/tests/apps.rs b/backend/windmill-api-integration-tests/tests/apps.rs index 3d70c09ce1..cf04e377d2 100644 --- a/backend/windmill-api-integration-tests/tests/apps.rs +++ b/backend/windmill-api-integration-tests/tests/apps.rs @@ -335,3 +335,111 @@ async fn test_public_app_by_custom_path(db: Pool) -> anyhow::Result<() Ok(()) } + +/// A raw app's kind lives on its version row, so a value deployed through the +/// low-code endpoint used to convert the app in place and strand its bundle. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_raw_app_kind_is_not_flipped_by_update(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace/apps"); + let raw_path = "u/test-user/raw_app"; + let low_code_path = "u/test-user/low_code_app"; + + let raw_app_form = |path: &str| { + reqwest::multipart::Form::new() + .text( + "app", + json!({ + "path": path, + "summary": "Raw app", + "value": { "files": { "index.ts": "export {}" }, "runnables": {} }, + "policy": { "execution_mode": "publisher", "triggerables_v2": {} } + }) + .to_string(), + ) + .part( + "js", + reqwest::multipart::Part::bytes(b"console.log(1)".to_vec()), + ) + }; + + let resp = authed(client().post(format!("{base}/create_raw"))) + .multipart(raw_app_form(raw_path)) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "create_raw: {}", resp.text().await?); + + let resp = authed(client().post(format!("{base}/create"))) + .json(&new_app(low_code_path, "Low code app")) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "create: {}", resp.text().await?); + + let versions_of = |path: &'static str| async move { + let resp = authed_get(port, "get/p", path).await; + let body = resp.json::().await.unwrap(); + body["versions"].as_array().unwrap().len() + }; + let raw_versions = versions_of(raw_path).await; + + // Neither endpoint may deploy a value onto an app of the other kind. + let resp = authed(client().post(format!("{base}/update/{raw_path}"))) + .json(&json!({ "value": { "grid": [] } })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 400); + assert!( + resp.text().await?.contains("is a raw app"), + "expected the low-code update of a raw app to be refused" + ); + // The refusal has to land before the version insert, not roll one back. + assert_eq!(versions_of(raw_path).await, raw_versions); + + let resp = authed(client().post(format!("{base}/update_raw/{low_code_path}"))) + .multipart(raw_app_form(low_code_path)) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 400); + assert!( + resp.text().await?.contains("is a low-code app"), + "expected the raw update of a low-code app to be refused" + ); + + // Metadata-only updates and same-kind deploys still go through. + let resp = authed(client().post(format!("{base}/update/{raw_path}"))) + .json(&json!({ "summary": "Renamed" })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "update summary: {}", resp.text().await?); + + let resp = authed(client().post(format!("{base}/update_raw/{raw_path}"))) + .multipart(raw_app_form(raw_path)) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "update_raw: {}", resp.text().await?); + + let resp = authed_get(port, "get/p", raw_path).await; + assert_eq!(resp.json::().await?["raw_app"], true); + + // A caller that means to convert says so, which is how an app converted by + // accident gets restored to what it was. + let resp = authed(client().post(format!("{base}/update/{raw_path}"))) + .json(&json!({ "value": { "grid": [] }, "allow_kind_change": true })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "update: {}", resp.text().await?); + + let resp = authed_get(port, "get/p", raw_path).await; + assert_eq!(resp.json::().await?["raw_app"], false); + + Ok(()) +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 8f3b1a9e41..650f3649a4 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -12299,6 +12299,7 @@ paths: summary: update app operationId: updateApp x-mcp-tool: true + x-mcp-instructions: "Low-code apps only. An app whose `raw_app` field (from getAppByPath) is true is a raw (full-code) app: its value holds source files that must be compiled to a js/css bundle, which this tool cannot upload, so updating one here is refused. Edit a raw app in its editor at /apps_raw/edit/ instead." x-mcp-tool-include-fields: - path - value @@ -12342,6 +12343,9 @@ paths: skip_draft_deletion: type: boolean description: "When true (set by the CLI / git sync), deploying this app does not delete an existing user draft at the same path." + allow_kind_change: + type: boolean + description: "When true, this deploy may switch the app between low-code and raw. Without it, deploying a value to an app of the other kind is refused so an app is never converted by accident." responses: "200": description: app updated @@ -12391,6 +12395,9 @@ paths: skip_draft_deletion: type: boolean description: "When true (set by the CLI / git sync), deploying this app does not delete an existing user draft at the same path." + allow_kind_change: + type: boolean + description: "When true, this deploy may switch the app between low-code and raw. Without it, deploying a value to an app of the other kind is refused so an app is never converted by accident." js: type: string css: diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 5712245b10..98c1f1ba2d 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -374,6 +374,11 @@ pub struct EditApp { /// Transient — never persisted. #[serde(default, skip_serializing_if = "Option::is_none")] pub skip_draft_deletion: Option, + /// Caller-intent flag: when true this deploy may switch the app between + /// low-code and raw (see `update_app_internal`). Transient — never + /// persisted. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub allow_kind_change: Option, } #[derive(Serialize, FromRow)] @@ -2693,6 +2698,47 @@ async fn update_app_internal<'a>( let mut tx = user_db.clone().begin(&authed).await?; + // `app_version.raw_app` is set by whichever endpoint writes the version, so a + // value deployed through the wrong one converts the app and strands its bundle. + // `FOR UPDATE` holds the app row until this transaction appends its own version, + // so a concurrent deploy of the other kind can't land between check and append. + if ns.value.is_some() && !ns.allow_kind_change.unwrap_or(false) { + let deployed_version = sqlx::query_scalar!( + "SELECT versions[array_upper(versions, 1)] FROM app + WHERE path = $1 AND workspace_id = $2 FOR UPDATE", + path, + w_id + ) + .fetch_optional(&mut *tx) + .await? + .flatten(); + let deployed_raw_app = match deployed_version { + // A separate statement: it needs the head the lock above pinned, not + // the snapshot the locking statement started from. + Some(version) => { + sqlx::query_scalar!("SELECT raw_app FROM app_version WHERE id = $1", version) + .fetch_optional(&mut *tx) + .await? + } + None => None, + }; + if deployed_raw_app.is_some_and(|deployed| deployed != raw_app) { + // Name the folder suffix too: a sync push picks the endpoint from the + // repo layout, so its operator has no endpoint to swap, only a folder. + let (kind, endpoint, folder) = if raw_app { + ("a low-code app", "/apps/update", ".app") + } else { + ("a raw app", "/apps/update_raw", ".raw_app") + }; + return Err(Error::BadRequest(format!( + "App {path} is {kind}: deploying a value to it through the other kind's endpoint \ + would convert it and strand its bundle. Deploy it through {endpoint} instead \ + (from a synced repo, from a `{folder}` folder), or set allow_kind_change to \ + convert it on purpose." + ))); + } + } + let mut preserved_on_behalf_of: Option = None; let npath = if ns.policy.is_some() || ns.path.is_some() diff --git a/backend/windmill-api/src/mcp/auto_generated_endpoints.rs b/backend/windmill-api/src/mcp/auto_generated_endpoints.rs index 762140c981..f4f39c2412 100644 --- a/backend/windmill-api/src/mcp/auto_generated_endpoints.rs +++ b/backend/windmill-api/src/mcp/auto_generated_endpoints.rs @@ -1113,7 +1113,7 @@ Creates a new version of an existing script when called with the same path and t EndpointTool { name: Cow::Borrowed("updateApp"), description: Cow::Borrowed("update app"), - instructions: Cow::Borrowed(""), + instructions: Cow::Borrowed("Low-code apps only. An app whose `raw_app` field (from getAppByPath) is true is a raw (full-code) app: its value holds source files that must be compiled to a js/css bundle, which this tool cannot upload, so updating one here is refused. Edit a raw app in its editor at /apps_raw/edit/ instead."), path: Cow::Borrowed("/w/{workspace}/apps/update/{path}"), method: Cow::Borrowed("POST"), path_params_schema: Some(serde_json::json!({ @@ -1521,6 +1521,10 @@ Creates a new version of an existing script when called with the same path and t "type": "boolean", "description": "is the job skipped" }, + "resolved": { + "type": "boolean", + "description": "filter on whether a failure has been marked as handled. true keeps only resolved failures, false hides them" + }, "is_flow_step": { "type": "boolean", "description": "is the job a flow step" diff --git a/frontend/src/lib/components/apps/editor/AppDeploymentHistory.svelte b/frontend/src/lib/components/apps/editor/AppDeploymentHistory.svelte index 6e73f97c19..7a508aac71 100644 --- a/frontend/src/lib/components/apps/editor/AppDeploymentHistory.svelte +++ b/frontend/src/lib/components/apps/editor/AppDeploymentHistory.svelte @@ -3,6 +3,7 @@ import { AppService } from '$lib/gen' import { workspaceStore } from '$lib/stores' import { sendUserToast } from '$lib/toast' + import { deployRawAppValue } from '$lib/rawAppDeploy' import DeploymentHistory from './DeploymentHistory.svelte' interface Props { @@ -17,13 +18,29 @@ } async function updateApp(app: any) { - await AppService.updateApp({ - workspace: $workspaceStore!, - path: app.path, - requestBody: { - ...app - } - }) + if (app.raw_app) { + // A raw version restores through the raw endpoint, which rebuilds its + // bundle. `allowKindChange` so the last version from before a conversion + // restores the app as raw — the way to undo one. The low-code direction + // stays refused: it has no bundle to write. + await deployRawAppValue({ + workspace: $workspaceStore!, + path: app.path, + value: app.value, + summary: app.summary, + policy: app.policy, + customPath: app.custom_path, + allowKindChange: true + }) + } else { + await AppService.updateApp({ + workspace: $workspaceStore!, + path: app.path, + requestBody: { + ...app + } + }) + } historyBrowserDrawerOpen = false } @@ -32,9 +49,13 @@ (historyBrowserDrawerOpen = false)}> { - sendUserToast('App restored from previous deployment') - updateApp(e.detail) + on:restore={async (e) => { + try { + await updateApp(e.detail) + sendUserToast('App restored from previous deployment') + } catch (err: any) { + sendUserToast(`Could not restore app: ${err.body ?? err.message}`, true) + } }} {appPath} on:close={() => { diff --git a/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte b/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte index 1e44b147f3..e60a149cfa 100644 --- a/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte +++ b/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte @@ -11,6 +11,7 @@ import { userStore, workspaceStore } from '$lib/stores' import { createEventDispatcher } from 'svelte' import { Globe, Loader2, Save } from 'lucide-svelte' + import { deployRawAppValue } from '$lib/rawAppDeploy' let jsonViewerDrawer: Drawer | undefined = $state() @@ -50,7 +51,17 @@ loading = false } + // The button drops a rejection, so failures — invalid JSON, a raw app whose + // sources don't bundle — have to surface here or Deploy looks like a no-op. export async function saveApp() { + try { + await deploy() + } catch (err: any) { + sendUserToast(`Could not save app: ${err.body ?? err.message}`, true) + } + } + + async function deploy() { const parsed = JSON.parse(code) if (isDraftOnly) { // No deployed row — `updateApp` would 404. Route through the syncer @@ -65,6 +76,19 @@ }) dispatch('change') sendUserToast('Draft saved') + } else if (isRawApp) { + // A raw app's value holds its sources, so deploying it means re-bundling. + await deployRawAppValue({ + workspace: $workspaceStore!, + path, + value: parsed, + summary: app?.summary, + policy: app?.policy, + customPath: app?.custom_path + }) + dispatch('change') + UserDraft.remove('raw_app', path) + sendUserToast('App deployed') } else { await AppService.updateApp({ workspace: $workspaceStore!, diff --git a/frontend/src/lib/mcpEndpointTools.ts b/frontend/src/lib/mcpEndpointTools.ts index f67c0415f0..103969952f 100644 --- a/frontend/src/lib/mcpEndpointTools.ts +++ b/frontend/src/lib/mcpEndpointTools.ts @@ -1121,7 +1121,7 @@ export const mcpEndpointTools: EndpointTool[] = [ { name: "updateApp", description: "update app", - instructions: "", + instructions: "Low-code apps only. An app whose `raw_app` field (from getAppByPath) is true is a raw (full-code) app: its value holds source files that must be compiled to a js/css bundle, which this tool cannot upload, so updating one here is refused. Edit a raw app in its editor at /apps_raw/edit/ instead.", path: "/w/{workspace}/apps/update/{path}", method: "POST", pathParamsSchema: { @@ -1529,6 +1529,10 @@ export const mcpEndpointTools: EndpointTool[] = [ "type": "boolean", "description": "is the job skipped" }, + "resolved": { + "type": "boolean", + "description": "filter on whether a failure has been marked as handled. true keeps only resolved failures, false hides them" + }, "is_flow_step": { "type": "boolean", "description": "is the job a flow step" diff --git a/frontend/src/lib/rawAppDeploy.ts b/frontend/src/lib/rawAppDeploy.ts index b13951c5f8..2a9bea2973 100644 --- a/frontend/src/lib/rawAppDeploy.ts +++ b/frontend/src/lib/rawAppDeploy.ts @@ -1,7 +1,8 @@ /** - * Deploy a raw app (code-based app) from its server-side draft. Raw apps can't - * be deployed through the normal AppService.updateApp/createApp path: their - * source `files` must be bundled to js/css and saved via the raw-app endpoints. + * Deploy a raw app (code-based app), from its server-side draft or from an + * explicit value. Raw apps can't be deployed through the normal + * AppService.updateApp/createApp path: their source `files` must be bundled to + * js/css and saved via the raw-app endpoints. * * This mirrors how the global AI chat deploys raw apps * (`copilot/chat/global/core.ts` → deployDraft, case 'app'): read the item with @@ -17,7 +18,73 @@ import { bundleRawAppDraft } from '$lib/components/copilot/chat/global/rawAppBun import type { AppDraftValue } from '$lib/components/copilot/chat/global/workspaceItems' import { updateRawAppPolicy } from '$lib/components/raw_apps/rawAppPolicy' import { DEFAULT_DATA as DEFAULT_RAW_APP_DATA } from '$lib/components/raw_apps/dataTableRefUtils' -import { appSourceToDraftValue } from '$lib/components/raw_apps/rawAppDraftValue' +import { + appSourceToDraftValue, + normalizeRawAppData +} from '$lib/components/raw_apps/rawAppDraftValue' +import { stateSnapshot } from '$lib/svelte5Utils.svelte' + +/** + * Deploy an explicit raw-app value — one the user edited as JSON, or one + * restored from a previous version — onto a deployed app. + */ +export async function deployRawAppValue({ + workspace, + path, + value, + summary, + policy: currentPolicy, + customPath, + deploymentMessage, + allowKindChange +}: { + workspace: string + path: string + value: any + summary?: string + policy?: Policy + customPath?: string | null + deploymentMessage?: string + /** Let this deploy turn a low-code app into a raw one. Off for every ordinary + * deploy — see the backend's `allow_kind_change`. */ + allowKindChange?: boolean +}): Promise { + // The value often comes straight out of a `$state` field, and the bundler + // runs in an iframe: postMessage refuses to clone a state proxy. + const plainValue = (stateSnapshot(value) ?? {}) as Record + const files = (plainValue.files ?? {}) as Record + const runnables = plainValue.runnables ?? {} + // The value carries the runnables, so the policy's triggerables have to be + // recomputed from it or the deployed app can't call what it now contains. + const policy = (await updateRawAppPolicy(runnables, currentPolicy)) as Policy + if (!policy.execution_mode) { + policy.execution_mode = 'publisher' + } + + const bundle = await bundleRawAppDraft({ workspace, files }) + + const isAdmin = !!(get(userStore)?.is_admin || get(userStore)?.is_super_admin) + await AppService.updateAppRaw({ + workspace, + path, + formData: { + app: { + // Through `normalizeRawAppData`, like every other deploy path: an old + // version can still carry the pre-`data` datatable shapes. + value: { files, runnables, data: normalizeRawAppData(plainValue) }, + summary: summary ?? '', + policy, + deployment_message: deploymentMessage, + // custom_path changes require admin (see deployRawAppDraft). + custom_path: isAdmin ? (customPath ?? '') : undefined, + preserve_on_behalf_of: policy.on_behalf_of ? true : undefined, + allow_kind_change: allowKindChange || undefined + }, + js: bundle.js, + css: bundle.css + } + }) +} /** * Promote a raw app's draft to deployed. Throws on failure (caller wraps into a