From 780ee8c09f11f6350282a2f16768b337092691ea Mon Sep 17 00:00:00 2001 From: centdix Date: Wed, 3 Jun 2026 21:56:46 +0200 Subject: [PATCH] fix: list app drafts and drop stale local drafts in ai chat Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/windmill-api/openapi.yaml | 4 + .../copilot/chat/global/core.test.ts | 110 +++++++++++++----- .../components/copilot/chat/global/core.ts | 35 ++++-- 3 files changed, 113 insertions(+), 36 deletions(-) diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 0c649f357f..7b35971ce1 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -26868,6 +26868,10 @@ components: items: type: string default: [] + has_draft: + type: boolean + draft_only: + type: boolean required: - id - workspace_id 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 b80a880d17..7f85b1d923 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -211,7 +211,8 @@ vi.mock('$lib/gen', async () => { existsResource: vi.fn(async () => false), getResource: vi.fn(async () => { throw new Error('getResource mock not configured') - }) + }), + listResource: vi.fn(async () => []) }), VariableService: wrapService(actual.VariableService, { existsVariable: vi.fn(async () => false), @@ -788,49 +789,32 @@ describe('global AI tools', () => { }) }) - it('applies path_prefix to local (live-editor) drafts before enforcing the result limit', async () => { - // list_workspace_items still surfaces live-editor localStorage drafts, so - // drive it through live editors rather than DB-only writes. + it('applies path_prefix to local drafts before enforcing the result limit', async () => { + // Group-B (resource/variable/…) drafts are localStorage-only and always + // listed; use them to exercise path_prefix filtering happening BEFORE the + // result limit (so a matching draft isn't dropped for a non-matching one). UserDraft.save( - 'script', + 'resource', 'f/other/outside', - { - path: 'f/other/outside', - summary: 'Outside draft', - description: '', - content: 'export async function main() { return "outside" }', - schema: {}, - is_template: false, - language: 'bun', - kind: 'script' - }, + { path: 'f/other/outside', description: '', args: {}, labels: [], wsSpecific: false }, { workspace: WORKSPACE } ) UserDraft.save( - 'script', + 'resource', 'f/matching/inside', - { - path: 'f/matching/inside', - summary: 'Inside draft', - description: '', - content: 'export async function main() { return "inside" }', - schema: {}, - is_template: false, - language: 'bun', - kind: 'script' - }, + { path: 'f/matching/inside', description: '', args: {}, labels: [], wsSpecific: false }, { workspace: WORKSPACE } ) const raw = await callGlobalTool('list_workspace_items', { - types: ['script'], + types: ['resource'], path_prefix: 'f/matching/', limit: 1 }) expect(JSON.parse(raw)).toEqual([ expect.objectContaining({ - type: 'script', + type: 'resource', path: 'f/matching/inside', isDraft: true }) @@ -943,6 +927,76 @@ describe('global AI tools', () => { expect(raw).not.toContain('Old deployed summary') }) + it('lists a draft-only app (includeDraftOnly) and flags it as a draft', async () => { + // `init_app` persists a `draft_only` app row; the backend excludes those + // unless `includeDraftOnly` is passed, so the app list must request it or a + // just-created app draft is invisible (scripts/flows already pass it). + vi.mocked(AppService.listApps).mockResolvedValueOnce([ + { path: 'f/apps/draft-only', summary: 'App draft', draft_only: true }, + { path: 'f/apps/deployed', summary: 'Deployed app', draft_only: false } + ] as any) + + const raw = await callGlobalTool('list_workspace_items', { types: ['app'] }) + const items = JSON.parse(raw) as Array<{ path: string; isDraft: boolean }> + + expect(items.find((i) => i.path === 'f/apps/draft-only')).toMatchObject({ isDraft: true }) + expect(items.find((i) => i.path === 'f/apps/deployed')).toMatchObject({ isDraft: false }) + expect(vi.mocked(AppService.listApps).mock.calls[0]?.[0]).toMatchObject({ + includeDraftOnly: true + }) + }) + + it('does not list a stale (non-live) Group-A localStorage draft', async () => { + // No live editor is registered, so `loadDraft` (read/deploy) ignores this + // localStorage entry — the list must not advertise a draft they won't use. + UserDraft.save( + 'script', + 'f/list/stale-local', + { + path: 'f/list/stale-local', + summary: 'Stale local', + description: '', + content: '', + schema: {}, + is_template: false, + language: 'bun', + kind: 'script' + }, + { workspace: WORKSPACE } + ) + + const raw = await callGlobalTool('list_workspace_items', { types: ['script'] }) + const items = JSON.parse(raw) as Array<{ path: string }> + expect(items.find((i) => i.path === 'f/list/stale-local')).toBeUndefined() + }) + + it('lists a live-editor Group-A localStorage draft', async () => { + UserDraft.setLiveEditorDraft({ + workspace: WORKSPACE, + itemKind: 'script', + storagePath: 'f/list/live-only' + }) + UserDraft.save( + 'script', + 'f/list/live-only', + { + path: 'f/list/live-only', + summary: 'Live only', + description: '', + content: '', + schema: {}, + is_template: false, + language: 'bun', + kind: 'script' + }, + { workspace: WORKSPACE } + ) + + const raw = await callGlobalTool('list_workspace_items', { types: ['script'] }) + const items = JSON.parse(raw) as Array<{ path: string; isDraft: boolean }> + expect(items.find((i) => i.path === 'f/list/live-only')).toMatchObject({ isDraft: true }) + }) + it('lists and edits the live script editor draft through its effective path', async () => { UserDraft.save( 'script', diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index b112a641cf..28b38c3ad0 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -920,7 +920,10 @@ function appToItem(app: ListableApp | AppWithLastVersion, includeValue: boolean) path: app.path, summary: app.summary, value: includeValue ? ((app as AppWithLastVersion).value as AppDraftValue) : undefined, - isDraft: false + // A backend `draft_only` app exists solely as a workspace draft (never + // deployed), so it is a draft. A deployed app (`draft_only` false) is not. + // Only `ListableApp` carries the flag; `AppWithLastVersion` never does. + isDraft: ('draft_only' in app ? app.draft_only : undefined) ?? false } } @@ -1227,7 +1230,7 @@ async function readScriptOrFlowItem( * (`value: undefined`) and `isDraft: true`. */ async function hydrateListedDbDraft( - type: 'script' | 'flow', + type: 'script' | 'flow' | 'app', item: WorkspaceItem, workspace: string ): Promise { @@ -1325,16 +1328,23 @@ async function listWorkspaceItems( } if (types.includes('app')) { + // `includeDraftOnly` so a never-deployed app that exists only as a workspace + // draft (created via `init_app`) is listed — without it the backend filters + // out `draft_only` rows and the AI can't see an app draft it just created. const apps = await AppService.listApps({ workspace, pathStart: pathPrefix, - perPage + perPage, + includeDraftOnly: true }) - // The generated `ListableApp` exposes no `has_draft`/`draft_only` flags, so - // we can't bound an app-draft hydration the way scripts/flows do. Accept a - // possibly-stale app summary here for now; client-side regeneration of those - // flags (or a per-app draft read) is the follow-up. - for (const app of apps) items.push(appToItem(app, false)) + for (const app of apps) { + const item = appToItem(app, false) + items.push( + app.has_draft || app.draft_only + ? await hydrateListedDbDraft('app', item, workspace) + : item + ) + } } return items @@ -1557,6 +1567,15 @@ export const globalTools: Tool<{}>[] = [ for (const draft of listGlobalDrafts(workspace)) { if (!types.includes(draft.type)) continue if (parsed.path_prefix && !draft.path.startsWith(parsed.path_prefix)) continue + // Group-A (script/flow/app) DB drafts already come from the backend list + // (`includeDraftOnly`). For those, localStorage is authoritative ONLY + // while a live editor is open — `read`/`deploy` ignore a stale closed- + // editor (or pre-upgrade) local draft via `loadDraft`, so the list must + // not advertise one either. Group B has no DB draft, so its localStorage + // entry is the source of truth and always counts. + const isGroupA = + draft.type === 'script' || draft.type === 'flow' || draft.type === 'app' + if (isGroupA && !draft.isLiveDraft) continue byKey.set(getWorkspaceItemKey(draft.type, draft.path, draft.triggerKind), { ...draft, value: undefined