From 31212150934b7326c53858254b38b6eebb46401b Mon Sep 17 00:00:00 2001
From: "Dustin J. Mitchell" <dustin@v.igoro.us>
Date: Mon, 10 Jun 2024 17:23:18 -0400
Subject: [PATCH 1/2] Fix `get_child_version` returning versions not reachable
 from latest

When a previous sync attempt has uploaded a new child version of
`latest`, but not updated `latest`, it sure looks like that version
is a child of `latest` -- but `get_child_version` should never return a
version not reachable from latest.

This includes a regression test for this scenario.
---
 taskchampion/src/server/cloud/server.rs | 35 ++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/taskchampion/src/server/cloud/server.rs b/taskchampion/src/server/cloud/server.rs
index 6aaee585b..0192b1b83 100644
--- a/taskchampion/src/server/cloud/server.rs
+++ b/taskchampion/src/server/cloud/server.rs
@@ -392,14 +392,13 @@ impl<SVC: Service> Server for CloudServer<SVC> {
     }
 
     fn get_child_version(&mut self, parent_version_id: VersionId) -> Result<GetVersionResult> {
-        // The `get_child_versions` function will usually return only one child version for a
-        // parent, in which case the work is easy. Otherwise, if there are several possible
-        // children, only one of those will lead to `latest`, and importantly the others will not
-        // have their own children. So we can detect the "true" child as the one that is equal to
-        // "latest" or has children.
+        // The `get_child_versions` function may return several possible children, only one of
+        // those will lead to `latest`, and importantly the others will not have their own
+        // children. So we can detect the "true" child as the one that is equal to "latest" or has
+        // children. Note that even if `get_child_versions` returns a single version, that version
+        // may not be valid and the appropriate result may be NoSuchVersion.
         let version_id = match &(self.get_child_versions(&parent_version_id)?)[..] {
             [] => return Ok(GetVersionResult::NoSuchVersion),
-            [child] => *child,
             children => {
                 // There are some extra version objects, so a cleanup is warranted.
                 self.cleanup_probability = 255;
@@ -934,6 +933,7 @@ mod tests {
         let mut server = make_server();
         let (v1, v2) = (Uuid::new_v4(), Uuid::new_v4());
         server.mock_add_version(v2, v1, 1000, b"first");
+        server.mock_set_latest(v1);
         assert_eq!(
             server.get_child_version(v1).unwrap(),
             GetVersionResult::NoSuchVersion
@@ -948,6 +948,29 @@ mod tests {
         );
     }
 
+    #[test]
+    fn get_child_version_single_invalid() {
+        // This is a regression test for #387.
+        let mut server = make_server();
+        let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4());
+        // Here v2 is latest, so v1 is not a valid child of v2.
+        server.mock_add_version(v2, v1, 1000, b"first");
+        server.mock_add_version(v3, v2, 1000, b"second");
+        server.mock_set_latest(v2);
+        assert_eq!(
+            server.get_child_version(v3).unwrap(),
+            GetVersionResult::Version {
+                version_id: v2,
+                parent_version_id: v3,
+                history_segment: b"second".to_vec(),
+            }
+        );
+        assert_eq!(
+            server.get_child_version(v2).unwrap(),
+            GetVersionResult::NoSuchVersion
+        );
+    }
+
     #[test]
     fn get_child_version_multiple() {
         let mut server = make_server();

From f5b20a45f90be050eec9d6d874f45731c93ad0c1 Mon Sep 17 00:00:00 2001
From: "Dustin J. Mitchell" <dustin@v.igoro.us>
Date: Tue, 11 Jun 2024 23:41:32 -0400
Subject: [PATCH 2/2] better test ordering

---
 taskchampion/src/server/cloud/server.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/taskchampion/src/server/cloud/server.rs b/taskchampion/src/server/cloud/server.rs
index 0192b1b83..5d5ab6874 100644
--- a/taskchampion/src/server/cloud/server.rs
+++ b/taskchampion/src/server/cloud/server.rs
@@ -953,15 +953,15 @@ mod tests {
         // This is a regression test for #387.
         let mut server = make_server();
         let (v1, v2, v3) = (Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4());
-        // Here v2 is latest, so v1 is not a valid child of v2.
-        server.mock_add_version(v2, v1, 1000, b"first");
-        server.mock_add_version(v3, v2, 1000, b"second");
+        // Here v2 is latest, so v3 is not a valid child of v2.
+        server.mock_add_version(v1, v2, 1000, b"second");
+        server.mock_add_version(v2, v3, 1000, b"third");
         server.mock_set_latest(v2);
         assert_eq!(
-            server.get_child_version(v3).unwrap(),
+            server.get_child_version(v1).unwrap(),
             GetVersionResult::Version {
                 version_id: v2,
-                parent_version_id: v3,
+                parent_version_id: v1,
                 history_segment: b"second".to_vec(),
             }
         );