From f99781ca5f77248206c951935cc44acfa5f072eb Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 24 Jun 2026 19:10:21 +0200 Subject: [PATCH] fix: persist on-behalf-of user across app deploy paths (#9773) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(frontend): persist on-behalf-of user when redeploying raw apps The raw-app deploy drawer reused AppEditorHeaderDeploy but never wired up the `preserveOnBehalfOf` bindable nor forwarded `preserve_on_behalf_of` in the createAppRaw/updateAppRaw request bodies. Without that flag, the shared backend handler (create_app_internal/update_app_internal) resets the policy's on_behalf_of to the deploying user on every deploy. So a publisher who set "App executed on behalf of " would silently lose it on the next deploy, unlike every other setting on the deploy page. Mirror the classic (low-code) app header: declare `preserveOnBehalfOf`, bind it to the deploy component, and send `preserve_on_behalf_of` on both create and update. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(frontend): preserve on-behalf-of in the draft-deploy path The draft-deploy path (deployDraft → AppService.createApp/updateApp for visual apps, deployRawAppDraft → createAppRaw/updateAppRaw for raw apps) carries the deployed app's policy forward but never sent preserve_on_behalf_of. So deploying a draft via the "Review & deploy drafts" UI silently reset the policy's on_behalf_of to the deploying user — the same backend reset behind the deploy-drawer bug, on a surface that has no on-behalf-of selector to re-set it. Send preserve_on_behalf_of whenever the carried policy has an on_behalf_of, for both app types. The backend still gates actual preservation on can_preserve_on_behalf_of, so a non-deployer cannot escalate. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(frontend): preserve on-behalf-of in the AI-chat raw-app deploy The global AI-chat deploy path (`deploy_workspace_item` → createAppRaw/ updateAppRaw in copilot/chat/global/core.ts) carried the recomputed policy forward but omitted preserve_on_behalf_of, so deploying a raw app via chat reset the policy's on_behalf_of to the deploying user — the last of the deploy surfaces with this gap. Send the flag when the policy has an on_behalf_of, mirroring the editor and draft-deploy paths; the backend still gates preservation on can_preserve_on_behalf_of. Add a regression test asserting the flag is forwarded when the deployed policy carries an on_behalf_of. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../copilot/chat/global/core.test.ts | 34 +++++++++++++++++++ .../components/copilot/chat/global/core.ts | 10 ++++-- .../raw_apps/RawAppEditorHeader.svelte | 10 +++++- frontend/src/lib/rawAppDeploy.ts | 10 ++++-- frontend/src/lib/utils_draft_deploy.ts | 9 +++-- 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index 3f91b12e33..efa64d6297 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -2135,6 +2135,40 @@ describe('global AI tools', () => { expect(getBackendDraft('raw_app', 'f/apps/report', { workspace: WORKSPACE })).toBeUndefined() }) + it('forwards preserve_on_behalf_of when the deployed policy carries an on_behalf_of', async () => { + // Without the flag the backend resets the policy's on_behalf_of to the + // deploying user; this chat path has no on-behalf-of selector, so it must + // preserve whatever the carried policy already holds. + vi.mocked(AppService.existsApp).mockResolvedValueOnce(true) + seedBackendDraft( + 'raw_app', + 'f/apps/obo', + { + summary: 'On-behalf app', + files: { '/index.tsx': 'console.log("obo")' }, + runnables: {}, + data: { tables: [] }, + policy: { + execution_mode: 'publisher', + on_behalf_of: 'u/alice', + on_behalf_of_email: 'alice@windmill.dev' + } + }, + { workspace: WORKSPACE } + ) + vi.mocked(AppService.getAppByPath).mockResolvedValueOnce({} as any) + + await callGlobalTool('deploy_workspace_item', { type: 'app', path: 'f/apps/obo' }) + + expect(AppService.updateAppRaw).toHaveBeenCalledWith( + expect.objectContaining({ + formData: expect.objectContaining({ + app: expect.objectContaining({ preserve_on_behalf_of: true }) + }) + }) + ) + }) + it('notifies the session preview (as raw_app) after deploying a raw app', async () => { const onDeployed = vi.fn() setDeployedInSessionHandler(onDeployed) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 5c1350f233..ffb2d7e0f6 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -3932,7 +3932,11 @@ async function deployDraft( value: rawAppValue, summary, policy, - deployment_message: deploymentMessage + deployment_message: deploymentMessage, + // Preserve the policy's on_behalf_of: this chat deploy path has no + // on-behalf-of selector, so without the flag the backend resets it to + // the deploying user (gated server-side by can_preserve_on_behalf_of). + preserve_on_behalf_of: policy.on_behalf_of ? true : undefined }, js: bundle.js, css: bundle.css @@ -3948,7 +3952,9 @@ async function deployDraft( summary, policy, deployment_message: deploymentMessage, - custom_path: appValue.custom_path + custom_path: appValue.custom_path, + // Preserve the policy's on_behalf_of (see update branch above). + preserve_on_behalf_of: policy.on_behalf_of ? true : undefined }, js: bundle.js, css: bundle.css diff --git a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte index c2f857d2e2..3963e1b16d 100644 --- a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte @@ -187,6 +187,11 @@ onOpenOthersDrafts }: Props = $props() + // Set by the on-behalf-of selector when the publisher picks a user other than + // themselves. Forwarded as `preserve_on_behalf_of` so the backend keeps the + // policy's on_behalf_of instead of resetting it to the deploying user. + let preserveOnBehalfOf = $state(false) + // The AutosaveIndicator watches these; in the sessions preview they're the // session's (workspace, path), else the full-page editor's own values. const indicatorWorkspace = $derived(autosaveWorkspace ?? $workspaceStore) @@ -315,7 +320,8 @@ summary: summary, policy, deployment_message: deploymentMsg, - custom_path: customPath + custom_path: customPath, + preserve_on_behalf_of: preserveOnBehalfOf || undefined }, js, css @@ -434,6 +440,7 @@ policy, path: npath, deployment_message: deploymentMsg, + preserve_on_behalf_of: preserveOnBehalfOf || undefined, // custom_path requires admin so to accept update without it, we need to send as undefined when non-admin (when undefined, it will be ignored) // it also means that customPath needs to be set to '' instead of undefined to unset it (when admin) custom_path: @@ -674,6 +681,7 @@ bind:customPathError bind:pathError bind:newEditedPath + bind:preserveOnBehalfOf /> diff --git a/frontend/src/lib/rawAppDeploy.ts b/frontend/src/lib/rawAppDeploy.ts index 3439cb6b6d..b13951c5f8 100644 --- a/frontend/src/lib/rawAppDeploy.ts +++ b/frontend/src/lib/rawAppDeploy.ts @@ -75,7 +75,11 @@ export async function deployRawAppDraft( summary, policy, deployment_message: deploymentMessage, - custom_path: isAdmin ? (value.custom_path ?? '') : undefined + custom_path: isAdmin ? (value.custom_path ?? '') : undefined, + // Preserve the policy's on_behalf_of: this draft-deploy path has no + // on-behalf-of selector, so without the flag the backend resets it to + // the deploying user (gated server-side by can_preserve_on_behalf_of). + preserve_on_behalf_of: policy.on_behalf_of ? true : undefined }, js: bundle.js, css: bundle.css @@ -91,7 +95,9 @@ export async function deployRawAppDraft( summary, policy, deployment_message: deploymentMessage, - custom_path: value.custom_path + custom_path: value.custom_path, + // Preserve the policy's on_behalf_of (see update branch above). + preserve_on_behalf_of: policy.on_behalf_of ? true : undefined }, js: bundle.js, css: bundle.css diff --git a/frontend/src/lib/utils_draft_deploy.ts b/frontend/src/lib/utils_draft_deploy.ts index 751f42e3cb..b07e7656b5 100644 --- a/frontend/src/lib/utils_draft_deploy.ts +++ b/frontend/src/lib/utils_draft_deploy.ts @@ -370,15 +370,20 @@ export async function deployDraft( // undefined so the backend preserves the existing route. The draft has no // custom_path, so admins fall back to the deployed route (`''` when none). const isAdmin = !!(get(userStore)?.is_admin || get(userStore)?.is_super_admin) + const policy = r.policy ?? { execution_mode: 'publisher' } const requestBody = { value: appValue, summary: draftSummary ?? r.summary ?? '', - policy: r.policy ?? { execution_mode: 'publisher' }, + policy, // Honor the draft's intended path; `draft_path` holds the user-typed path // for a never-deployed app parked at a `u/{user}/draft_{uuid}` storage key. path: draftPath ?? r.path ?? path, custom_path: isAdmin ? (r.custom_path ?? '') : undefined, - deployment_message: deploymentMessage + deployment_message: deploymentMessage, + // The draft carries no on-behalf-of selector — the policy comes straight + // from the deployed app. Preserve its on_behalf_of (the backend resets it + // to the deploying user without this flag, gated by can_preserve_on_behalf_of). + preserve_on_behalf_of: policy?.on_behalf_of ? true : undefined } // Same as flows: draft-only apps have no app row → create; // drafts on a deployed app update it.