fix: stop app updates from silently converting an app between raw and low-code (#10495)

* fix: stop app updates from silently converting an app between raw and low-code

* fix: lock the app row for the kind guard and route MCP away from raw apps

* style: condense the restore kind-change comment
This commit is contained in:
Ruben Fiszel
2026-08-04 08:52:20 +00:00
committed by GitHub
parent 546b8d0769
commit d9dd036edc
10 changed files with 321 additions and 16 deletions
@@ -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"
}
+1
View File
@@ -515,6 +515,7 @@ def main():
preserve_on_behalf_of: None,
labels: None,
skip_draft_deletion: None,
allow_kind_change: None,
})
.send()
.await
@@ -335,3 +335,111 @@ async fn test_public_app_by_custom_path(db: Pool<Postgres>) -> 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<Postgres>) -> 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::<serde_json::Value>().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::<serde_json::Value>().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::<serde_json::Value>().await?["raw_app"], false);
Ok(())
}
+7
View File
@@ -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/<path> 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:
+46
View File
@@ -374,6 +374,11 @@ pub struct EditApp {
/// Transient — never persisted.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub skip_draft_deletion: Option<bool>,
/// 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<bool>,
}
#[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<String> = None;
let npath = if ns.policy.is_some()
|| ns.path.is_some()
@@ -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/<path> 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"
@@ -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 @@
<Drawer bind:open={historyBrowserDrawerOpen} size="1200px">
<DrawerContent title="Deployment History" on:close={() => (historyBrowserDrawerOpen = false)}>
<DeploymentHistory
on:restore={(e) => {
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={() => {
@@ -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!,
+5 -1
View File
@@ -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/<path> 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"
+71 -4
View File
@@ -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<void> {
// 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<string, any>
const files = (plainValue.files ?? {}) as Record<string, string>
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