fix: an asked-for history page is bounded, and a failed one is not the end of the list

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

backend/tests/app_version_order.rs

@@ -155,6 +155,14 @@ async fn test_app_head_follows_the_append_order_not_the_timestamps(
         appended[10..20],
         "the next page carries on where the first left off, skipping nothing"
     );
+    // A page past the end runs off it rather than overflowing into one. (The clamp on an
+    // asked-for size is pinned where it lives, in `paginate_optional`'s own test.)
+    assert!(
+        versions_at("?per_page=10&page=99999999")
+            .await?
+            .is_empty(),
+        "a page past the end is empty"
+    );
 
     Ok(())
 }

backend/windmill-api-flows/src/flows.rs

@@ -52,7 +52,7 @@ use windmill_common::{
     schedule::Schedule,
     triggers::MovedNativeTrigger,
     utils::{
-        http_get_from_hub, not_found_if_none, paginate, paginate_without_limits, Pagination,
+        http_get_from_hub, not_found_if_none, paginate, paginate_optional, Pagination,
         RunnableKind, StripPath,
     },
 };
@@ -937,7 +937,7 @@ async fn get_flow_history(
     check_scopes(&authed, || format!("flows:read:{}", path))?;
     // Unasked-for, this listing stays whole: the history panels, the restart picker and
     // the CLI all read it without paging. The diff picker asks for a page.
-    let (per_page, offset) = paginate_without_limits(pagination);
+    let (per_page, offset) = paginate_optional(pagination);
     let mut tx = user_db.begin(&authed).await?;
 
     let flows = sqlx::query_as!(

backend/windmill-api-scripts/src/scripts.rs

@@ -55,7 +55,7 @@ use windmill_common::{
         min_version_supports_runnable_settings_v0, RunnableSettings, RunnableSettingsTrait,
     },
     scripts::{hash_script, ScriptRunnableSettingsHandle, ScriptRunnableSettingsInline},
-    utils::{paginate_without_limits, WarnAfterExt},
+    utils::{paginate_optional, paginate_without_limits, WarnAfterExt},
     worker::CLOUD_HOSTED,
 };
 use windmill_object_store::upload_artifact_to_store;
@@ -3094,7 +3094,7 @@ async fn get_script_history(
     check_scopes(&authed, || format!("scripts:read:{}", path))?;
     // Unasked-for, this listing stays whole: the deployment-history panels, the restart
     // picker and the CLI all read it without paging. The diff picker asks for a page.
-    let (per_page, offset) = paginate_without_limits(pagination);
+    let (per_page, offset) = paginate_optional(pagination);
     let mut tx = user_db.begin(&authed).await?;
     let query_result = sqlx::query!(
         "SELECT s.hash as hash, dm.deployment_msg as deployment_msg, s.created_at as created_at, s.created_by as created_by

backend/windmill-api/src/apps.rs

@@ -69,7 +69,7 @@ use windmill_common::{
     user_drafts::{overlay_or_draft_only, DraftUserRef, UserDraftItemKind, WithDraftOverlay},
     users::username_to_permissioned_as,
     utils::{
-        http_get_from_hub, not_found_if_none, paginate, paginate_without_limits,
+        http_get_from_hub, not_found_if_none, paginate, paginate_optional,
         query_elems_from_hub, require_admin, strip_json_nul, Pagination, RunnableKind, StripPath,
     },
     variables::{build_crypt, build_crypt_with_key_suffix, encrypt},
@@ -1243,7 +1243,7 @@ async fn get_app_history(
     check_scopes(&authed, || format!("apps:read:{}", &path))?;
     // Unasked-for, this listing stays whole: the deployment-history panel reads it
     // without paging. The diff picker asks for a page.
-    let (per_page, offset) = paginate_without_limits(pagination);
+    let (per_page, offset) = paginate_optional(pagination);
     let mut tx = user_db.begin(&authed).await?;
     // Newest first in the order the versions were deployed, which is their position in
     // `app.versions` and not `created_at`: the latter is the deploying transaction's

backend/windmill-common/src/utils.rs

@@ -479,6 +479,25 @@ pub fn paginate(pagination: Pagination) -> (usize, usize) {
     (per_page, offset)
 }
 
+/// [`paginate`] for a listing that answers whole unless a size is asked for: the deploy
+/// histories, which the history panels and the CLI read unpaged while the diff picker takes
+/// a page at a time. An asked-for size is still clamped, and the offset saturates rather
+/// than wrapping, so no caller can turn this into an unbounded scan or a negative bind.
+pub fn paginate_optional(pagination: Pagination) -> (usize, usize) {
+    let per_page = pagination
+        .per_page
+        .unwrap_or(MAX_PER_PAGE)
+        .max(1)
+        .min(MAX_PER_PAGE);
+    let offset = pagination
+        .page
+        .unwrap_or(1)
+        .max(1)
+        .saturating_sub(1)
+        .saturating_mul(per_page);
+    (per_page, offset)
+}
+
 pub fn paginate_without_limits(pagination: Pagination) -> (usize, usize) {
     let per_page = pagination.per_page.unwrap_or(MAX_PER_PAGE);
     let offset = (pagination.page.unwrap_or(1).max(1) - 1) * per_page;
@@ -1657,6 +1676,31 @@ pub fn truncate_with_ellipsis(s: &str, max_chars: usize) -> String {
 mod tests {
     use super::*;
 
+    #[test]
+    fn test_paginate_optional_answers_whole_but_bounds_what_is_asked_for() {
+        // Nothing asked for: the whole listing, which is what the history panels and the
+        // CLI read.
+        assert_eq!(
+            paginate_optional(Pagination { page: None, per_page: None }),
+            (MAX_PER_PAGE, 0)
+        );
+        assert_eq!(
+            paginate_optional(Pagination { page: Some(3), per_page: Some(20) }),
+            (20, 40)
+        );
+        // An asked-for size is still capped, so no caller turns this into an unbounded scan.
+        assert_eq!(
+            paginate_optional(Pagination { page: None, per_page: Some(usize::MAX) }),
+            (MAX_PER_PAGE, 0)
+        );
+        // And the offset saturates instead of wrapping into a negative bind.
+        assert_eq!(
+            paginate_optional(Pagination { page: Some(usize::MAX), per_page: Some(20) }).0,
+            20
+        );
+        assert!(paginate_optional(Pagination { page: Some(usize::MAX), per_page: Some(20) }).1 > 0);
+    }
+
     /// A 5-field crontab line is the most common way to get a schedule rejected, and both
     /// parsers report it in terms a crontab user cannot act on, so the seconds field and the
     /// equivalent expression must reach the caller for v1 and v2 alike.

frontend/src/lib/components/DiffDrawer.svelte

@@ -181,7 +181,9 @@
 				sendUserToast(`Could not load older versions: ${e?.body ?? e?.message ?? e}`, true)
 			}
 		} finally {
-			loadingMore = false
+			// Guarded like the writes above: an outlived request clearing this would hand
+			// the drawer that replaced it a second concurrent page.
+			if (generation === versionListGeneration) loadingMore = false
 		}
 	}
 

frontend/src/lib/components/FlowBuilder.svelte

@@ -1138,10 +1138,11 @@
 
 	/** Deployed versions for the diff picker, newest first. Best-effort: losing the
 	 *  list costs the picker, not the diff. */
-	async function deployedVersionOptions(page = 1) {
+	/** Throws: the drawer says so and lets the reader ask for the same page again. */
+	async function fetchVersionPage(page: number) {
 		const path = userDraftPath || initialPath
 		if (!opWorkspace || !path) return undefined
-		try {
+		{
 			const history = await FlowService.getFlowHistory({
 				workspace: opWorkspace,
 				path,
@@ -1168,16 +1169,28 @@
 					isHead
 				}
 			})
+		}
+	}
+
+	/** The first page, best-effort: losing it costs the picker, not the diff. */
+	async function deployedVersionOptions() {
+		try {
+			return await fetchVersionPage(1)
 		} catch {
 			return undefined
 		}
 	}
 
-	/** Hands the drawer the next page each time the reader asks for one. Held here rather
-	 *  than in the drawer because the page number belongs to this item's history. */
+	/** Hands the drawer the next page each time the reader asks for one. The page number
+	 *  belongs to this item's history, so it lives here — and only moves once a page has
+	 *  actually arrived, or a failed request would skip it. */
 	function moreVersionsLoader() {
-		let page = 1
-		return async () => deployedVersionOptions(++page)
+		let loaded = 1
+		return async () => {
+			const page = await fetchVersionPage(loaded + 1)
+			loaded += 1
+			return page
+		}
 	}
 
 	/** The opening this editor claimed last. A path change remounts this editor while the

frontend/src/lib/components/ScriptBuilder.svelte

@@ -844,9 +844,10 @@
 	/** Deployed versions to offer in the diff picker, newest first. Best-effort: a
 	 *  failure here costs the picker, not the diff, so the drawer still opens on the
 	 *  head. `deployment_msg` is all the history endpoint carries besides the hash. */
-	async function deployedVersionOptions(headHash: string | undefined, page = 1) {
+	/** Throws: the drawer says so and lets the reader ask for the same page again. */
+	async function fetchVersionPage(headHash: string | undefined, page: number) {
 		if (!opWorkspace || !userDraftPath) return undefined
-		try {
+		{
 			const history = await ScriptService.getScriptHistoryByPath({
 				workspace: opWorkspace,
 				path: userDraftPath,
@@ -870,16 +871,28 @@
 					isHead
 				}
 			})
+		}
+	}
+
+	/** The first page, best-effort: losing it costs the picker, not the diff. */
+	async function deployedVersionOptions(headHash: string | undefined) {
+		try {
+			return await fetchVersionPage(headHash, 1)
 		} catch {
 			return undefined
 		}
 	}
 
-	/** Hands the drawer the next page each time the reader asks for one. Held here rather
-	 *  than in the drawer because the page number belongs to this item's history. */
+	/** Hands the drawer the next page each time the reader asks for one. The page number
+	 *  belongs to this item's history, so it lives here — and only moves once a page has
+	 *  actually arrived, or a failed request would skip it. */
 	function moreVersionsLoader(headHash: string | undefined) {
-		let page = 1
-		return async () => deployedVersionOptions(headHash, ++page)
+		let loaded = 1
+		return async () => {
+			const page = await fetchVersionPage(headHash, loaded + 1)
+			loaded += 1
+			return page
+		}
 	}
 
 	/** The opening this editor claimed last. A path change remounts this editor while the

frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte

@@ -433,9 +433,10 @@
 
 	/** Deployed versions for the diff picker, newest first. Best-effort: losing the
 	 *  list costs the picker, not the diff. */
-	async function deployedVersionOptions(page = 1) {
+	/** Throws: the drawer says so and lets the reader ask for the same page again. */
+	async function fetchVersionPage(page: number) {
 		if (!opWorkspace || !appPath) return undefined
-		try {
+		{
 			const history = await AppService.getAppHistoryByPath({
 				workspace: opWorkspace,
 				path: appPath,
@@ -460,16 +461,28 @@
 					isHead
 				}
 			})
+		}
+	}
+
+	/** The first page, best-effort: losing it costs the picker, not the diff. */
+	async function deployedVersionOptions() {
+		try {
+			return await fetchVersionPage(1)
 		} catch {
 			return undefined
 		}
 	}
 
-	/** Hands the drawer the next page each time the reader asks for one. Held here rather
-	 *  than in the drawer because the page number belongs to this item's history. */
+	/** Hands the drawer the next page each time the reader asks for one. The page number
+	 *  belongs to this item's history, so it lives here — and only moves once a page has
+	 *  actually arrived, or a failed request would skip it. */
 	function moreVersionsLoader() {
-		let page = 1
-		return async () => deployedVersionOptions(++page)
+		let loaded = 1
+		return async () => {
+			const page = await fetchVersionPage(loaded + 1)
+			loaded += 1
+			return page
+		}
 	}
 
 	/** The opening this editor claimed last. A path change remounts this editor while the