From b1d48fe858618a8b6165ee3e7a4e6856972ef9ec Mon Sep 17 00:00:00 2001 From: centdix Date: Mon, 8 Jun 2026 13:32:12 +0200 Subject: [PATCH] fix(drafts): surface draft-only apps and path-prefixed draft-only rows Co-Authored-By: Claude Opus 4.8 (1M context) --- ...c37404e9d6bb598842d3c7f846c2e50457f0.json} | 5 ++- ...e47c26034d500c35dad5442bdc14bf8d1aa8.json} | 5 ++- ...e7cc4515081955b8850bebc4ee817819e71a.json} | 5 ++- backend/windmill-api-flows/src/flows.rs | 8 ++-- backend/windmill-api-scripts/src/scripts.rs | 8 ++-- backend/windmill-api/src/apps.rs | 7 +++- .../chat/global/MIGRATION_DB_DRAFTS.md | 39 ++++++++++++++----- .../copilot/chat/global/core.test.ts | 23 ++++++++++- .../components/copilot/chat/global/core.ts | 3 +- 9 files changed, 78 insertions(+), 25 deletions(-) rename backend/.sqlx/{query-56741ef2a510917f6c460968117b26e3ef9171f478e7ab3bd68eb09663b393c4.json => query-697c8c8794a4ad6aece92810319dc37404e9d6bb598842d3c7f846c2e50457f0.json} (81%) rename backend/.sqlx/{query-e39a6474d84e1bc10a33351b6970ade7b6eb611beed13570aeda41a37efa927d.json => query-86a7ff7c5f178844007b6e66ca0fe47c26034d500c35dad5442bdc14bf8d1aa8.json} (84%) rename backend/.sqlx/{query-bf945c34c18a9fe5f0af8f54042df39178f5111119d50f2975dd1180502c0c52.json => query-f8e8761da49f33e9cf7d73586757e7cc4515081955b8850bebc4ee817819e71a.json} (81%) diff --git a/backend/.sqlx/query-56741ef2a510917f6c460968117b26e3ef9171f478e7ab3bd68eb09663b393c4.json b/backend/.sqlx/query-697c8c8794a4ad6aece92810319dc37404e9d6bb598842d3c7f846c2e50457f0.json similarity index 81% rename from backend/.sqlx/query-56741ef2a510917f6c460968117b26e3ef9171f478e7ab3bd68eb09663b393c4.json rename to backend/.sqlx/query-697c8c8794a4ad6aece92810319dc37404e9d6bb598842d3c7f846c2e50457f0.json index 37f6872cc9..5f32f36afc 100644 --- a/backend/.sqlx/query-56741ef2a510917f6c460968117b26e3ef9171f478e7ab3bd68eb09663b393c4.json +++ b/backend/.sqlx/query-697c8c8794a4ad6aece92810319dc37404e9d6bb598842d3c7f846c2e50457f0.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT path,\n value as \"value!: sqlx::types::Json>\",\n created_at\n FROM draft\n WHERE workspace_id = $1\n AND typ = 'script'\n AND email = $2\n AND NOT EXISTS (\n SELECT 1 FROM script s\n WHERE s.workspace_id = draft.workspace_id\n AND s.path = draft.path\n )", + "query": "SELECT path,\n value as \"value!: sqlx::types::Json>\",\n created_at\n FROM draft\n WHERE workspace_id = $1\n AND typ = 'script'\n AND email = $2\n AND ($3::text IS NULL OR path LIKE $3 || '%')\n AND NOT EXISTS (\n SELECT 1 FROM script s\n WHERE s.workspace_id = draft.workspace_id\n AND s.path = draft.path\n )", "describe": { "columns": [ { @@ -21,6 +21,7 @@ ], "parameters": { "Left": [ + "Text", "Text", "Text" ] @@ -31,5 +32,5 @@ false ] }, - "hash": "56741ef2a510917f6c460968117b26e3ef9171f478e7ab3bd68eb09663b393c4" + "hash": "697c8c8794a4ad6aece92810319dc37404e9d6bb598842d3c7f846c2e50457f0" } diff --git a/backend/.sqlx/query-e39a6474d84e1bc10a33351b6970ade7b6eb611beed13570aeda41a37efa927d.json b/backend/.sqlx/query-86a7ff7c5f178844007b6e66ca0fe47c26034d500c35dad5442bdc14bf8d1aa8.json similarity index 84% rename from backend/.sqlx/query-e39a6474d84e1bc10a33351b6970ade7b6eb611beed13570aeda41a37efa927d.json rename to backend/.sqlx/query-86a7ff7c5f178844007b6e66ca0fe47c26034d500c35dad5442bdc14bf8d1aa8.json index d4370c851f..587b74c687 100644 --- a/backend/.sqlx/query-e39a6474d84e1bc10a33351b6970ade7b6eb611beed13570aeda41a37efa927d.json +++ b/backend/.sqlx/query-86a7ff7c5f178844007b6e66ca0fe47c26034d500c35dad5442bdc14bf8d1aa8.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT path,\n value as \"value!: sqlx::types::Json>\",\n created_at,\n typ::text as \"typ!\"\n FROM draft\n WHERE workspace_id = $1\n AND typ IN ('app', 'raw_app')\n AND email = $2\n AND NOT EXISTS (\n SELECT 1 FROM app a\n WHERE a.workspace_id = draft.workspace_id\n AND a.path = draft.path\n )", + "query": "SELECT path,\n value as \"value!: sqlx::types::Json>\",\n created_at,\n typ::text as \"typ!\"\n FROM draft\n WHERE workspace_id = $1\n AND typ IN ('app', 'raw_app')\n AND email = $2\n AND ($3::text IS NULL OR path LIKE $3 || '%')\n AND NOT EXISTS (\n SELECT 1 FROM app a\n WHERE a.workspace_id = draft.workspace_id\n AND a.path = draft.path\n )", "describe": { "columns": [ { @@ -26,6 +26,7 @@ ], "parameters": { "Left": [ + "Text", "Text", "Text" ] @@ -37,5 +38,5 @@ null ] }, - "hash": "e39a6474d84e1bc10a33351b6970ade7b6eb611beed13570aeda41a37efa927d" + "hash": "86a7ff7c5f178844007b6e66ca0fe47c26034d500c35dad5442bdc14bf8d1aa8" } diff --git a/backend/.sqlx/query-bf945c34c18a9fe5f0af8f54042df39178f5111119d50f2975dd1180502c0c52.json b/backend/.sqlx/query-f8e8761da49f33e9cf7d73586757e7cc4515081955b8850bebc4ee817819e71a.json similarity index 81% rename from backend/.sqlx/query-bf945c34c18a9fe5f0af8f54042df39178f5111119d50f2975dd1180502c0c52.json rename to backend/.sqlx/query-f8e8761da49f33e9cf7d73586757e7cc4515081955b8850bebc4ee817819e71a.json index a4ad3fd76b..d365a56f7c 100644 --- a/backend/.sqlx/query-bf945c34c18a9fe5f0af8f54042df39178f5111119d50f2975dd1180502c0c52.json +++ b/backend/.sqlx/query-f8e8761da49f33e9cf7d73586757e7cc4515081955b8850bebc4ee817819e71a.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT path,\n value as \"value!: sqlx::types::Json>\",\n created_at\n FROM draft\n WHERE workspace_id = $1\n AND typ = 'flow'\n AND email = $2\n AND NOT EXISTS (\n SELECT 1 FROM flow f\n WHERE f.workspace_id = draft.workspace_id\n AND f.path = draft.path\n )", + "query": "SELECT path,\n value as \"value!: sqlx::types::Json>\",\n created_at\n FROM draft\n WHERE workspace_id = $1\n AND typ = 'flow'\n AND email = $2\n AND ($3::text IS NULL OR path LIKE $3 || '%')\n AND NOT EXISTS (\n SELECT 1 FROM flow f\n WHERE f.workspace_id = draft.workspace_id\n AND f.path = draft.path\n )", "describe": { "columns": [ { @@ -21,6 +21,7 @@ ], "parameters": { "Left": [ + "Text", "Text", "Text" ] @@ -31,5 +32,5 @@ false ] }, - "hash": "bf945c34c18a9fe5f0af8f54042df39178f5111119d50f2975dd1180502c0c52" + "hash": "f8e8761da49f33e9cf7d73586757e7cc4515081955b8850bebc4ee817819e71a" } diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index 3d35201fcc..a09b196db2 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -233,12 +233,12 @@ async fn list_flows( // // Concatenated after the deployed page so the home page surfaces // them too. Fields not in the draft JSON fall back to sensible - // defaults. Skipped when filters narrow the list or we're past - // page 0 — keeps pagination semantics clean. + // defaults. `path_start` is honored in-query (so prefix listings + // still include draft-only rows); other narrowing filters or pages + // past 0 skip the append to keep pagination semantics clean. if lq.include_draft_only.unwrap_or(false) && !authed.is_operator && offset == 0 - && lq.path_start.is_none() && lq.path_exact.is_none() && lq.edited_by.is_none() && lq.dedicated_worker.is_none() @@ -254,6 +254,7 @@ async fn list_flows( WHERE workspace_id = $1 AND typ = 'flow' AND email = $2 + AND ($3::text IS NULL OR path LIKE $3 || '%') AND NOT EXISTS ( SELECT 1 FROM flow f WHERE f.workspace_id = draft.workspace_id @@ -261,6 +262,7 @@ async fn list_flows( )"#, &w_id, &authed.email, + lq.path_start.as_deref(), ) .fetch_all(&db) .await?; diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index a98f4c0420..08d3243e02 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -343,12 +343,12 @@ async fn list_scripts( // // Concatenated after the deployed page so the home page surfaces // them too. Fields not in the draft JSON fall back to sensible - // defaults. Skipped when filters narrow the list or we're past - // page 0 — keeps pagination semantics clean. + // defaults. `path_start` is honored in-query (so prefix listings + // still include draft-only rows); other narrowing filters or pages + // past 0 skip the append to keep pagination semantics clean. if lq.include_draft_only.unwrap_or(false) && !authed.is_operator && offset == 0 - && lq.path_start.is_none() && lq.path_exact.is_none() && lq.created_by.is_none() && lq.first_parent_hash.is_none() @@ -369,6 +369,7 @@ async fn list_scripts( WHERE workspace_id = $1 AND typ = 'script' AND email = $2 + AND ($3::text IS NULL OR path LIKE $3 || '%') AND NOT EXISTS ( SELECT 1 FROM script s WHERE s.workspace_id = draft.workspace_id @@ -376,6 +377,7 @@ async fn list_scripts( )"#, &w_id, &authed.email, + lq.path_start.as_deref(), ) .fetch_all(&db) .await?; diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 0a7b8e2a1c..c77383df50 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -458,11 +458,12 @@ async fn list_apps( // // Concatenated after the deployed page so the home page surfaces // them too. Fields not in the draft JSON fall back to defaults. - // Skipped when filters narrow the list or we're past page 0. + // `path_start` is honored in-query (so prefix listings still include + // draft-only rows); other narrowing filters or pages past 0 skip the + // append to keep pagination clean. if lq.include_draft_only.unwrap_or(false) && !authed.is_operator && offset == 0 - && lq.path_start.is_none() && lq.path_exact.is_none() && lq.label.is_none() && !lq.starred_only.unwrap_or(false) @@ -476,6 +477,7 @@ async fn list_apps( WHERE workspace_id = $1 AND typ IN ('app', 'raw_app') AND email = $2 + AND ($3::text IS NULL OR path LIKE $3 || '%') AND NOT EXISTS ( SELECT 1 FROM app a WHERE a.workspace_id = draft.workspace_id @@ -483,6 +485,7 @@ async fn list_apps( )"#, &w_id, &authed.email, + lq.path_start.as_deref(), ) .fetch_all(&db) .await?; diff --git a/frontend/src/lib/components/copilot/chat/global/MIGRATION_DB_DRAFTS.md b/frontend/src/lib/components/copilot/chat/global/MIGRATION_DB_DRAFTS.md index ed6d561576..45683c34a0 100644 --- a/frontend/src/lib/components/copilot/chat/global/MIGRATION_DB_DRAFTS.md +++ b/frontend/src/lib/components/copilot/chat/global/MIGRATION_DB_DRAFTS.md @@ -139,13 +139,19 @@ row: - `draft_only` — the item exists only as a draft (never deployed) - `is_draft` — a deployed item that has a pending draft for this user -(draft-only rows also set `is_draft = true`). `listWorkspaceItems` *already* called those -endpoints with `includeDraftOnly: true` — it just hardcoded `isDraft: false`. So the fix -is to read the flags: no new endpoint, no `DraftService.listDrafts`, no `typ`→type mapper. +(draft-only rows also set `is_draft = true`). `listWorkspaceItems` *already* called the +script/flow endpoints with `includeDraftOnly: true` — it just hardcoded `isDraft: false`. +So the core fix is to read the flags: no new endpoint, no `DraftService.listDrafts`, no +`typ`→type mapper. Two follow-ups from review were also needed: the **app** call was +missing `includeDraftOnly` (issue #1), and listing under a **`path_prefix`** needed a +small backend change so draft-only rows are filtered by prefix instead of dropped (issue +#2 / option (c) — see below). ### What was implemented -- **`core.ts` `listWorkspaceItems`** — `isDraft` is now derived from the flags: +- **`core.ts` `listWorkspaceItems`** — all three list calls pass `includeDraftOnly: true` + (the app call previously omitted it — **issue #1 fix**), and `isDraft` is derived from + the returned flags: - script/flow: `draft_only === true || is_draft === true` (both fields on the list row) - app: `is_draft === true` (`ListableApp` exposes only `is_draft`; draft-only apps set it) - **`core.ts` list tool** — dropped the `listGlobalDrafts` merge. Script/flow/app drafts @@ -168,6 +174,19 @@ is to read the flags: no new endpoint, no `DraftService.listDrafts`, no `typ`→ - **`global_drafts/+page.svelte`** (dev inspector) — now calls `DraftService.listDrafts` directly (raw `{ path, typ, saved_at }`) and deletes via `DraftService.saveDraft` (`value: null`). Self-contained; no dependency on the adapter. +- **Backend — `scripts.rs` / `flows.rs` / `apps.rs` `list_*`** (**issue #2, option (c)**): + the draft-only append no longer bails when `path_start` is set; instead the draft-only + query filters by it (`AND ($N::text IS NULL OR path LIKE $N || '%')`, mirroring the + deployed query's `and_where_like_left`). Other narrowing filters (`path_exact`, + `created_by`, `label`, languages, pages past 0) still skip the append. sqlx offline cache + regenerated for the three changed queries. + +> **⚠️ Reviewer — please confirm (c):** removing the `path_start.is_none()` guard from the +> draft-only append is the chosen fix for issue #2. The guard was presumably there so +> pickers/selectors get a deployed-only listing; the global-chat list tool needs draft-only +> rows under a prefix, and honoring `path_start` in-query (rather than dropping draft-only) +> is the least-surprising behavior. If a caller relies on "prefix query ⇒ no synthesized +> draft-only rows", this changes that. Flagging for sign-off. ### Known limitation (intentional, awaiting backend) @@ -180,11 +199,13 @@ Lift it by adding `includeDraftOnly` (email-scoped) to those endpoints. - Backend confirmed live: a seeded draft-only script returns `draft_only: true, is_draft: true` from `listScripts?include_draft_only=true`, scoped to the authed email. -- Unit (`core.test.ts`): `flags backend draft scripts and forwards path_prefix + limit` - passes; the live-editor **list** assertions in `lists … the live script/flow editor - draft` pass too. Those two tests then fail later in their *edit/write* half on the - unrelated read-after-write gap (`getXByPath mock not configured` — see §3/§8). Net - suite delta: **29 → 28 failures, no regressions.** + After the (c) change, the same query **with** `path_start` set returns the draft-only row + when it matches the prefix (and omits it otherwise) — verified via the API. +- Unit (`core.test.ts`): `flags backend draft scripts and forwards path_prefix + limit` and + `requests draft-only apps and flags them via is_draft` (issue #1 guard) pass; the + live-editor **list** assertions in `lists … the live script/flow editor draft` pass too. + Those two live-editor tests then fail later in their *edit/write* half on the unrelated + read-after-write gap (`getXByPath mock not configured` — see §3/§8). No regressions. - E2E (not yet run): with `wm_dev_global_ai` enabled, create a draft script via the global mode, reload, confirm `list_workspace_items` flags it `isDraft: true`. 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 e66a265c49..8eeebd7d07 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -444,7 +444,9 @@ describe('global AI tools', () => { // Post #9351 the list tool sources script drafts from `listScripts` // (`includeDraftOnly` + the `draft_only`/`is_draft` flags), not a local // store — so path_prefix (pathStart) and limit (perPage) are forwarded to - // the backend query, which does the filtering. + // the backend query, which does the filtering. The backend now filters + // draft-only rows by `path_start` too (the (c) fix in scripts/flows/apps), + // so a draft-only row under the prefix is returned and flagged here. vi.mocked(ScriptService.listScripts).mockResolvedValueOnce([ { path: 'f/matching/inside', summary: 'Inside draft', language: 'bun', draft_only: true } ] as any) @@ -467,6 +469,25 @@ describe('global AI tools', () => { ]) }) + it('requests draft-only apps and flags them via is_draft', async () => { + // Regression guard: `listApps` must be called with `includeDraftOnly` + // (the app call previously omitted it, so DB-only app drafts were missed), + // and a draft row maps to `isDraft: true` (draft-only apps carry + // `is_draft === true`). + vi.mocked(AppService.listApps).mockResolvedValueOnce([ + { path: 'f/apps/inflight', summary: 'In-flight app', draft_only: true, is_draft: true } + ] as any) + + const raw = await callGlobalTool('list_workspace_items', { types: ['app'] }) + + expect(AppService.listApps).toHaveBeenCalledWith( + expect.objectContaining({ includeDraftOnly: true }) + ) + expect(JSON.parse(raw)).toEqual([ + expect.objectContaining({ type: 'app', path: 'f/apps/inflight', 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 90d764c33f..64f17d3c9e 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -1277,7 +1277,8 @@ async function listWorkspaceItems( const apps = await AppService.listApps({ workspace, pathStart: pathPrefix, - perPage + perPage, + includeDraftOnly: true }) // Draft-only apps also carry `is_draft === true`, so it covers both // draft-only and deployed-with-draft (ListableApp exposes only `is_draft`).