feat: list draft-only runnables on the homepage again (#10361)

* feat: list draft-only runnables on the homepage again

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf: trim the draft listing index to the columns that measure

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: address review findings on draft-only runnables

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-07-27 18:17:32 +02:00
committed by GitHub
parent 86e7f18f09
commit 7973549e7f
7 changed files with 320 additions and 16 deletions
@@ -0,0 +1 @@
DROP INDEX IF EXISTS draft_kind_user_listing_idx;
@@ -0,0 +1,22 @@
-- Index backing draft-only rows in the unified homepage listing
-- (/runnables/list) and its per-owner counts (/runnables/counts).
--
-- Every draft query there is "this caller's drafts of this kind", so `typ` and
-- `email` are key columns rather than filters: without them the existing
-- draft_user_listing_idx hands back the caller's drafts of every kind and each
-- per-kind branch throws most away.
--
-- Deliberately just these three columns. `path` would not help, because a draft
-- is listed, grouped and filtered under the path it says it will deploy to,
-- which lives in the draft JSON and is only resolved after the DISTINCT ON
-- dedup, so no index can prune on it. Nor can "draft-only" itself be pushed
-- into the index: it means "no deployed row at this path", and Postgres rejects
-- a subquery in an index predicate. The anti-join prunes that at query time off
-- the deployed tables' own (workspace_id, path) indexes.
--
-- Keep this file free of statement separators outside the statement below,
-- comments included: the CONCURRENTLY rewrite in windmill-api/src/db.rs splits
-- the file on them and would run the trailing comment text as SQL.
-- Created CONCURRENTLY via the OVERRIDDEN_MIGRATIONS rewrite in windmill-api/src/db.rs.
CREATE INDEX IF NOT EXISTS draft_kind_user_listing_idx
ON draft (workspace_id, typ, email);
+15
View File
@@ -10564,6 +10564,14 @@ paths:
description: opaque keyset cursor from a previous page's next_cursor
schema:
type: string
- name: include_draft_only
in: query
description: >-
also list the caller's drafts at paths with no deployed row, sorted
and paginated with the deployed ones. Ignored for operators, in the
archived view, and under a label filter (a draft carries no labels).
schema:
type: boolean
responses:
"200":
description: a page of merged, ordered runnables
@@ -10598,6 +10606,13 @@ paths:
description: include library scripts (no runnable main)
schema:
type: boolean
- name: include_draft_only
in: query
description: >-
also count the caller's drafts at paths with no deployed row,
matching the same flag on /runnables/list
schema:
type: boolean
responses:
"200":
description: >-
+3
View File
@@ -99,6 +99,9 @@ lazy_static::lazy_static! {
(20260727093955, include_str!(
"../../migrations/20260727093955_runnable_owner_prefix_indexes.up.sql"
).replace("CREATE INDEX", "CREATE INDEX CONCURRENTLY")),
(20260727151319, include_str!(
"../../migrations/20260727151319_draft_only_listing_indexes.up.sql"
).replace("CREATE INDEX", "CREATE INDEX CONCURRENTLY")),
].into_iter().collect();
}
+238 -6
View File
@@ -64,6 +64,9 @@ struct ListRunnablesQuery {
per_page: Option<usize>,
/// Opaque keyset cursor from a previous page's `next_cursor`.
cursor: Option<String>,
/// Also list the caller's drafts at paths with no deployed row. Off by
/// default so picker callers stay deployed-only.
include_draft_only: Option<bool>,
}
// Absent optional fields are omitted (not serialized as null) to match the
@@ -289,6 +292,65 @@ fn branch_sqls() -> Branches {
Branches { script, flow, app }
}
/// The draft-only branch for a kind: the caller's drafts at paths carrying no
/// deployed row, projected into the same column set as `branch_sqls` so they
/// sort, search and paginate as ordinary rows. Same `$1`/`$2`/`$3` contract.
fn draft_branch_sql(kind: &str) -> String {
let (typ_pred, deployed) = match kind {
"script" => ("d.typ = 'script'", "script"),
"flow" => ("d.typ = 'flow'", "flow"),
_ => ("d.typ IN ('app', 'raw_app')", "app"),
};
// Scripts bind the Path widget to `script.path`, so the typed path round-trips
// through the draft JSON's own `path`; flows and apps write a separate
// `draft_path` only when it differs from the deployed one. See scripts.rs.
let typed_path = if kind == "script" {
"path"
} else {
"draft_path"
};
// `auto_kind` is only what the editor stamped into the draft — a `// pipeline`
// annotation is not re-derived from the content here, unlike the per-kind
// endpoints. That errs toward listing a pipeline member as its own row rather
// than folding it into a pipeline entry and hiding it.
let kind_cols = match kind {
"script" => {
"d.value->>'language' as language, d.value->>'kind' as script_kind, \
d.value->>'auto_kind' as auto_kind, false as raw_app"
}
_ => {
"NULL::text as language, NULL::text as script_kind, NULL::text as auto_kind, \
(d.typ = 'raw_app') as raw_app"
}
};
format!(
"SELECT '{kind}' as kind, o.path, o.summary, o.workspace_id, '{{}}'::jsonb as extra_perms, \
false as starred, false as archived, \
true as is_draft, true as draft_only, o.draft_path, \
json_build_array(json_build_object('username', $2::text)) as draft_users, \
NULL::text[] as labels, NULL::text[] as inherited_labels, \
NULL::bool as ws_error_handler_muted, o.created_at as edited_at, \
NULL::bigint as hash, o.language, o.script_kind, o.auto_kind, \
NULL::bool as use_codebase, NULL::bool as has_deploy_errors, \
o.raw_app, NULL::text as execution_mode, NULL::bigint as id, NULL::bigint as version, \
o.created_at as sort_time, lower(COALESCE(NULLIF(o.summary, ''), o.draft_path, o.path)) as sort_name, 0::bigint as tiebreak \
FROM ( \
SELECT DISTINCT ON (d.path) d.workspace_id, d.path, d.created_at, \
COALESCE(d.value->>'summary', '') as summary, \
NULLIF(NULLIF(d.value->>'{typed_path}', ''), d.path) as draft_path, \
{kind_cols} \
FROM draft d \
WHERE d.workspace_id = $1 AND {typ_pred} AND (d.email = $3 OR d.email IS NULL) \
AND NOT EXISTS (SELECT 1 FROM {deployed} x \
WHERE x.workspace_id = d.workspace_id AND x.path = d.path) \
-- Owned draft over a legacy NULL-email one, then newest: the app branch spans
-- two draft kinds (`app` and `raw_app`) that can both exist at a path, and the
-- pick decides raw_app, summary and draft_path. Same tiebreak as apps.rs.
ORDER BY d.path, (d.email IS NULL), d.created_at DESC \
) o"
)
}
async fn list_runnables(
authed: ApiAuthed,
Extension(user_db): Extension<UserDB>,
@@ -331,13 +393,24 @@ async fn list_runnables(
};
let mut common: Vec<String> = vec!["o.workspace_id = $1".to_string()];
// Same predicates for the draft branches, except the search: a draft is named by
// the path typed in the editor, its stored path being a generated `draft_<uuid>`
// nobody searches for. Labels never apply (the draft branches are dropped under a
// label filter), so they are not repeated here.
let mut draft_common: Vec<String> = vec!["o.workspace_id = $1".to_string()];
if let Some(ps) = q.path_start.as_ref().filter(|s| !s.is_empty()) {
let p = add_bind(&mut binds, format!("{}%", escape_like(ps)));
common.push(format!("o.path LIKE {}", p));
// An owner filter follows where the draft says it will live, not the
// `u/<caller>/draft_<uuid>` it is parked at.
draft_common.push(format!("COALESCE(o.draft_path, o.path) LIKE {}", p));
}
if let Some(search) = q.search.as_ref().filter(|s| !s.is_empty()) {
let p = add_bind(&mut binds, format!("%{}%", escape_like(search)));
common.push(format!("(o.summary ILIKE {p} OR o.path ILIKE {p})"));
draft_common.push(format!(
"(o.summary ILIKE {p} OR o.path ILIKE {p} OR o.draft_path ILIKE {p})"
));
}
if let Some(label) = q.label.as_ref().filter(|s| !s.is_empty()) {
for l in label.split(',') {
@@ -348,6 +421,7 @@ async fn list_runnables(
}
}
let common_where = common.join(" AND ");
let draft_common_where = draft_common.join(" AND ");
// Keyset predicate for pages after the first (non-starred rows only). A
// row-value comparison keeps the composite order; the key is cast to the
@@ -428,11 +502,38 @@ async fn list_runnables(
app_extras.push(s.clone());
}
// Draft-only rows are the caller's own work in progress: never archived, so they
// have no place in the archived view, and carrying no labels of their own they are
// out of scope of a label filter (as in the per-kind endpoints). Operators don't
// see other people's drafts and have none of their own to see.
let include_drafts = q.include_draft_only.unwrap_or(false)
&& !authed.is_operator
&& !show_archived
&& q.label.as_ref().filter(|s| !s.is_empty()).is_none();
let draft_extras_for = |kind: &str| -> Vec<String> {
let mut extras: Vec<String> = vec![];
let scope = match kind {
"script" => &script_scope,
"flow" => &flow_scope,
_ => &app_scope,
};
// The lib filter reads the same projected `auto_kind`, so a draft-only library
// script hides with the deployed ones.
if kind == "script" && (!q.include_without_main.unwrap_or(false) || authed.is_operator) {
extras.push("(o.auto_kind IS NULL OR o.auto_kind <> 'lib')".to_string());
}
if let Some(s) = scope {
extras.push(s.clone());
}
extras
};
// Favorite filter for a branch: Some(true) = starred only, Some(false) =
// non-starred only, None = no filter. Both views pin starred on the first page
// (each is one row per path), so the paged stream always passes Some(false).
let build_branch = |base: &str,
kind: &str,
common: &str,
extras: &[String],
fav: Option<bool>,
keyset: Option<&str>,
@@ -441,7 +542,7 @@ async fn list_runnables(
// Base-table predicates go inside the projection subquery (they read
// o.*/favorite.*); the keyset reads the projected sort aliases, so it
// sits in the wrapper WHERE where those aliases are visible.
let mut w = vec![common_where.clone()];
let mut w = vec![common.to_string()];
w.extend(extras.iter().cloned());
match fav {
Some(true) => w.push("favorite.path IS NOT NULL".to_string()),
@@ -476,7 +577,37 @@ async fn list_runnables(
"app" => (&branches.app, &app_extras),
_ => return None,
};
Some(build_branch(base, kind, extras, fav, keyset, limit))
Some(build_branch(
base,
kind,
&common_where,
extras,
fav,
keyset,
limit,
))
};
let draft_branch_for = |kind: &str,
fav: Option<bool>,
keyset: Option<&str>,
limit: Option<usize>|
-> Option<String> {
if !include_drafts || !kinds.contains(&kind) {
return None;
}
// `fav` is ignored: with no favorite join there is nothing to filter on, and the
// starred pass skips draft branches entirely.
let _ = fav;
Some(build_branch(
&draft_branch_sql(kind),
&format!("draft_{kind}"),
&draft_common_where,
&draft_extras_for(kind),
None,
keyset,
limit,
))
};
let run_union = |branches_sql: Vec<String>, limit: Option<usize>| -> String {
@@ -497,6 +628,9 @@ async fn list_runnables(
// favorite is a single row in either — the starred-first contract holds in the
// archived view too, and the pinned first page stays bounded.
if first_page {
// No draft branch here: a draft-only path has no favorite row (the UI won't let
// you star one), so it would scan the caller's whole draft slice per kind to
// return nothing. The main stream below takes them unfiltered instead.
let starred_branches: Vec<String> = ["script", "flow", "app"]
.iter()
.filter_map(|k| branch_for(k, Some(true), None, None))
@@ -516,10 +650,14 @@ async fn list_runnables(
// Main paged stream: non-starred rows (starred were pinned on the first page above).
let main_fav = Some(false);
let ns_branches: Vec<String> = ["script", "flow", "app"]
.iter()
.filter_map(|k| branch_for(k, main_fav, keyset_sql.as_deref(), Some(per_page)))
.collect();
let ns_branches: Vec<String> =
["script", "flow", "app"]
.iter()
.filter_map(|k| branch_for(k, main_fav, keyset_sql.as_deref(), Some(per_page)))
.chain(["script", "flow", "app"].iter().filter_map(|k| {
draft_branch_for(k, main_fav, keyset_sql.as_deref(), Some(per_page))
}))
.collect();
let mut next_cursor: Option<String> = None;
if !ns_branches.is_empty() {
@@ -551,6 +689,9 @@ struct CountRunnablesQuery {
kinds: Option<String>,
/// Include library scripts (no runnable main). Ignored for flows/apps.
include_without_main: Option<bool>,
/// Also count the caller's drafts at paths with no deployed row, matching
/// the same flag on `/list`.
include_draft_only: Option<bool>,
}
#[derive(Serialize)]
@@ -702,6 +843,7 @@ async fn count_runnables_by_owner(
for r in query.fetch_all(&db).await? {
counts.insert(r.owner, r.count);
}
add_draft_counts(&authed, &db, &w_id, &kinds, with_libs, &q, &mut counts).await?;
counts.retain(|_, c| *c > 0);
return Ok(Json(RunnableCountsResponse { counts }));
}
@@ -774,6 +916,96 @@ async fn count_runnables_by_owner(
counts.entry(r.owner).or_insert(r.count);
}
add_draft_counts(&authed, &db, &w_id, &kinds, with_libs, &q, &mut counts).await?;
counts.retain(|_, c| *c > 0);
Ok(Json(RunnableCountsResponse { counts }))
}
/// Adds the caller's draft-only rows to `counts`, in the same shape `/list`
/// returns them so a badge never disagrees with the rows behind it.
///
/// Not restricted to the readable owners the deployed passes walk: a draft is
/// the caller's own and `/list` reads it off the non-RLS `draft` table, so
/// scoping it to folder grants would hide a count for a row that still lists.
/// Deployed and draft rows can't overlap (the anti-join is what makes a draft
/// "draft-only"), so the two counts add.
async fn add_draft_counts(
authed: &ApiAuthed,
db: &DB,
w_id: &str,
kinds: &[&str],
with_libs: bool,
q: &CountRunnablesQuery,
counts: &mut HashMap<String, i64>,
) -> Result<(), Error> {
if !q.include_draft_only.unwrap_or(false) || authed.is_operator {
return Ok(());
}
// $1 = workspace, $2 = the caller's email.
let mut binds: Vec<String> = vec![];
let branches: Vec<String> = kinds
.iter()
.map(|kind| {
let (typ_pred, deployed, domain) = match *kind {
"script" => ("d.typ = 'script'", "script", "scripts"),
"flow" => ("d.typ = 'flow'", "flow", "flows"),
_ => ("d.typ IN ('app', 'raw_app')", "app", "apps"),
};
// The owner a draft counts under is where it says it will live, not the
// `u/<caller>/draft_<uuid>` it is parked at — same path `/list` groups and
// filters on. Scripts round-trip the typed path through the draft JSON's
// own `path`; flows and apps write `draft_path`. See scripts.rs.
let typed_path = if *kind == "script" { "path" } else { "draft_path" };
let effective_path =
format!("COALESCE(NULLIF(d.value->>'{typed_path}', ''), d.path) as path");
let mut w = vec![
"d.workspace_id = $1".to_string(),
typ_pred.to_string(),
"(d.email = $2 OR d.email IS NULL)".to_string(),
format!(
"NOT EXISTS (SELECT 1 FROM {deployed} x \
WHERE x.workspace_id = d.workspace_id AND x.path = d.path)"
),
];
if *kind == "script" {
// Same rule as the deployed count, over the `auto_kind` the editor
// stamped into the draft: a pipeline member is folded into its
// pipeline entry rather than listed, and `lib` follows the caller.
let mut hidden = vec!["'pipeline'"];
if !with_libs {
hidden.push("'lib'");
}
w.push(format!(
"(d.value->>'auto_kind' IS NULL OR d.value->>'auto_kind' NOT IN ({}))",
hidden.join(", ")
));
}
if let Some(s) = scope_path_predicate(authed, domain, "d", 2, &mut binds) {
w.push(s);
}
// DISTINCT ON collapses a path holding both the caller's draft and a
// legacy NULL-email one, which `/list` shows as a single row. Wrapped
// because its ORDER BY would otherwise bind to the whole UNION.
format!(
"SELECT path FROM (SELECT DISTINCT ON (d.path) {effective_path} FROM draft d WHERE {} ORDER BY d.path, (d.email IS NULL), d.created_at DESC) s",
w.join(" AND ")
)
})
.collect();
let sql = format!(
"SELECT split_part(p.path, '/', 1) || '/' || split_part(p.path, '/', 2) AS owner, \
count(*)::bigint AS count \
FROM ({}) p GROUP BY 1",
branches.join(" UNION ALL ")
);
let mut query = sqlx::query_as::<_, OwnerCount>(&sql)
.bind(w_id)
.bind(&authed.email);
for b in &binds {
query = query.bind(b);
}
for r in query.fetch_all(db).await? {
*counts.entry(r.owner).or_insert(0) += r.count;
}
Ok(())
}
@@ -47,7 +47,7 @@
import DrawerContent from '../common/drawer/DrawerContent.svelte'
import Item from './Item.svelte'
import TreeViewRoot from './TreeViewRoot.svelte'
import type { ItemType } from './treeViewUtils'
import { effectivePath, type ItemType } from './treeViewUtils'
import Popover from '$lib/components/meltComponents/Popover.svelte'
import { getContext, tick, untrack } from 'svelte'
import { triggerableByAI } from '$lib/actions/triggerableByAI.svelte'
@@ -260,6 +260,9 @@
// so a folder's full contents load on demand rather than relying on the
// folder happening to be within the loaded browse window.
pathStart: ownerFilter ? ownerFilter + '/' : undefined,
// Your own not-yet-deployed work belongs in the list you browse; the
// endpoint sorts and pages it with everything else.
includeDraftOnly: true,
perPage: 100,
cursor
})
@@ -284,7 +287,7 @@
if (it.type === 'script') {
// Pipeline-member scripts are folded into their pipeline entry.
if (it.auto_kind === 'pipeline') {
const m = it.path.match(/^f\/([^/]+)\//)
const m = effectivePath(it).match(/^f\/([^/]+)\//)
if (m) memberFolders.add(m[1])
continue
}
@@ -419,6 +422,7 @@
includeWithoutMain: includeWithoutMain ? true : undefined,
kinds: itemKind !== 'all' ? itemKind : undefined,
pathStart: `${owner}/`,
includeDraftOnly: true,
perPage: 100,
cursor: more ? st?.cursor : undefined
})
@@ -436,7 +440,9 @@
// other owner's untouched) so a re-sort/re-filter swaps its items atomically
// without blanking the whole tree; load-more appends to what's already shown.
const prefix = `${owner}/`
const base = more ? treeOwnerItems : treeOwnerItems.filter((x) => !x.path.startsWith(prefix))
const base = more
? treeOwnerItems
: treeOwnerItems.filter((x) => !effectivePath(x).startsWith(prefix))
const have = new Set(base.map(itemKey))
const merged = [...base]
for (const it of res.items ?? []) {
@@ -483,7 +489,7 @@
// previous scope's items instead of re-fetching. Drop them and let expand reload.
if (treeLazyMode) {
const open = new Set(toReload)
treeOwnerItems = treeOwnerItems.filter((x) => open.has(ownerOf(x.path)))
treeOwnerItems = treeOwnerItems.filter((x) => open.has(ownerOf(effectivePath(x))))
ownerLoad = Object.fromEntries(Object.entries(ownerLoad).filter(([o]) => open.has(o)))
}
await loadRunnables(true)
@@ -507,9 +513,10 @@
filterUserFoldersType: 'only f/*' | 'u/username and f/*' | undefined
) {
if (!filterUserFoldersType || !filterUserFolders) return true
if (filterUserFoldersType === 'only f/*') return item.path.startsWith('f/')
const path = effectivePath(item)
if (filterUserFoldersType === 'only f/*') return path.startsWith('f/')
if (filterUserFoldersType === 'u/username and f/*')
return item.path.startsWith('f/') || item.path.startsWith(`u/${$userStore?.username}/`)
return path.startsWith('f/') || path.startsWith(`u/${$userStore?.username}/`)
return true // should not happen
}
@@ -705,7 +712,7 @@
// counting 0 gets no chip at all — including your own space.
let owners = $derived.by(() => {
const self = $userStore?.username ? `u/${$userStore.username}` : undefined
const loaded = filteredItems?.map((x) => x.path.split('/').slice(0, 2).join('/')) ?? []
const loaded = filteredItems?.map((x) => ownerOf(effectivePath(x))) ?? []
if (ownerCounts == undefined) {
// Counts still in flight: the folder/user lists resolve first, so painting the full
// list here would show the wall this drops and snap to the ranked set a tick later.
@@ -769,7 +776,9 @@
const res = await ScriptService.countRunnablesByOwner({
workspace: ws,
kinds: kind !== 'all' ? kind : undefined,
includeWithoutMain: withoutMain ? true : undefined
includeWithoutMain: withoutMain ? true : undefined,
// Same scope as the listing, so a badge counts the rows behind it.
includeDraftOnly: true
})
return res.counts
} catch {
@@ -866,6 +875,7 @@
includeWithoutMain: withoutMain ? true : undefined,
kinds: kind !== 'all' ? kind : undefined,
pathStart: owner ? owner + '/' : undefined,
includeDraftOnly: true,
perPage: 1000
})
} catch {
@@ -912,6 +922,7 @@
includeWithoutMain: withoutMain ? true : undefined,
kinds: kind !== 'all' ? kind : undefined,
pathStart: owner ? owner + '/' : undefined,
includeDraftOnly: true,
perPage: 1000,
cursor
})
@@ -1308,7 +1319,12 @@
{filter}
items={preFilteredItems}
bind:filteredItems
f={(x) => (x.summary ? x.summary + ' (' + x.path + ')' : x.path)}
f={(x) => {
// A draft-only row is named by the path typed in the editor — its stored path is a
// generated `draft_<uuid>` nobody types into the search box.
const p = x.draft_only && x.draft_path ? x.draft_path : x.path
return x.summary ? x.summary + ' (' + p + ')' : p
}}
{opts}
/>
@@ -27,6 +27,21 @@ export type UserItem = {
items: (ItemType | FolderItem)[]
}
/**
* Where an item belongs: its owner, folder, and the name it is filtered and searched by.
* A draft-only item is parked at a generated `u/<you>/draft_<uuid>` but names the path it
* will deploy to, and that is what the row shows and what the server lists, filters and
* counts it under — so every categorization has to follow it. `path` stays the storage
* identity that the editor link and the row key resolve.
*/
export function effectivePath(item: {
path: string
draft_only?: boolean | null
draft_path?: string | null
}): string {
return (item.draft_only && item.draft_path) || item.path
}
function insertItemInFolder(
root: (ItemType | FolderItem | UserItem)[],
item: ItemType,
@@ -75,7 +90,7 @@ export function groupItems(
const root: (ItemType | FolderItem | UserItem)[] = []
items.forEach((item) => {
const pathSplit = item.path.split('/')
const pathSplit = effectivePath(item).split('/')
if (pathSplit[0] === 'u') {
const username = pathSplit[1]
let userItem = root.find((f): f is UserItem => 'username' in f && f.username === username) as