mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-11 16:09:39 +00:00
fix(ai-chat): test_run_flow could test a different flow than the one asked (#11066)
* fix(ai-chat): test_run_flow could test a different flow than the one asked Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TmQDoFXVPwYyEN8PfV2oL7 * fix(ai-chat): prefer the flow editor stored at the path over one renamed to it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TmQDoFXVPwYyEN8PfV2oL7 * test(ai-chat): default the flow helpers factory and trim duplicated setup Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TmQDoFXVPwYyEN8PfV2oL7 * refactor(ai-chat): resolve the flow editor to run by its storage path Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TmQDoFXVPwYyEN8PfV2oL7 * refactor(ai-chat): move the editor storage path context out of sessions Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TmQDoFXVPwYyEN8PfV2oL7 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
569adb85c1
commit
fa73539839
@@ -110,6 +110,7 @@
|
||||
import { isCloudHosted } from '$lib/cloud'
|
||||
import { UserDraft } from '$lib/userDraft.svelte'
|
||||
import { setOpenInSessionHandoff } from './sessions/openInSessionContext'
|
||||
import { getEditorStoragePath, setEditorStoragePath } from './editorStoragePathContext'
|
||||
|
||||
let {
|
||||
initialPath = $bindable(''),
|
||||
@@ -819,6 +820,12 @@
|
||||
return entries
|
||||
}
|
||||
|
||||
// The storage path this editor is bound to, narrowing whatever an outer mount
|
||||
// published (a session tab): the full-page editor holds its own, a drawer mount
|
||||
// holds none.
|
||||
const outerStoragePath = getEditorStoragePath()
|
||||
setEditorStoragePath(() => liveEditorDraftStoragePath ?? outerStoragePath?.())
|
||||
|
||||
// "Open in AI session" target: the URL draft path the editor loads/saves by
|
||||
// (which for a new flow differs from the live-edited friendly `$pathStore`),
|
||||
// falling back to `$pathStore` in drawer mounts that carry no storage path.
|
||||
|
||||
@@ -705,7 +705,13 @@ export class AIChatManager {
|
||||
>(undefined)
|
||||
scriptEditorShowDiffMode = $state<(() => void) | undefined>(undefined)
|
||||
scriptEditorGetLintErrors = $state<(() => ScriptLintResult) | undefined>(undefined)
|
||||
/** The editor a FLOW-mode chat belongs to: the page owning the chat names itself here, and a
|
||||
* nested editor (a subflow drawer) takes it over while it is open. Unset in a session chat,
|
||||
* which keeps every open editor tab mounted and could only name an arbitrary one — a session
|
||||
* resolves an editor by its storage path through `flowEditorFor`. */
|
||||
flowAiChatHelpers = $state<FlowAIChatHelpers | undefined>(undefined)
|
||||
/** Every mounted flow editor. */
|
||||
#flowEditors = new Set<FlowAIChatHelpers>()
|
||||
appAiChatHelpers = $state<AppAIChatHelpers | undefined>(undefined)
|
||||
/** Datatable creation policy: enabled flag, datatable name, and optional schema */
|
||||
datatableCreationPolicy = $state<{
|
||||
@@ -2415,7 +2421,8 @@ export class AIChatManager {
|
||||
openArtifact: this.openArtifact
|
||||
}
|
||||
: {}),
|
||||
testActiveFlow: async (args?: Record<string, any>) => this.flowAiChatHelpers?.testFlow(args),
|
||||
testActiveFlow: async (storagePath: string, args?: Record<string, any>) =>
|
||||
this.flowEditorFor(storagePath)?.testFlow(args),
|
||||
getModifiedItems: () => (this.modifiedItems ? [...this.modifiedItems] : undefined),
|
||||
attachedFiles: this.attachedFiles,
|
||||
getUserInstructions: () => getUserCustomPrompts()[AIMode.GLOBAL] ?? '',
|
||||
@@ -4664,7 +4671,11 @@ export class AIChatManager {
|
||||
}
|
||||
|
||||
setFlowHelpers = (flowHelpers: FlowAIChatHelpers) => {
|
||||
this.flowAiChatHelpers = flowHelpers
|
||||
this.#flowEditors.add(flowHelpers)
|
||||
// Only a chat that can reach FLOW mode names an editor (see `flowAiChatHelpers`).
|
||||
if (!this.isSessionChat) {
|
||||
this.flowAiChatHelpers = flowHelpers
|
||||
}
|
||||
untrack(() => {
|
||||
if (this.autoAcceptEditsActive) {
|
||||
this.acceptPendingFlowEdits(flowHelpers)
|
||||
@@ -4672,10 +4683,17 @@ export class AIChatManager {
|
||||
})
|
||||
|
||||
return () => {
|
||||
this.flowAiChatHelpers = undefined
|
||||
this.#flowEditors.delete(flowHelpers)
|
||||
if (!this.isSessionChat) {
|
||||
this.flowAiChatHelpers = undefined
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private flowEditorFor(storagePath: string): FlowAIChatHelpers | undefined {
|
||||
return [...this.#flowEditors].find((helpers) => helpers.getStoragePath() === storagePath)
|
||||
}
|
||||
|
||||
// Registered by the /pipeline editor while it is mounted. Rebuilds the global
|
||||
// tool set so the pipeline tools appear (and disappear on unregister). Pipeline
|
||||
// AI edits apply directly as drafts, so there is nothing to auto-accept.
|
||||
|
||||
@@ -191,15 +191,18 @@ beforeEach(() => {
|
||||
})
|
||||
|
||||
function createFlowHelpers({
|
||||
hasPendingChanges,
|
||||
acceptAllModuleActions,
|
||||
testFlow = vi.fn()
|
||||
hasPendingChanges = () => false,
|
||||
acceptAllModuleActions = vi.fn(),
|
||||
testFlow = vi.fn(),
|
||||
storagePath = 'u/admin/live_flow'
|
||||
}: {
|
||||
hasPendingChanges: () => boolean
|
||||
acceptAllModuleActions: () => void
|
||||
hasPendingChanges?: () => boolean
|
||||
acceptAllModuleActions?: () => void
|
||||
testFlow?: FlowAIChatHelpers['testFlow']
|
||||
}): FlowAIChatHelpers {
|
||||
storagePath?: string
|
||||
} = {}): FlowAIChatHelpers {
|
||||
return {
|
||||
getStoragePath: () => storagePath,
|
||||
getFlowAndSelectedId: vi.fn(),
|
||||
getRootModules: vi.fn(),
|
||||
inlineScriptSession: { get: vi.fn(), set: vi.fn(), clear: vi.fn() },
|
||||
@@ -853,19 +856,36 @@ describe('AIChatManager autonomy mode', () => {
|
||||
|
||||
manager.isSessionChat = true
|
||||
manager.sessionId = 'htc1xouxd96dcyo6ruqo39'
|
||||
manager.setFlowHelpers(
|
||||
createFlowHelpers({
|
||||
hasPendingChanges: () => false,
|
||||
acceptAllModuleActions: vi.fn(),
|
||||
testFlow
|
||||
})
|
||||
)
|
||||
manager.setFlowHelpers(createFlowHelpers({ testFlow }))
|
||||
|
||||
manager.changeMode(AIMode.GLOBAL)
|
||||
const jobId = await manager.helpers.testActiveFlow({ name: 'Ada' })
|
||||
const jobId = await manager.helpers.testActiveFlow('u/admin/live_flow', { name: 'Ada' })
|
||||
|
||||
expect(jobId).toBe('job-flow-preview')
|
||||
expect(testFlow).toHaveBeenCalledWith({ name: 'Ada' })
|
||||
// A session chat resolves an editor by its storage path, so it never names one.
|
||||
expect(manager.flowAiChatHelpers).toBeUndefined()
|
||||
})
|
||||
|
||||
// Session tabs keep every open flow editor mounted, so the last one to register is routinely
|
||||
// a different flow than the one being tested.
|
||||
it('tests the flow editor mounted on the storage path, not the last one registered', async () => {
|
||||
const manager = new AIChatManager()
|
||||
const testTarget = vi.fn(async () => 'job-target-flow')
|
||||
const testLast = vi.fn(async () => 'job-last-flow')
|
||||
|
||||
manager.setFlowHelpers(
|
||||
createFlowHelpers({ testFlow: testTarget, storagePath: 'u/admin/live_flow' })
|
||||
)
|
||||
manager.setFlowHelpers(
|
||||
createFlowHelpers({ testFlow: testLast, storagePath: 'u/admin/other_flow' })
|
||||
)
|
||||
|
||||
manager.changeMode(AIMode.GLOBAL)
|
||||
const jobId = await manager.helpers.testActiveFlow('u/admin/live_flow', { name: 'Ada' })
|
||||
|
||||
expect(jobId).toBe('job-target-flow')
|
||||
expect(testLast).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -14,6 +14,9 @@
|
||||
import type { ScriptLintResult } from '../shared'
|
||||
import { applyFlowJsonUpdate, updateRawScriptModuleContent } from './helperUtils'
|
||||
import { findModuleInFlow } from '$lib/components/flows/flowTree'
|
||||
import { getEditorStoragePath } from '$lib/components/editorStoragePathContext'
|
||||
|
||||
const editorStoragePath = getEditorStoragePath()
|
||||
|
||||
let {
|
||||
flowModuleSchemaMap,
|
||||
@@ -162,6 +165,8 @@
|
||||
selectionManager.selectId(id, { openPanel: true })
|
||||
},
|
||||
|
||||
getStoragePath: () => editorStoragePath?.(),
|
||||
|
||||
testFlow: async (args, conversationId) => {
|
||||
// Set preview args if provided
|
||||
if (args) {
|
||||
|
||||
@@ -136,6 +136,10 @@ export interface FlowAIChatHelpers {
|
||||
/** Run a test of the current flow using the UI's preview mechanism */
|
||||
testFlow: (args?: Record<string, any>, conversationId?: string) => Promise<string | undefined>
|
||||
|
||||
/** The path this editor's draft is stored under. Tells a caller which of several mounted
|
||||
* editors is the one an active-editor context names. */
|
||||
getStoragePath: () => string | undefined
|
||||
|
||||
/** Get lint errors from a specific module (focuses it first, waits for Monaco to analyze) */
|
||||
getLintErrors: (moduleId: string) => Promise<ScriptLintResult>
|
||||
}
|
||||
|
||||
@@ -4806,10 +4806,12 @@ describe('global AI tools', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('test_run_flow uses the live flow editor test hook when the active editor matches the path', async () => {
|
||||
// The editor is driven by the key its draft is stored under, which reads and edits of the
|
||||
// path resolve through too: a staged rename leaves that key where it was.
|
||||
it('test_run_flow drives the live flow editor by its storage path', async () => {
|
||||
seedBackendDraft(
|
||||
'flow',
|
||||
'',
|
||||
'u/admin/live_flow_storage',
|
||||
{
|
||||
path: 'u/admin/live_flow',
|
||||
summary: 'Live flow',
|
||||
@@ -4825,7 +4827,7 @@ describe('global AI tools', () => {
|
||||
UserDraft.setLiveEditorDraft({
|
||||
workspace: WORKSPACE,
|
||||
itemKind: 'flow',
|
||||
storagePath: '',
|
||||
storagePath: 'u/admin/live_flow_storage',
|
||||
effectivePath: 'u/admin/live_flow'
|
||||
})
|
||||
const testActiveFlow = vi.fn(async () => 'job-live-flow')
|
||||
@@ -4842,7 +4844,7 @@ describe('global AI tools', () => {
|
||||
)
|
||||
)
|
||||
|
||||
expect(testActiveFlow).toHaveBeenCalledWith({ name: 'Ada' })
|
||||
expect(testActiveFlow).toHaveBeenCalledWith('u/admin/live_flow_storage', { name: 'Ada' })
|
||||
expect(FlowService.getFlowByPath).not.toHaveBeenCalled()
|
||||
expect(JobService.runFlowPreview).not.toHaveBeenCalled()
|
||||
expect(result).toContain('Result (SUCCESS)')
|
||||
@@ -4851,7 +4853,7 @@ describe('global AI tools', () => {
|
||||
it('test_run_flow falls back to preview when the live flow editor test hook returns undefined', async () => {
|
||||
seedBackendDraft(
|
||||
'flow',
|
||||
'',
|
||||
'u/admin/live_flow_fallback',
|
||||
{
|
||||
path: 'u/admin/live_flow_fallback',
|
||||
summary: 'Live flow fallback',
|
||||
@@ -4867,7 +4869,7 @@ describe('global AI tools', () => {
|
||||
UserDraft.setLiveEditorDraft({
|
||||
workspace: WORKSPACE,
|
||||
itemKind: 'flow',
|
||||
storagePath: '',
|
||||
storagePath: 'u/admin/live_flow_fallback',
|
||||
effectivePath: 'u/admin/live_flow_fallback'
|
||||
})
|
||||
const testActiveFlow = vi.fn(async () => undefined)
|
||||
@@ -4884,7 +4886,7 @@ describe('global AI tools', () => {
|
||||
)
|
||||
)
|
||||
|
||||
expect(testActiveFlow).toHaveBeenCalledWith({ name: 'Ada' })
|
||||
expect(testActiveFlow).toHaveBeenCalledWith('u/admin/live_flow_fallback', { name: 'Ada' })
|
||||
expect(FlowService.getFlowByPath).not.toHaveBeenCalled()
|
||||
expect(JobService.runFlowPreview).toHaveBeenCalledWith({
|
||||
workspace: WORKSPACE,
|
||||
@@ -4896,6 +4898,52 @@ describe('global AI tools', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// The flow may be open in a session tab that isn't the one on screen: driving its editor
|
||||
// would paint the run into a tab the user is not looking at.
|
||||
it('test_run_flow previews rather than driving an editor the user is not looking at', async () => {
|
||||
seedBackendDraft(
|
||||
'flow',
|
||||
'u/admin/background_flow',
|
||||
{
|
||||
path: 'u/admin/background_flow',
|
||||
summary: 'Background flow',
|
||||
value: { modules: [{ id: 'background_step', value: { type: 'identity' } }] },
|
||||
schema: {},
|
||||
edited_by: '',
|
||||
edited_at: '',
|
||||
archived: false,
|
||||
extra_perms: {}
|
||||
},
|
||||
{ workspace: WORKSPACE }
|
||||
)
|
||||
UserDraft.setLiveEditorDraft({
|
||||
workspace: WORKSPACE,
|
||||
itemKind: 'flow',
|
||||
storagePath: 'u/admin/flow_on_screen',
|
||||
effectivePath: 'u/admin/flow_on_screen'
|
||||
})
|
||||
const testActiveFlow = vi.fn(async () => 'job-live-flow')
|
||||
|
||||
await withCompletedTestJob(() =>
|
||||
callGlobalTool(
|
||||
'test_run_flow',
|
||||
{ path: 'u/admin/background_flow', args: { name: 'Ada' } },
|
||||
toolCallbacks,
|
||||
{ testActiveFlow }
|
||||
)
|
||||
)
|
||||
|
||||
expect(testActiveFlow).not.toHaveBeenCalled()
|
||||
expect(JobService.runFlowPreview).toHaveBeenCalledWith({
|
||||
workspace: WORKSPACE,
|
||||
requestBody: {
|
||||
path: 'u/admin/background_flow',
|
||||
value: { modules: [{ id: 'background_step', value: { type: 'identity' } }] },
|
||||
args: { name: 'Ada' }
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('test_run_step previews rawscript steps from the draft flow', async () => {
|
||||
const content = 'export async function main(name: string) {\n\treturn name.toUpperCase()\n}'
|
||||
await callGlobalTool('write_flow', {
|
||||
|
||||
@@ -288,6 +288,8 @@ const ACTIVE_GLOBAL_EDITOR_DRAFTS: readonly {
|
||||
export type GlobalActiveEditorContext = {
|
||||
type: ActiveGlobalEditorType
|
||||
path: string
|
||||
/** The key the draft is stored under, which `path` leaves behind on a rename. */
|
||||
storagePath: string
|
||||
isLiveDraft: true
|
||||
}
|
||||
|
||||
@@ -4367,7 +4369,8 @@ type WriteDraftCtx = {
|
||||
export type SessionToolHelpers = { sessionId?: string }
|
||||
|
||||
export type GlobalToolHelpers = SessionToolHelpers & {
|
||||
testActiveFlow?: (args?: Record<string, any>) => Promise<string | undefined>
|
||||
/** Runs the flow editor mounted on `storagePath`, if one is. */
|
||||
testActiveFlow?: (storagePath: string, args?: Record<string, any>) => Promise<string | undefined>
|
||||
attachedFiles?: AttachedFilesStore
|
||||
// Read/write the user-level Global instructions. `setUserInstructions` persists the
|
||||
// value and rebuilds the system message so the change applies on the next chat-loop
|
||||
@@ -4396,15 +4399,21 @@ function operatingWorkspaceFromHelpers(helpers: unknown): string | undefined {
|
||||
return (helpers as GlobalToolHelpers | undefined)?.operatingWorkspace
|
||||
}
|
||||
|
||||
function activeFlowTestFromCtx(
|
||||
// Drive a live editor only for the flow on screen: several can be open at once (session tabs),
|
||||
// and a run painted into a background tab is a side effect the user never sees. Undefined
|
||||
// sends the caller to a preview run, which reports into the chat alone. The hook is bound to
|
||||
// that editor's storage path — the same key reads and edits of `path` resolve through, so a
|
||||
// staged rename cannot send the run to a different editor than the one being edited.
|
||||
function liveFlowTestHookFromCtx(
|
||||
ctx: { workspace: string; helpers?: unknown },
|
||||
path: string
|
||||
): GlobalToolHelpers['testActiveFlow'] | undefined {
|
||||
): ((args?: Record<string, any>) => Promise<string | undefined>) | undefined {
|
||||
const activeEditor = getActiveGlobalEditorContext(ctx.workspace)
|
||||
if (activeEditor?.type !== 'flow' || activeEditor.path !== path) {
|
||||
return undefined
|
||||
}
|
||||
return (ctx.helpers as GlobalToolHelpers | undefined)?.testActiveFlow
|
||||
const testActiveFlow = (ctx.helpers as GlobalToolHelpers | undefined)?.testActiveFlow
|
||||
return testActiveFlow && ((args) => testActiveFlow(activeEditor.storagePath, args))
|
||||
}
|
||||
|
||||
export type OpenPreviewHandler = (req: {
|
||||
@@ -5775,7 +5784,7 @@ async function testRunFlowByPath(
|
||||
): Promise<string> {
|
||||
const { workspace, toolId, toolCallbacks } = ctx
|
||||
const testArgs = normalizeTestRunArgs(args.args)
|
||||
const testActiveFlow = activeFlowTestFromCtx(ctx, args.path)
|
||||
const testActiveFlow = liveFlowTestHookFromCtx(ctx, args.path)
|
||||
|
||||
if (testActiveFlow) {
|
||||
return executeTestRun({
|
||||
@@ -8171,9 +8180,10 @@ export function getActiveGlobalEditorContext(
|
||||
): GlobalActiveEditorContext | undefined {
|
||||
for (const { itemKind, type } of ACTIVE_GLOBAL_EDITOR_DRAFTS) {
|
||||
const liveDraft = UserDraft.getLiveEditorDraft(itemKind, { workspace })
|
||||
const path = liveDraft?.effectivePath || liveDraft?.storagePath
|
||||
if (!liveDraft) continue
|
||||
const path = liveDraft.effectivePath || liveDraft.storagePath
|
||||
if (!path) continue
|
||||
return { type, path, isLiveDraft: true }
|
||||
return { type, path, storagePath: liveDraft.storagePath, isLiveDraft: true }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
import { getContext, setContext } from 'svelte'
|
||||
|
||||
// The path an editor's draft is stored under, the same key the live-editor draft
|
||||
// registers. Published by whichever ancestor owns it (the session tab, or the
|
||||
// full-page editor) because the parts that must say which item an editor is open
|
||||
// on sit below both, and the path they can see themselves is the renamed one.
|
||||
|
||||
const KEY = 'EditorStoragePath'
|
||||
|
||||
/** `undefined` where the editor has no stored draft to speak of (a drawer mount). */
|
||||
export type EditorStoragePath = () => string | undefined
|
||||
|
||||
export function setEditorStoragePath(storagePath: EditorStoragePath): void {
|
||||
setContext(KEY, storagePath)
|
||||
}
|
||||
|
||||
export function getEditorStoragePath(): EditorStoragePath | undefined {
|
||||
return getContext<EditorStoragePath | undefined>(KEY)
|
||||
}
|
||||
@@ -8,6 +8,7 @@
|
||||
import { makeFlowCodec, makeScriptCodec, makeRawAppCodec } from './sessionDraftCodecs'
|
||||
import { itemDisplayName } from './previewRouter'
|
||||
import SessionItemNotFound from './SessionItemNotFound.svelte'
|
||||
import { setEditorStoragePath } from '../editorStoragePathContext'
|
||||
|
||||
let {
|
||||
runtime,
|
||||
@@ -51,6 +52,10 @@
|
||||
// rely on its presence, not its identity.
|
||||
setContext('aiChatManager', runtime.manager)
|
||||
|
||||
// This tab's storage path, for the editor below: several tabs are mounted at
|
||||
// once and only this one knows which item each is open on.
|
||||
setEditorStoragePath(() => path)
|
||||
|
||||
// This tab's own editor cell (per (kind, path)); several tabs can be live at once.
|
||||
const cell = $derived(
|
||||
kind === 'flow'
|
||||
|
||||
Reference in New Issue
Block a user