From bd5d8d864ae4981058681977cc69b6745dbaeec8 Mon Sep 17 00:00:00 2001 From: Blomios Date: Tue, 14 Jul 2026 19:20:40 +0200 Subject: [PATCH] fix(tickets): matching exact du #ref et curseur de pagination opaque/stable pour ticket_list (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Épurement de la dette de ticket_list sur deux axes : Recherche texte — matching exact du numéro/#ref via parse_issue_search_ref, court-circuité avant load_issue, sans matching par substring numérique (« 1 » ne remonte plus « 12 », « 123 »…). Pagination — curseur opaque et stable anchor-based au lieu d'un offset fragile : token v1. encodant le tri + l'ancre {number, sortKey}, reprise strictement après l'ancre. Curseur legacy / invalide / de version inconnue / avec sort divergent rejeté par une erreur explicite « Invalid cursor ». Le DTO cursor reste String → non-breaking côté UI. Fichiers : infrastructure/src/issues.rs, app-tauri/src/tickets.rs, app-tauri/Cargo.toml (+ base64 0.22). Tests : issue_store_text_filter, ticket_list_ 7/7 (anchor-based + rejets). Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 1 + crates/app-tauri/Cargo.toml | 1 + crates/app-tauri/src/tickets.rs | 284 ++++++++++++++++++--- crates/infrastructure/src/issues.rs | 24 +- crates/infrastructure/tests/issue_store.rs | 126 +++++++++ 5 files changed, 401 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0b5e50a..4b6eccc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -73,6 +73,7 @@ version = "0.3.0" dependencies = [ "application", "async-trait", + "base64 0.22.1", "domain", "infrastructure", "interprocess", diff --git a/crates/app-tauri/Cargo.toml b/crates/app-tauri/Cargo.toml index dbcaea3..c2ba971 100644 --- a/crates/app-tauri/Cargo.toml +++ b/crates/app-tauri/Cargo.toml @@ -32,6 +32,7 @@ serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } uuid = { workspace = true } +base64 = "0.22" # `AppAgentResumer` implements the application's async `AgentResumer` port (LS7). async-trait = { workspace = true } # Cross-OS local IPC for the MCP loopback transport (M5a): Unix domain socket diff --git a/crates/app-tauri/src/tickets.rs b/crates/app-tauri/src/tickets.rs index c0ed986..37f9452 100644 --- a/crates/app-tauri/src/tickets.rs +++ b/crates/app-tauri/src/tickets.rs @@ -4,10 +4,12 @@ #![allow(missing_docs)] +use std::cmp::Ordering; use std::str::FromStr; use std::sync::Arc; use async_trait::async_trait; +use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use tauri::State; @@ -208,7 +210,7 @@ pub struct TicketListRequestDto { } /// List sort request. -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct TicketListSortDto { pub field: TicketListSortFieldDto, @@ -216,7 +218,7 @@ pub struct TicketListSortDto { } /// List sort field. -#[derive(Debug, Clone, Copy, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum TicketListSortFieldDto { Number, @@ -226,7 +228,7 @@ pub enum TicketListSortFieldDto { } /// List sort direction. -#[derive(Debug, Clone, Copy, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum TicketListSortDirectionDto { Asc, @@ -455,7 +457,9 @@ impl TicketToolProvider for AppTicketToolProvider { .await .map_err(ticket_error)? .sprints; - json!(paginate_with_sprints(rows, req.limit, req.cursor, &sprints)) + let page = paginate_with_sprints(rows, req.limit, req.cursor, req.sort, &sprints) + .map_err(|e| TicketToolError::new("invalid", e.message))?; + json!(page) } "idea_sprint_list" => { let rows = self @@ -722,7 +726,7 @@ pub async fn ticket_list( .map_err(ErrorDto::from)? .issues; sort_ticket_rows(&mut rows, page.sort); - Ok(paginate(rows, page.limit, page.cursor)) + paginate(rows, page.limit, page.cursor, page.sort) } #[tauri::command] @@ -1219,6 +1223,30 @@ fn status_rank(status: IssueStatus) -> u8 { } } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct TicketCursorToken { + v: u8, + sort: Option, + anchor: TicketCursorAnchor, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct TicketCursorAnchor { + number: u64, + sort_key: TicketCursorSortKey, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", tag = "kind", content = "value")] +enum TicketCursorSortKey { + Number(u64), + Priority(u8), + Status(u8), + Title { lower: String, raw: String }, +} + async fn resolve_project(state: &AppState, project_id: &str) -> Result { let id = ProjectId::from_uuid( Uuid::parse_str(project_id) @@ -1305,44 +1333,149 @@ fn parse_link_request(link: TicketLinkRequestDto) -> Result }) } -fn paginate(rows: Vec, limit: usize, cursor: Option) -> TicketListDto { - let start = cursor - .as_deref() - .and_then(|raw| raw.parse::().ok()) - .unwrap_or(0); - let total = rows.len(); - let end = total.min(start.saturating_add(limit)); - TicketListDto { - items: rows - .into_iter() - .skip(start) - .take(limit) - .map(TicketSummaryDto::from) - .collect(), - next_cursor: (end < total).then(|| end.to_string()), - } +fn paginate( + rows: Vec, + limit: usize, + cursor: Option, + sort: Option, +) -> Result { + paginate_rows(rows, limit, cursor, sort, TicketSummaryDto::from) } fn paginate_with_sprints( rows: Vec, limit: usize, cursor: Option, + sort: Option, sprints: &[application::SprintListEntry], -) -> TicketListDto { - let start = cursor - .as_deref() - .and_then(|raw| raw.parse::().ok()) - .unwrap_or(0); +) -> Result { + paginate_rows(rows, limit, cursor, sort, |row| { + TicketSummaryDto::from_row_with_sprints(row, sprints) + }) +} + +fn paginate_rows( + rows: Vec, + limit: usize, + cursor: Option, + sort: Option, + map_row: impl Fn(IssueIndexEntry) -> TicketSummaryDto, +) -> Result { + let start = cursor_start(&rows, cursor.as_deref(), sort)?; let total = rows.len(); let end = total.min(start.saturating_add(limit)); - TicketListDto { + let next_cursor = if end < total && end > start { + Some(encode_ticket_cursor(&rows[end - 1], sort)?) + } else { + None + }; + Ok(TicketListDto { items: rows .into_iter() .skip(start) .take(limit) - .map(|row| TicketSummaryDto::from_row_with_sprints(row, sprints)) + .map(map_row) .collect(), - next_cursor: (end < total).then(|| end.to_string()), + next_cursor, + }) +} + +fn cursor_start( + rows: &[IssueIndexEntry], + cursor: Option<&str>, + sort: Option, +) -> Result { + let Some(raw) = cursor else { + return Ok(0); + }; + let token = decode_ticket_cursor(raw)?; + if token.sort != sort { + return Err(ErrorDto::invalid("Invalid cursor: sort mismatch")); + } + Ok(rows + .iter() + .position(|row| compare_row_to_anchor(row, &token.anchor, sort) == Ordering::Greater) + .unwrap_or(rows.len())) +} + +fn encode_ticket_cursor( + row: &IssueIndexEntry, + sort: Option, +) -> Result { + let token = TicketCursorToken { + v: 1, + sort, + anchor: row_cursor_anchor(row, sort), + }; + let json = serde_json::to_vec(&token) + .map_err(|err| ErrorDto::invalid(format!("Invalid cursor: {err}")))?; + Ok(format!("v1.{}", URL_SAFE_NO_PAD.encode(json))) +} + +fn decode_ticket_cursor(raw: &str) -> Result { + let encoded = raw + .strip_prefix("v1.") + .ok_or_else(|| ErrorDto::invalid("Invalid cursor: unknown version"))?; + let bytes = URL_SAFE_NO_PAD + .decode(encoded) + .map_err(|err| ErrorDto::invalid(format!("Invalid cursor: {err}")))?; + let token: TicketCursorToken = serde_json::from_slice(&bytes) + .map_err(|err| ErrorDto::invalid(format!("Invalid cursor: {err}")))?; + if token.v != 1 { + return Err(ErrorDto::invalid("Invalid cursor: unknown version")); + } + Ok(token) +} + +fn row_cursor_anchor(row: &IssueIndexEntry, sort: Option) -> TicketCursorAnchor { + TicketCursorAnchor { + number: row.issue_ref.number().get(), + sort_key: row_sort_key(row, sort), + } +} + +fn row_sort_key(row: &IssueIndexEntry, sort: Option) -> TicketCursorSortKey { + match sort.map(|sort| sort.field) { + None | Some(TicketListSortFieldDto::Number) => { + TicketCursorSortKey::Number(row.issue_ref.number().get()) + } + Some(TicketListSortFieldDto::Priority) => { + TicketCursorSortKey::Priority(priority_rank(row.priority)) + } + Some(TicketListSortFieldDto::Status) => { + TicketCursorSortKey::Status(status_rank(row.status)) + } + Some(TicketListSortFieldDto::Title) => TicketCursorSortKey::Title { + lower: row.title.to_lowercase(), + raw: row.title.clone(), + }, + } +} + +fn compare_row_to_anchor( + row: &IssueIndexEntry, + anchor: &TicketCursorAnchor, + sort: Option, +) -> Ordering { + let row_key = row_sort_key(row, sort); + let field_order = compare_sort_key(&row_key, &anchor.sort_key); + let directed = match sort.map(|sort| sort.direction) { + Some(TicketListSortDirectionDto::Desc) => field_order.reverse(), + None | Some(TicketListSortDirectionDto::Asc) => field_order, + }; + directed.then_with(|| row.issue_ref.number().get().cmp(&anchor.number)) +} + +fn compare_sort_key(a: &TicketCursorSortKey, b: &TicketCursorSortKey) -> Ordering { + match (a, b) { + (TicketCursorSortKey::Number(a), TicketCursorSortKey::Number(b)) => a.cmp(b), + (TicketCursorSortKey::Priority(a), TicketCursorSortKey::Priority(b)) + | (TicketCursorSortKey::Status(a), TicketCursorSortKey::Status(b)) => a.cmp(b), + ( + TicketCursorSortKey::Title { lower: al, raw: ar }, + TicketCursorSortKey::Title { lower: bl, raw: br }, + ) => al.cmp(bl).then_with(|| ar.cmp(br)), + _ => Ordering::Equal, } } @@ -1679,7 +1812,7 @@ mod tests { text: None, sort: None, limit: Some(1), - cursor: Some("1".into()), + cursor: None, }) .unwrap(); let rows = vec![ @@ -1693,10 +1826,13 @@ mod tests { vec![IssueStatus::Open, IssueStatus::Qa] ); assert_eq!(page.filter.priorities, vec![IssuePriority::High]); - let out = paginate(rows, page.limit, page.cursor); + let out = paginate(rows, page.limit, page.cursor, page.sort).unwrap(); assert_eq!(out.items.len(), 1); - assert_eq!(out.items[0].r#ref, "#2"); - assert_eq!(out.next_cursor, Some("2".to_owned())); + assert_eq!(out.items[0].r#ref, "#1"); + assert!(out + .next_cursor + .as_deref() + .is_some_and(|cursor| cursor.starts_with("v1."))); } #[test] @@ -1724,7 +1860,7 @@ mod tests { ]; sort_ticket_rows(&mut rows, page.sort); - let out = paginate(rows, page.limit, page.cursor); + let out = paginate(rows, page.limit, page.cursor, page.sort).unwrap(); assert_eq!( out.items @@ -1735,6 +1871,82 @@ mod tests { ); } + #[test] + fn ticket_list_cursor_is_anchor_based_when_items_are_inserted_or_removed_before_anchor() { + let rows = vec![ + issue_row(10, IssueStatus::Open, IssuePriority::High, "Alpha"), + issue_row(20, IssueStatus::Open, IssuePriority::High, "Beta"), + issue_row(30, IssueStatus::Open, IssuePriority::High, "Gamma"), + issue_row(40, IssueStatus::Open, IssuePriority::High, "Delta"), + ]; + let first = paginate(rows, 2, None, None).unwrap(); + let cursor = first.next_cursor.clone().expect("next cursor"); + assert_eq!(refs(&first), vec!["#10", "#20"]); + + let with_insert_before_anchor = vec![ + issue_row(10, IssueStatus::Open, IssuePriority::High, "Alpha"), + issue_row(15, IssueStatus::Open, IssuePriority::High, "Inserted"), + issue_row(20, IssueStatus::Open, IssuePriority::High, "Beta"), + issue_row(30, IssueStatus::Open, IssuePriority::High, "Gamma"), + issue_row(40, IssueStatus::Open, IssuePriority::High, "Delta"), + ]; + let second = paginate(with_insert_before_anchor, 2, Some(cursor.clone()), None).unwrap(); + assert_eq!(refs(&second), vec!["#30", "#40"]); + + let with_removed_before_anchor = vec![ + issue_row(20, IssueStatus::Open, IssuePriority::High, "Beta"), + issue_row(30, IssueStatus::Open, IssuePriority::High, "Gamma"), + issue_row(40, IssueStatus::Open, IssuePriority::High, "Delta"), + ]; + let second = paginate(with_removed_before_anchor, 2, Some(cursor.clone()), None).unwrap(); + assert_eq!(refs(&second), vec!["#30", "#40"]); + + let with_removed_anchor = vec![ + issue_row(10, IssueStatus::Open, IssuePriority::High, "Alpha"), + issue_row(30, IssueStatus::Open, IssuePriority::High, "Gamma"), + issue_row(40, IssueStatus::Open, IssuePriority::High, "Delta"), + ]; + let second = paginate(with_removed_anchor, 2, Some(cursor), None).unwrap(); + assert_eq!(refs(&second), vec!["#30", "#40"]); + } + + #[test] + fn ticket_list_cursor_rejects_legacy_or_invalid_tokens() { + let rows = vec![issue_row( + 1, + IssueStatus::Open, + IssuePriority::High, + "Alpha", + )]; + + for cursor in ["2", "v1.not-base64", "v2.abc"] { + let err = paginate(rows.clone(), 1, Some(cursor.to_owned()), None).unwrap_err(); + assert_eq!(err.code, "INVALID"); + assert!(err.message.contains("Invalid cursor")); + } + } + + #[test] + fn ticket_list_cursor_rejects_sort_mismatch() { + let mut rows = vec![ + issue_row(1, IssueStatus::Open, IssuePriority::Low, "Alpha"), + issue_row(2, IssueStatus::Open, IssuePriority::Critical, "Beta"), + issue_row(3, IssueStatus::Open, IssuePriority::High, "Gamma"), + ]; + let priority_sort = Some(TicketListSortDto { + field: TicketListSortFieldDto::Priority, + direction: TicketListSortDirectionDto::Desc, + }); + sort_ticket_rows(&mut rows, priority_sort); + let first = paginate(rows.clone(), 1, None, priority_sort).unwrap(); + let cursor = first.next_cursor.expect("next cursor"); + + let err = paginate(rows, 1, Some(cursor), None).unwrap_err(); + + assert_eq!(err.code, "INVALID"); + assert!(err.message.contains("sort mismatch")); + } + fn issue_row( number: u64, status: IssueStatus, @@ -1752,4 +1964,8 @@ mod tests { updated_at: number, } } + + fn refs(out: &TicketListDto) -> Vec { + out.items.iter().map(|item| item.r#ref.clone()).collect() + } } diff --git a/crates/infrastructure/src/issues.rs b/crates/infrastructure/src/issues.rs index 28c73f8..9c40cea 100644 --- a/crates/infrastructure/src/issues.rs +++ b/crates/infrastructure/src/issues.rs @@ -254,6 +254,9 @@ fn filter_matches(row: &IssueIndexEntry, filter: &IssueListFilter) -> bool { .map(|s| s.trim()) .filter(|s| !s.is_empty()) { + if let Some(number) = parse_issue_search_ref(text) { + return row.issue_ref.number() == number; + } row.title .to_ascii_lowercase() .contains(&text.to_ascii_lowercase()) @@ -262,6 +265,17 @@ fn filter_matches(row: &IssueIndexEntry, filter: &IssueListFilter) -> bool { } } +fn parse_issue_search_ref(needle: &str) -> Option { + let trimmed = needle.trim(); + let raw = trimmed.strip_prefix('#').unwrap_or(trimmed); + if raw.is_empty() || !raw.chars().all(|ch| ch.is_ascii_digit()) { + return None; + } + raw.parse::() + .ok() + .and_then(|number| IssueNumber::new(number).ok()) +} + #[async_trait] impl IssueStore for FsIssueStore { async fn create(&self, root: &ProjectPath, issue: &Issue) -> Result<(), IssueStoreError> { @@ -309,7 +323,9 @@ impl IssueStore for FsIssueStore { .is_some_and(|text| !text.trim().is_empty()) { // Text search may need description/carnet, so fall back to source files. - let needle = filter.text.as_ref().unwrap().trim().to_ascii_lowercase(); + let raw_needle = filter.text.as_ref().unwrap().trim(); + let ref_needle = parse_issue_search_ref(raw_needle); + let needle = raw_needle.to_ascii_lowercase(); let base_filter = IssueListFilter { text: None, ..filter @@ -319,6 +335,12 @@ impl IssueStore for FsIssueStore { if !filter_matches(&row, &base_filter) { continue; } + if let Some(number) = ref_needle { + if row.issue_ref.number() == number { + out.push(row); + } + continue; + } if row.title.to_ascii_lowercase().contains(&needle) { out.push(row); continue; diff --git a/crates/infrastructure/tests/issue_store.rs b/crates/infrastructure/tests/issue_store.rs index 393d763..6368d6e 100644 --- a/crates/infrastructure/tests/issue_store.rs +++ b/crates/infrastructure/tests/issue_store.rs @@ -184,6 +184,132 @@ async fn issue_store_lists_by_index_filters_with_empty_or_and_and_semantics() { assert_eq!(by_status_and_priority[0].title, "Beta"); } +#[tokio::test] +async fn issue_store_text_filter_matches_exact_ref_or_number_without_numeric_substring() { + let tmp = TempDir::new(); + let root = tmp.root(); + let store = FsIssueStore::new(); + store.create(&root, &issue(&root, 4, "Four")).await.unwrap(); + store + .create(&root, &issue(&root, 40, "Forty")) + .await + .unwrap(); + store + .create(&root, &issue(&root, 42, "Forty two")) + .await + .unwrap(); + + let by_ref = store + .list( + &root, + IssueListFilter { + text: Some("#42".to_owned()), + ..IssueListFilter::default() + }, + ) + .await + .unwrap(); + assert_eq!( + by_ref.iter().map(|row| row.issue_ref).collect::>(), + vec![IssueRef::from_str("#42").unwrap()] + ); + + let by_number = store + .list( + &root, + IssueListFilter { + text: Some("42".to_owned()), + ..IssueListFilter::default() + }, + ) + .await + .unwrap(); + assert_eq!( + by_number + .iter() + .map(|row| row.issue_ref) + .collect::>(), + vec![IssueRef::from_str("#42").unwrap()] + ); + + let by_padded_number = store + .list( + &root, + IssueListFilter { + text: Some("0042".to_owned()), + ..IssueListFilter::default() + }, + ) + .await + .unwrap(); + assert_eq!( + by_padded_number + .iter() + .map(|row| row.issue_ref) + .collect::>(), + vec![IssueRef::from_str("#42").unwrap()] + ); + + let by_single_digit = store + .list( + &root, + IssueListFilter { + text: Some("4".to_owned()), + ..IssueListFilter::default() + }, + ) + .await + .unwrap(); + assert_eq!( + by_single_digit + .iter() + .map(|row| row.issue_ref) + .collect::>(), + vec![IssueRef::from_str("#4").unwrap()] + ); +} + +#[tokio::test] +async fn issue_store_text_filter_keeps_classic_title_description_and_carnet_matching() { + let tmp = TempDir::new(); + let root = tmp.root(); + let store = FsIssueStore::new(); + let by_title = issue(&root, 1, "Needle in title"); + let by_description = issue(&root, 2, "Plain title") + .mutate(IssueActor::System, 2_000, |i| { + i.description = MarkdownDoc::new("Needle in description"); + }) + .unwrap(); + let by_carnet = issue(&root, 3, "Other title") + .mutate(IssueActor::System, 2_000, |i| { + i.carnet = MarkdownDoc::new("Needle in carnet"); + }) + .unwrap(); + store.create(&root, &by_title).await.unwrap(); + store.create(&root, &by_description).await.unwrap(); + store.create(&root, &by_carnet).await.unwrap(); + + let rows = store + .list( + &root, + IssueListFilter { + text: Some("needle".to_owned()), + ..IssueListFilter::default() + }, + ) + .await + .unwrap(); + + assert_eq!( + rows.iter().map(|row| row.issue_ref).collect::>(), + vec![ + IssueRef::from_str("#1").unwrap(), + IssueRef::from_str("#2").unwrap(), + IssueRef::from_str("#3").unwrap() + ] + ); +} + #[tokio::test] async fn issue_store_persists_and_filters_sprint_membership() { let tmp = TempDir::new();