mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix(frontend): strip raw-app post-deploy diff noise (raw_app/lock/data) (#9706)
* fix(frontend): strip raw-app post-deploy diff noise (raw_app/lock/data)
The raw-app editor's Diff drawer showed a spurious deployed-vs-current
diff immediately after deploy, even with no edits: `raw_app: true`, a
server-recomputed inline-script `lock`, and an empty `data` mismatch.
These come from comparing the deployed app row (from getAppByPath) against
the editor's current value, which differ on server-managed fields the
editor never carries, on inline-script locks (recomputed at deploy, cleared
on edit), and on `data` (the deployed row omits an empty `data` while the
editor always carries the default `{tables: []}`).
Add `stripRawAppDiffNoise` (strip server columns, null inline locks,
canonicalize data) and apply it symmetrically to both diff sides in the
editor header. For the session/compare draft diff, the draft is stored flat
(files/runnables/data top-level) while the deployed row nests under `value`,
so add `canonicalRawAppDiffValue` (= appSourceToDraftValue + stripRawAppDiffNoise)
and route both sides through it in getDraftDiffValues. Both diff surfaces now
share the same normalizer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(frontend): use canonicalized current value in deploy-drawer raw-app diff
The Deploy drawer's "Diff" action still built the current side inline from
raw editor state, bypassing stripRawAppDiffNoise — so inline-script `lock`
and data-shape noise could resurface via Deploy → Diff even though the
top-level Diff button was already fixed. Route it through `currentDiffValue`
(and strip the savedApp fallback) so both entry points behave identically.
Addresses Codex review finding on PR #9706.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -68,6 +68,7 @@
|
||||
// `runtime.syncPreviewWithDeployed`, which discards the fork draft + reloads
|
||||
// the preview to the deployed version.
|
||||
import { AIBtnClasses } from '../copilot/chat/AIButtonStyle'
|
||||
import { stripRawAppDiffNoise } from './utils'
|
||||
import type { RawAppData } from './dataTableRefUtils'
|
||||
import { isRuleActive } from '$lib/workspaceProtectionRules.svelte'
|
||||
import { buildForkEditUrl } from '$lib/utils/editInFork'
|
||||
@@ -369,15 +370,7 @@
|
||||
savedApp &&
|
||||
app &&
|
||||
orderedJsonStringify(deployedValue) ===
|
||||
orderedJsonStringify(
|
||||
replaceFalseWithUndefined({
|
||||
summary: summary,
|
||||
value: app,
|
||||
path: newEditedPath || savedApp.path,
|
||||
policy,
|
||||
custom_path: customPath
|
||||
})
|
||||
)
|
||||
orderedJsonStringify(replaceFalseWithUndefined(currentDiffValue))
|
||||
) {
|
||||
await updateApp(npath)
|
||||
} else {
|
||||
@@ -400,15 +393,9 @@
|
||||
|
||||
deployedBy = deployedApp.created_by
|
||||
|
||||
// Strip off extra information
|
||||
deployedValue = replaceFalseWithUndefined({
|
||||
...deployedApp,
|
||||
id: undefined,
|
||||
created_at: undefined,
|
||||
created_by: undefined,
|
||||
versions: undefined,
|
||||
extra_perms: undefined
|
||||
})
|
||||
// Normalize away post-deploy noise (see stripRawAppDiffNoise) so the
|
||||
// diff/comparison only reflects what the editor actually changed.
|
||||
deployedValue = replaceFalseWithUndefined(stripRawAppDiffNoise(deployedApp))
|
||||
}
|
||||
|
||||
async function openDiffDrawer() {
|
||||
@@ -422,14 +409,8 @@
|
||||
diffDrawer?.openDrawer()
|
||||
diffDrawer?.setDiff({
|
||||
mode: 'normal',
|
||||
deployed: deployedValue ?? savedApp,
|
||||
current: {
|
||||
summary: summary,
|
||||
value: app,
|
||||
path: newEditedPath || savedApp.path,
|
||||
policy,
|
||||
custom_path: customPath
|
||||
}
|
||||
deployed: deployedValue ?? stripRawAppDiffNoise(savedApp),
|
||||
current: currentDiffValue
|
||||
})
|
||||
}
|
||||
|
||||
@@ -594,6 +575,18 @@
|
||||
|
||||
let app = $derived(files ? { runnables: runnables, files, data } : undefined)
|
||||
|
||||
// Editor-side value for diffing/comparison against the deployed app, with the
|
||||
// same noise stripped as the deployed side (see stripRawAppDiffNoise).
|
||||
let currentDiffValue = $derived(
|
||||
stripRawAppDiffNoise({
|
||||
summary: summary,
|
||||
value: app,
|
||||
path: newEditedPath || savedApp?.path,
|
||||
policy,
|
||||
custom_path: customPath
|
||||
})
|
||||
)
|
||||
|
||||
$effect(() => {
|
||||
saveDrawerOpen && compareVersions()
|
||||
})
|
||||
@@ -605,13 +598,7 @@
|
||||
bind:open
|
||||
{diffDrawer}
|
||||
bind:deployedValue
|
||||
currentValue={{
|
||||
summary: summary,
|
||||
value: app,
|
||||
path: newEditedPath || savedApp?.path,
|
||||
policy,
|
||||
custom_path: customPath
|
||||
}}
|
||||
currentValue={currentDiffValue}
|
||||
/>
|
||||
|
||||
<Drawer bind:open={saveDrawerOpen} size="800px">
|
||||
@@ -632,14 +619,8 @@
|
||||
diffDrawer?.openDrawer()
|
||||
diffDrawer?.setDiff({
|
||||
mode: 'normal',
|
||||
deployed: deployedValue ?? savedApp,
|
||||
current: {
|
||||
summary: summary,
|
||||
value: app,
|
||||
path: newEditedPath || savedApp.path,
|
||||
policy,
|
||||
custom_path: customPath
|
||||
},
|
||||
deployed: deployedValue ?? stripRawAppDiffNoise(savedApp),
|
||||
current: currentDiffValue,
|
||||
button: {
|
||||
text: 'Looks good, deploy',
|
||||
onClick: () => {
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import {
|
||||
canonicalRawAppDiffValue,
|
||||
formatRuntimeLogsForChat,
|
||||
genWmillTs,
|
||||
normalizeRawAppRuntimeLogs,
|
||||
stripRawAppDiffNoise,
|
||||
type Runnable
|
||||
} from './utils'
|
||||
|
||||
@@ -61,3 +63,100 @@ describe('normalizeRawAppRuntimeLogs', () => {
|
||||
expect(formatRuntimeLogsForChat(entries)).toBe('[06:13:20.000] LOG: ready')
|
||||
})
|
||||
})
|
||||
|
||||
// A deployed raw-app row as returned by getAppByPath: nested `value`, plus the
|
||||
// server-managed columns and a recomputed inline-script lock.
|
||||
function deployedRow() {
|
||||
return {
|
||||
id: 42,
|
||||
raw_app: true,
|
||||
is_draft: false,
|
||||
created_at: '2024-01-01',
|
||||
created_by: 'admin',
|
||||
versions: [1, 2],
|
||||
extra_perms: { 'u/admin': true },
|
||||
summary: 'app',
|
||||
path: 'u/admin/app',
|
||||
policy: { execution_mode: 'publisher' },
|
||||
value: {
|
||||
files: { '/App.tsx': 'export default 1' },
|
||||
runnables: {
|
||||
a: { type: 'inline', inlineScript: { content: 'main()', language: 'bun', lock: 'deps\n' } }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('stripRawAppDiffNoise', () => {
|
||||
it('drops server-managed columns, nulls inline locks and canonicalizes data', () => {
|
||||
const cleaned = stripRawAppDiffNoise(deployedRow())
|
||||
|
||||
for (const key of [
|
||||
'raw_app',
|
||||
'id',
|
||||
'created_at',
|
||||
'created_by',
|
||||
'versions',
|
||||
'extra_perms',
|
||||
'is_draft'
|
||||
]) {
|
||||
expect(cleaned).not.toHaveProperty(key)
|
||||
}
|
||||
expect(cleaned.value.runnables.a.inlineScript.lock).toBeUndefined()
|
||||
// absent `data` is canonicalized to the default empty shape
|
||||
expect(cleaned.value.data).toEqual({ tables: [], datatable: undefined, schema: undefined })
|
||||
})
|
||||
|
||||
it('does not mutate the input (live editor state)', () => {
|
||||
const input = deployedRow()
|
||||
stripRawAppDiffNoise(input)
|
||||
expect(input.raw_app).toBe(true)
|
||||
expect(input.value.runnables.a.inlineScript.lock).toBe('deps\n')
|
||||
})
|
||||
|
||||
it('handles the flat editor/draft shape (files/runnables top-level)', () => {
|
||||
const flat = {
|
||||
summary: 'app',
|
||||
files: { '/App.tsx': 'x' },
|
||||
runnables: {
|
||||
a: { type: 'inline', inlineScript: { content: 'm()', language: 'bun', lock: 'l' } }
|
||||
}
|
||||
}
|
||||
const cleaned = stripRawAppDiffNoise(flat)
|
||||
expect(cleaned.runnables.a.inlineScript.lock).toBeUndefined()
|
||||
expect(cleaned.data).toEqual({ tables: [], datatable: undefined, schema: undefined })
|
||||
})
|
||||
})
|
||||
|
||||
describe('canonicalRawAppDiffValue', () => {
|
||||
it('collapses a nested deployed row and a flat draft to an identical value when content matches', () => {
|
||||
const deployed = deployedRow()
|
||||
// The flat draft shape a raw app autosaves: top-level files/runnables/data,
|
||||
// no server columns, lock cleared on edit.
|
||||
const draft = {
|
||||
summary: 'app',
|
||||
files: { '/App.tsx': 'export default 1' },
|
||||
runnables: { a: { type: 'inline', inlineScript: { content: 'main()', language: 'bun' } } },
|
||||
data: { tables: [] },
|
||||
policy: { execution_mode: 'publisher' }
|
||||
}
|
||||
|
||||
expect(canonicalRawAppDiffValue(deployed)).toEqual(canonicalRawAppDiffValue(draft))
|
||||
})
|
||||
|
||||
it('still surfaces a real change (summary edit)', () => {
|
||||
const deployed = deployedRow()
|
||||
const draft = {
|
||||
summary: 'app EDITED',
|
||||
files: { '/App.tsx': 'export default 1' },
|
||||
runnables: { a: { type: 'inline', inlineScript: { content: 'main()', language: 'bun' } } },
|
||||
data: { tables: [] }
|
||||
}
|
||||
|
||||
const a = canonicalRawAppDiffValue(deployed)
|
||||
const b = canonicalRawAppDiffValue(draft)
|
||||
expect(a).not.toEqual(b)
|
||||
expect(a.summary).toBe('app')
|
||||
expect(b.summary).toBe('app EDITED')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -3,6 +3,8 @@ import type { Schema } from '../../common'
|
||||
import { schemaToTsType } from '../../schema'
|
||||
import { isRunnableByName, isRunnableByPath, type RunnableWithFields } from '../apps/inputType'
|
||||
import type { InlineScript } from '../apps/sharedTypes'
|
||||
import { stateSnapshot } from '$lib/svelte5Utils.svelte'
|
||||
import { appSourceToDraftValue, normalizeRawAppData } from './rawAppDraftValue'
|
||||
|
||||
// export type RunnableWithFields = any
|
||||
|
||||
@@ -15,6 +17,66 @@ export type RawApp = {
|
||||
files: string[]
|
||||
}
|
||||
|
||||
// Server-managed columns the deployed app row (getAppByPath) carries but the
|
||||
// editor's current value never does — leaving them in renders as spurious diff.
|
||||
const RAW_APP_DEPLOYED_METADATA_KEYS = [
|
||||
'raw_app',
|
||||
'id',
|
||||
'created_at',
|
||||
'created_by',
|
||||
'versions',
|
||||
'extra_perms',
|
||||
'is_draft'
|
||||
] as const
|
||||
|
||||
/**
|
||||
* Normalize a raw-app value before a deployed-vs-current diff (or unsaved-change
|
||||
* comparison). Three sources of spurious post-deploy diff:
|
||||
* - the deployed row carries server-managed columns (`raw_app`, timestamps, …)
|
||||
* that the editor's current value lacks;
|
||||
* - inline-script `lock`s are recomputed server-side at every deploy and the
|
||||
* editor clears them on edit, so the editor value and the freshly deployed
|
||||
* one always diverge on `lock` even though the user changed nothing there;
|
||||
* - the deployed value omits an empty `data` while the editor always carries
|
||||
* the default `{ tables: [] }`, so even an untouched app reads as changed.
|
||||
* All three must be neutralized symmetrically on both sides. Returns a deep
|
||||
* clone; never mutates the input (the current side is live editor state).
|
||||
*/
|
||||
export function stripRawAppDiffNoise<T extends Record<string, any>>(value: T): T {
|
||||
const cloned = structuredClone(stateSnapshot(value)) as Record<string, any>
|
||||
for (const key of RAW_APP_DEPLOYED_METADATA_KEYS) {
|
||||
delete cloned[key]
|
||||
}
|
||||
// Runnables/data live under `.value` on a deployed row and on the editor's
|
||||
// diff value alike, but a flat draft shape carries them top-level.
|
||||
const source = cloned.value ?? cloned
|
||||
const runnables = source.runnables
|
||||
if (runnables && typeof runnables === 'object') {
|
||||
for (const k of Object.keys(runnables)) {
|
||||
const inlineScript = runnables[k]?.inlineScript
|
||||
if (inlineScript && inlineScript.lock != undefined) {
|
||||
inlineScript.lock = undefined
|
||||
}
|
||||
}
|
||||
}
|
||||
// Canonicalize `data` so an absent and a default-empty `data` compare equal.
|
||||
source.data = normalizeRawAppData(source)
|
||||
return cloned as T
|
||||
}
|
||||
|
||||
/**
|
||||
* Canonical raw-app value for diffing a *draft* against a *deployed* row. On top
|
||||
* of the noise stripped by stripRawAppDiffNoise, the two also differ in shape: a
|
||||
* deployed row nests its source under `value`, whereas a draft carries
|
||||
* `files`/`runnables`/`data` at the top level. `appSourceToDraftValue` collapses
|
||||
* both onto the same flat field set first. Use this for the session/compare
|
||||
* draft diff so it matches the editor's Diff button (which shares
|
||||
* stripRawAppDiffNoise).
|
||||
*/
|
||||
export function canonicalRawAppDiffValue(source: Record<string, any>) {
|
||||
return stripRawAppDiffNoise(appSourceToDraftValue(source))
|
||||
}
|
||||
|
||||
export type RawAppRuntimeLogLevel = 'log' | 'info' | 'warn' | 'error' | 'debug'
|
||||
export type RawAppRuntimeLogEntry = {
|
||||
level: RawAppRuntimeLogLevel
|
||||
|
||||
@@ -37,6 +37,7 @@ import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
import type { DeployResult } from '$lib/utils_workspace_deploy'
|
||||
import { TRIGGER_RUNTIME_IGNORE } from '$lib/utils_deployable'
|
||||
import { deployRawAppDraft } from '$lib/rawAppDeploy'
|
||||
import { canonicalRawAppDiffValue } from '$lib/components/raw_apps/utils'
|
||||
import { invalidateWorkspaceDrafts } from '$lib/workspaceDrafts.svelte'
|
||||
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
|
||||
import { userStore } from '$lib/stores'
|
||||
@@ -219,6 +220,16 @@ export async function getDraftDiffValues(
|
||||
getDraft: true,
|
||||
rawApp: kind === 'raw_app'
|
||||
})) as any
|
||||
if (kind === 'raw_app' || r.raw_app === true) {
|
||||
// Raw-app drafts are stored flat (files/runnables/data top-level) while the
|
||||
// deployed row nests them under `value`, and deployed inline scripts carry
|
||||
// server-recomputed locks. Canonicalize both onto the same shape with the
|
||||
// post-deploy noise stripped — the same module the editor's Diff button uses.
|
||||
return {
|
||||
deployed: draftOnly ? canonicalRawAppDiffValue({}) : canonicalRawAppDiffValue(r),
|
||||
draft: canonicalRawAppDiffValue(r.draft ?? r)
|
||||
}
|
||||
}
|
||||
const deployed = {
|
||||
summary: r.summary,
|
||||
value: r.value,
|
||||
|
||||
Reference in New Issue
Block a user