From 1f1df1254ebc417a8344801b571c4be8e98636c4 Mon Sep 17 00:00:00 2001 From: Guilhem Date: Thu, 15 Oct 2026 17:21:28 +0200 Subject: [PATCH] fix: relock scripts and flow steps whose code changed on deploy (#11794) * fix: relock scripts deployed from a draft that carries the deployed lock Co-Authored-By: Claude Opus 5.5 (1M context) * chore: reword fetchDeployed lock comment Co-Authored-By: Claude Opus 5.5 (1M context) * fix: drop the lock of flow steps whose code the chat changed Co-Authored-By: Claude Opus 5.5 (1M context) * fix: drop stale flow step locks at deploy, from drafts and the flow editor Co-Authored-By: Claude Opus 5.5 (1M context) * fix: keep locks of copied flows and never block a deploy on the lock check Co-Authored-By: Claude Opus 5.5 (1M context) * fix: keep the lock of a renamed or duplicated flow step with unchanged code Co-Authored-By: Claude Opus 5.5 (1M context) --------- Co-authored-by: Claude Opus 5.5 (1M context) --- .../src/lib/components/FlowBuilder.svelte | 11 ++++ .../copilot/chat/global/core.test.ts | 33 +++++++++++ .../components/copilot/chat/global/core.ts | 10 +++- .../components/flows/staleStepLocks.test.ts | 25 ++++++++ .../lib/components/flows/staleStepLocks.ts | 33 +++++++++++ frontend/src/lib/utils_draft_deploy.test.ts | 58 +++++++++++++++++++ frontend/src/lib/utils_draft_deploy.ts | 16 ++++- 7 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 frontend/src/lib/components/flows/staleStepLocks.test.ts create mode 100644 frontend/src/lib/components/flows/staleStepLocks.ts diff --git a/frontend/src/lib/components/FlowBuilder.svelte b/frontend/src/lib/components/FlowBuilder.svelte index 17f984bac9..f533fcc1f0 100644 --- a/frontend/src/lib/components/FlowBuilder.svelte +++ b/frontend/src/lib/components/FlowBuilder.svelte @@ -49,6 +49,7 @@ import WorkspaceScriptSettingsDrawer from './flows/content/WorkspaceScriptSettingsDrawer.svelte' import FlowEditorDrawer from './flows/content/FlowEditorDrawer.svelte' import { dfs as dfsApply } from './flows/dfs' + import { dropStaleStepLocks } from './flows/staleStepLocks' import { claimLinkedToolsFetch, invalidateLinkedToolsFetches, @@ -627,6 +628,16 @@ } } + // The editor clears a step's lock when its code is edited, but a draft written elsewhere + // can hold a step whose code changed and whose lock did not. A failed lookup skips the + // check rather than blocking the deploy. + const liveFlowValue = isNewFlow + ? undefined + : await FlowService.getFlowByPath({ workspace: opWorkspace!, path: initialPath }) + .then((f) => f.value) + .catch(() => undefined) + dropStaleStepLocks(flow.value, liveFlowValue) + if (isNewFlow) { await FlowService.createFlow({ workspace: opWorkspace!, 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 ba71ced69b..a07b0102ca 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -4773,6 +4773,39 @@ describe('global AI tools', () => { ).resolves.toBe(code) }) + // The worker reuses a non-empty step lock, so an edited step keeping its lock deploys + // against the old dependencies. + it('set_flow_module_code drops the lock of the edited step only', async () => { + const step = (id: string) => ({ + id, + value: { + type: 'rawscript', + language: 'python3', + content: 'import requests', + lock: 'requests==2.0', + input_transforms: {} + } + }) + seedBackendDraft('flow', 'f/flows/locked', { + path: 'f/flows/locked', + summary: 'f', + description: '', + value: { modules: [step('edited'), step('untouched')] }, + schema: {} + }) + + await callGlobalTool('set_flow_module_code', { + path: 'f/flows/locked', + module_id: 'edited', + code: 'import yaml' + }) + + const [edited, untouched] = getBackendDraft('flow', 'f/flows/locked').value.modules + expect(edited.value.content).toBe('import yaml') + expect(edited.value).not.toHaveProperty('lock') + expect(untouched.value.lock).toBe('requests==2.0') + }) + it('warns about every empty rawscript body (top-level, nested, preprocessor, failure) and skips populated ones', async () => { const result = JSON.parse( await callGlobalTool('write_flow', { diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 3b6064dee6..6f762a4a22 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -52,6 +52,7 @@ import { withAgentDrafts } from '$lib/components/flows/linkedAgentDrafts' import { resolveLinkedAgentTools } from '$lib/components/flows/flowState' import { enabledToolNames, type AgentTool } from '$lib/components/flows/agentToolUtils' import { evalValue } from '$lib/components/flows/utils.svelte' +import { dropStaleStepLocks } from '$lib/components/flows/staleStepLocks' import { quickjsReady } from '$lib/utils/quickjsEval.svelte' import { updateRawAppPolicy } from '$lib/components/raw_apps/rawAppPolicy' import { @@ -5804,7 +5805,12 @@ type ScriptDraftArgs = { const SCRIPT_SPEC: WriteSpec = { probe: (workspace, path) => ScriptService.existsScriptByPath({ workspace, path }), fetchDeployed: async (workspace, path) => { - const existing = await ScriptService.getScriptByPath({ workspace, path }) + // The deployed lock describes the deployed content, not the draft's; the deploy recomputes it. + const { + lock: _lock, + lock_error_logs: _lockErrorLogs, + ...existing + } = await ScriptService.getScriptByPath({ workspace, path }) return { ...(existing as unknown as NewScript), parent_hash: existing.hash } }, buildDraft: async (base, args, path) => { @@ -5903,6 +5909,8 @@ const FLOW_SPEC: WriteSpec = { } } } + // Also checked at deploy; dropped here too because a test run of the draft reads the locks. + dropStaleStepLocks(value, base?.value) return base ? { ...structuredClone(base), diff --git a/frontend/src/lib/components/flows/staleStepLocks.test.ts b/frontend/src/lib/components/flows/staleStepLocks.test.ts new file mode 100644 index 0000000000..ae12860391 --- /dev/null +++ b/frontend/src/lib/components/flows/staleStepLocks.test.ts @@ -0,0 +1,25 @@ +import { describe, it, expect } from 'vitest' +import { dropStaleStepLocks } from './staleStepLocks' + +const step = (id: string, content: string, lock?: string) => + ({ id, value: { type: 'rawscript', language: 'python3', content, lock } }) as any + +describe('dropStaleStepLocks', () => { + // A rename or duplicate gives the step a new id; its code and lock still match. + it('keeps the lock of a renamed or duplicated step whose code is unchanged', () => { + const base = { modules: [step('a', 'import requests', 'requests==2.0')] } + const value = { + modules: [ + step('a', 'import requests', 'requests==2.0'), + step('a_copy', 'import requests', 'requests==2.0'), + step('a_edited_copy', 'import yaml', 'requests==2.0') + ] + } + dropStaleStepLocks(value as any, base as any) + expect(value.modules.map((m) => m.value.lock)).toEqual([ + 'requests==2.0', + 'requests==2.0', + undefined + ]) + }) +}) diff --git a/frontend/src/lib/components/flows/staleStepLocks.ts b/frontend/src/lib/components/flows/staleStepLocks.ts new file mode 100644 index 0000000000..2b9cacfb72 --- /dev/null +++ b/frontend/src/lib/components/flows/staleStepLocks.ts @@ -0,0 +1,33 @@ +import type { FlowModule, FlowValue } from '$lib/gen' +import { forEachFlowModule } from './dfs' + +/** + * Drops, in place, the lock of every inline step whose code differs from the same step in + * `base`. The worker reuses any non-empty step lock, so a step whose code changed but kept its + * lock runs against the old dependencies. Unchanged steps keep theirs, so a deploy does not + * re-resolve (and possibly upgrade) what nobody touched. A step id `base` lacks (a renamed or + * duplicated step) keeps its lock only when some `base` step has the same code. Without a base + * nothing is dropped: a copied or imported flow carries locks that match its code. + */ +export function dropStaleStepLocks(value: FlowValue, base: FlowValue | undefined): void { + if (!base) return + const allModules = (v: FlowValue) => + [...(v.modules ?? []), v.preprocessor_module, v.failure_module].filter( + (m): m is FlowModule => m != null + ) + const contentOf = (m: FlowModule) => (m.value as { content?: unknown }).content + const baseContentById = new Map() + const baseContents = new Set() + forEachFlowModule(allModules(base), (m) => { + baseContentById.set(m.id, contentOf(m)) + baseContents.add(contentOf(m)) + }) + forEachFlowModule(allModules(value), (m) => { + const step = m.value as { content?: unknown; lock?: unknown } + if (step.lock == undefined) return + const unchanged = baseContentById.has(m.id) + ? baseContentById.get(m.id) === step.content + : baseContents.has(step.content) + if (!unchanged) delete step.lock + }) +} diff --git a/frontend/src/lib/utils_draft_deploy.test.ts b/frontend/src/lib/utils_draft_deploy.test.ts index 5f503989c8..3164a05bfc 100644 --- a/frontend/src/lib/utils_draft_deploy.test.ts +++ b/frontend/src/lib/utils_draft_deploy.test.ts @@ -124,6 +124,64 @@ describe('deployDraft preserves on_behalf_of', () => { }) }) +// The backend skips the dependency job when `lock` is set, so a draft that inherited the +// deployed lock would deploy new content against the old lockfile. +describe('deployDraft: script lock', () => { + beforeEach(() => vi.clearAllMocks()) + + it('never sends the draft lock, so the backend relocks', async () => { + vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce({ + hash: 'v1', + draft: { + path: 'f/admin/fetch', + content: 'import requests', + lock: 'certifi==2024.1.1', + lock_error_logs: 'old error' + } + } as any) + + await deployDraft('script', 'f/admin/fetch', 'ws') + const body = vi.mocked(ScriptService.createScript).mock.calls[0][0].requestBody + expect(body).not.toHaveProperty('lock') + expect(body).not.toHaveProperty('lock_error_logs') + }) + + it('flow: drops the lock of a step whose code differs from the deployed one only', async () => { + const step = (id: string, content: string) => ({ + id, + value: { type: 'rawscript', language: 'python3', content, lock: 'requests==2.0' } + }) + vi.mocked(FlowService.getFlowByPath).mockResolvedValueOnce({ + value: { modules: [step('edited', 'import requests'), step('kept', 'import requests')] }, + draft: { + path: 'f/admin/flow', + value: { modules: [step('edited', 'import yaml'), step('kept', 'import requests')] } + } + } as any) + + await deployDraft('flow', 'f/admin/flow', 'ws') + const [edited, kept] = vi.mocked(FlowService.updateFlow).mock.calls[0][0].requestBody.value + .modules as any[] + expect(edited.value).not.toHaveProperty('lock') + expect(kept.value.lock).toBe('requests==2.0') + }) + + // A never-deployed flow is often a copy or an import, whose locks match its code. + it('flow: keeps every lock when nothing is deployed to compare against', async () => { + const step = { id: 'copied', value: { type: 'rawscript', content: 'x', lock: 'requests==2.0' } } + vi.mocked(FlowService.getFlowByPath).mockResolvedValueOnce({ + no_deployed: true, + value: { modules: [step] }, + draft: { path: 'f/admin/copy', value: { modules: [step] } } + } as any) + + await deployDraft('flow', 'f/admin/copy', 'ws', { draftOnly: true }) + const [copied] = vi.mocked(FlowService.createFlow).mock.calls[0][0].requestBody.value + .modules as any[] + expect(copied.value.lock).toBe('requests==2.0') + }) +}) + // The resource branch reads the item again when the deploy lands, and falls back to the deployed // row when the draft has gone. That row keeps its value under `value` and carries no `args` at // all, so reading it as a draft (`value: d.args ?? {}`) would replace a live resource with `{}`. diff --git a/frontend/src/lib/utils_draft_deploy.ts b/frontend/src/lib/utils_draft_deploy.ts index 44dec7f124..b17b17cbc7 100644 --- a/frontend/src/lib/utils_draft_deploy.ts +++ b/frontend/src/lib/utils_draft_deploy.ts @@ -46,6 +46,7 @@ import { setLocalDraftHint } from '$lib/localDraftHints.svelte' import { userStore } from '$lib/stores' import { deployTriggers, type Trigger } from '$lib/components/triggers/utils' import { saveScheduleFromCfg } from '$lib/components/flows/scheduleUtils' +import { dropStaleStepLocks } from '$lib/components/flows/staleStepLocks' import { saveHttpRouteFromCfg } from '$lib/components/triggers/http/utils' import { saveWebsocketTriggerFromCfg } from '$lib/components/triggers/websocket/utils' import { savePostgresTriggerFromCfg } from '$lib/components/triggers/postgres/utils' @@ -469,7 +470,15 @@ export async function deployDraft( const r = (await ScriptService.getScriptByPath({ workspace, path, getDraft: true })) as any const d = r.draft ?? r // Drop editor-only / server-managed keys; deploy as a real (non-draft) version. - const { draft_triggers: draftTriggers, draft_only: _o, ...rest } = d + // A draft seeded from the deployed script carries its `lock`, and the backend skips + // the dependency job whenever `lock` is set, so the new content would keep the old lock. + const { + draft_triggers: draftTriggers, + draft_only: _o, + lock: _lock, + lock_error_logs: _lockErrorLogs, + ...rest + } = d const scriptPath = d.path ?? path // Deploy at the draft's path so a rename in the draft is honored (same as // the editor: createScript at the new path with parent_hash links lineage). @@ -490,6 +499,9 @@ export async function deployDraft( } else if (kind === 'flow') { const r = (await FlowService.getFlowByPath({ workspace, path, getDraft: true })) as any const d = r.draft ?? r + const value = d.value ? structuredClone(d.value) : d.value + // With nothing deployed the server fills `r.value` from the draft itself. + if (value) dropStaleStepLocks(value, draftOnly || r.no_deployed ? undefined : r.value) const requestBody = { // Deploy at the draft's intended path: flow/app/raw-app drafts keep the // user-typed path in `draft_path` (a never-deployed item is parked at a @@ -498,7 +510,7 @@ export async function deployDraft( path: d.draft_path ?? d.path ?? path, summary: d.summary ?? '', description: d.description ?? '', - value: d.value, + value, schema: d.schema, tag: d.tag, dedicated_worker: d.dedicated_worker,