diff --git a/backend/.sqlx/query-221ebaace16df131a8a75de233869bcce087fc5630dc74c62831847959f2f345.json b/backend/.sqlx/query-221ebaace16df131a8a75de233869bcce087fc5630dc74c62831847959f2f345.json deleted file mode 100644 index 69b65f1afd..0000000000 --- a/backend/.sqlx/query-221ebaace16df131a8a75de233869bcce087fc5630dc74c62831847959f2f345.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT app_version.raw_app FROM app\n JOIN app_version ON app_version.id = app.versions[array_upper(app.versions, 1)]\n WHERE app.path = $1 AND app.workspace_id = $2", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "raw_app", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - false - ] - }, - "hash": "221ebaace16df131a8a75de233869bcce087fc5630dc74c62831847959f2f345" -} 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 990c1a256c..cf04e377d2 100644 --- a/backend/windmill-api-integration-tests/tests/apps.rs +++ b/backend/windmill-api-integration-tests/tests/apps.rs @@ -379,6 +379,13 @@ async fn test_raw_app_kind_is_not_flipped_by_update(db: Pool) -> anyho .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": [] } })) @@ -390,6 +397,8 @@ async fn test_raw_app_kind_is_not_flipped_by_update(db: Pool) -> anyho 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)) diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 8868c17e81..7c458a20b9 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -12259,6 +12259,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 diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 5f8621b53f..9bb227748c 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -366,9 +366,8 @@ pub struct EditApp { #[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. Only a deliberate conversion sets it (restoring a - /// version from the app's other-kind history); every other write is - /// refused rather than converted. Transient — never persisted. + /// low-code and raw (see `update_app_internal`). Transient — never + /// persisted. #[serde(default, skip_serializing_if = "Option::is_none")] pub allow_kind_change: Option, } @@ -2425,32 +2424,43 @@ async fn update_app_internal<'a>( let mut tx = user_db.clone().begin(&authed).await?; - // `app_version.raw_app` is whatever the endpoint that wrote the version says - // it is, so deploying a value through the wrong one converts the app in - // place: a raw app updated via /apps/update becomes a low-code app whose - // value no editor can render, and its js/css bundle (keyed on the previous - // version id) is orphaned. Refuse instead of letting the write land — the - // caller reached for the wrong endpoint, unless it says otherwise. + // `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_raw_app = sqlx::query_scalar!( - "SELECT app_version.raw_app FROM app - JOIN app_version ON app_version.id = app.versions[array_upper(app.versions, 1)] - WHERE app.path = $1 AND app.workspace_id = $2", + 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?; + .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) { - let (kind, endpoint) = if raw_app { - ("a low-code app", "/apps/update") + // 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") + ("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. Use {endpoint} instead, or set \ - allow_kind_change to convert it on purpose." + 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." ))); } } 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 1a821b02a5..6e8a997238 100644 --- a/frontend/src/lib/components/apps/editor/AppDeploymentHistory.svelte +++ b/frontend/src/lib/components/apps/editor/AppDeploymentHistory.svelte @@ -17,13 +17,13 @@ historyBrowserDrawerOpen = true } - // Picking a version is an explicit choice of what the app should become, so a - // version from before the app changed kind is allowed to convert it back — - // that is the only supported way to undo an accidental conversion. async function updateApp(app: any) { if (app.raw_app) { - // Restoring a raw app version means re-bundling its sources: `updateApp` - // would write a low-code version and leave the bundle behind. + // A raw version restores through the raw endpoint, which rebuilds the + // bundle from its sources. `allowKindChange` so picking the last version + // from before an app was converted to low-code restores it as raw — the + // supported way to undo such a conversion. The low-code direction stays + // refused: it has no bundle to write, so it would only break the app. await deployRawAppValue({ workspace: $workspaceStore!, path: app.path, @@ -38,8 +38,7 @@ workspace: $workspaceStore!, path: app.path, requestBody: { - ...app, - allow_kind_change: true + ...app } }) } diff --git a/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte b/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte index 2e1fddaad2..e60a149cfa 100644 --- a/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte +++ b/frontend/src/lib/components/apps/editor/AppJsonEditor.svelte @@ -51,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 @@ -67,9 +77,7 @@ dispatch('change') sendUserToast('Draft saved') } else if (isRawApp) { - // A raw app's value holds its sources, not a renderable grid: it has to be - // re-bundled and written through the raw endpoint. `updateApp` would flag - // the new version low-code and leave the bundle on the old one. + // A raw app's value holds its sources, so deploying it means re-bundling. await deployRawAppValue({ workspace: $workspaceStore!, path, 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 bdf9e37e5d..2a9bea2973 100644 --- a/frontend/src/lib/rawAppDeploy.ts +++ b/frontend/src/lib/rawAppDeploy.ts @@ -18,15 +18,15 @@ 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 an already deployed raw app. Going - * through AppService.updateApp instead would write a version flagged low-code - * and leave the bundle behind on the old version, so the app stops rendering; - * the backend rejects that mismatch. + * restored from a previous version — onto a deployed app. */ export async function deployRawAppValue({ workspace, @@ -45,15 +45,15 @@ export async function deployRawAppValue({ policy?: Policy customPath?: string | null deploymentMessage?: string - /** Let this deploy turn a low-code app into a raw one (restoring a version - * from before an accidental conversion). Off for every ordinary deploy. */ + /** 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 any - const files = (plainValue?.files ?? {}) as Record - const runnables = plainValue?.runnables ?? {} + 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 @@ -69,7 +69,9 @@ export async function deployRawAppValue({ path, formData: { app: { - value: { files, runnables, data: plainValue?.data ?? { ...DEFAULT_RAW_APP_DATA } }, + // 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,