Skip to main content

max / goingson

Opt tasks into field merge, deriving completed_at at every status write tasks is the table field merge is for: 29 synced columns, so two devices editing the same one is the uncommon case, and plain LWW discards a whole edit (and stashes it) for touching a row it never contested. Re-ran the enumeration of every UPDATE tasks site. Three findings, all now recorded at the declaration: - actual_minutes is a counter (time_session_repo.rs increments it in place). Declared, so a conflict that moved it on both sides falls back to LWW-and-stash instead of silently keeping one device's total. - completed_at is derived from status. Declared as a dependent pair, because the app cannot fix this on its own: a device that starts a task leaves completed_at at its base value, so it is never a change to contest, and a column-by-column merge keeps the other device's completion time whichever way status falls. Verified against the real resolver before relying on it. - urgency is derived too and is deliberately NOT grouped. Grouping it would drag most of the row in and leave nothing to merge; it is a sort score rather than a claim about the task, recomputed on the next write, and a remote apply already overwrites it under plain LWW today. delete() and start() now derive completed_at rather than setting status alone, so deleting a completed task no longer leaves it carrying the completion time of a status it lost. Written as a CASE rather than update()'s read-modify-write: the new status is fixed at both sites, so SQL can do the derivation, and one statement has no window for a concurrent complete() to land between a read and a write. The manifest test freezes the opt-in set and both declarations, so adding a table means editing the list deliberately rather than a refactor doing it quietly. Cargo.lock: synckit-client 0.6.0 -> 0.7.0, plus cargo pruning windows-* entries the current graph no longer requires.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-06 20:12 UTC
Signed with PGP, not checked
Commit: e529dd30b3a64c98c57f49dd8d871edadd0c46ff
Parent: c9ec052
4 files changed, +236 insertions, -53 deletions
M Cargo.lock +26 -39
@@ -1336,7 +1336,7 @@
1336 1336 "libc",
1337 1337 "option-ext",
1338 1338 "redox_users",
1339 - "windows-sys 0.61.2",
1339 + "windows-sys 0.59.0",
1340 1340 ]
1341 1341
1342 1342 [[package]]
@@ -1586,7 +1586,7 @@
1586 1586 checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb"
1587 1587 dependencies = [
1588 1588 "libc",
1589 - "windows-sys 0.61.2",
1589 + "windows-sys 0.59.0",
1590 1590 ]
1591 1591
1592 1592 [[package]]
@@ -2621,7 +2621,7 @@
2621 2621 "js-sys",
2622 2622 "log",
2623 2623 "wasm-bindgen",
2624 - "windows-core 0.62.2",
2624 + "windows-core",
2625 2625 ]
2626 2626
2627 2627 [[package]]
@@ -3583,7 +3583,7 @@
3583 3583 "png 0.18.1",
3584 3584 "serde",
3585 3585 "thiserror 2.0.19",
3586 - "windows-sys 0.61.2",
3586 + "windows-sys 0.60.2",
3587 3587 ]
3588 3588
3589 3589 [[package]]
@@ -3716,7 +3716,7 @@
3716 3716 source = "registry+https://github.com/rust-lang/crates.io-index"
3717 3717 checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5"
3718 3718 dependencies = [
3719 - "windows-sys 0.61.2",
3719 + "windows-sys 0.59.0",
3720 3720 ]
3721 3721
3722 3722 [[package]]
@@ -3824,7 +3824,7 @@
3824 3824 source = "registry+https://github.com/rust-lang/crates.io-index"
3825 3825 checksum = "680998035259dcfcafe653688bf2aa6d3e2dc05e98be6ab46afb089dc84f1df8"
3826 3826 dependencies = [
3827 - "proc-macro-crate 3.5.0",
3827 + "proc-macro-crate 1.3.1",
3828 3828 "proc-macro2",
3829 3829 "quote",
3830 3830 "syn 2.0.119",
@@ -4923,7 +4923,7 @@
4923 4923 "pin-project-lite",
4924 4924 "rustls",
4925 4925 "rustls-pki-types",
4926 - "rustls-platform-verifier 0.7.0",
4926 + "rustls-platform-verifier 0.6.2",
4927 4927 "serde",
4928 4928 "serde_json",
4929 4929 "serde_urlencoded",
@@ -5029,7 +5029,7 @@
5029 5029 "errno",
5030 5030 "libc",
5031 5031 "linux-raw-sys",
5032 - "windows-sys 0.61.2",
5032 + "windows-sys 0.59.0",
5033 5033 ]
5034 5034
5035 5035 [[package]]
@@ -5086,7 +5086,7 @@
5086 5086 "security-framework",
5087 5087 "security-framework-sys",
5088 5088 "webpki-root-certs",
5089 - "windows-sys 0.61.2",
5089 + "windows-sys 0.59.0",
5090 5090 ]
5091 5091
5092 5092 [[package]]
@@ -5107,7 +5107,7 @@
5107 5107 "security-framework",
5108 5108 "security-framework-sys",
5109 5109 "webpki-root-certs",
5110 - "windows-sys 0.61.2",
5110 + "windows-sys 0.59.0",
5111 5111 ]
5112 5112
5113 5113 [[package]]
@@ -5641,7 +5641,7 @@
5641 5641 checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4"
5642 5642 dependencies = [
5643 5643 "libc",
5644 - "windows-sys 0.61.2",
5644 + "windows-sys 0.60.2",
5645 5645 ]
5646 5646
5647 5647 [[package]]
@@ -6039,7 +6039,7 @@
6039 6039
6040 6040 [[package]]
6041 6041 name = "synckit-client"
6042 - version = "0.6.0"
6042 + version = "0.7.0"
6043 6043 dependencies = [
6044 6044 "apple-native-keyring-store",
6045 6045 "argon2",
@@ -6161,7 +6161,7 @@
6161 6161 "unicode-segmentation",
6162 6162 "url",
6163 6163 "windows",
6164 - "windows-core 0.61.2",
6164 + "windows-core",
6165 6165 "windows-version",
6166 6166 "x11-dl",
6167 6167 ]
@@ -6612,10 +6612,10 @@
6612 6612 checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd"
6613 6613 dependencies = [
6614 6614 "fastrand",
6615 - "getrandom 0.4.3",
6615 + "getrandom 0.3.4",
6616 6616 "once_cell",
6617 6617 "rustix",
6618 - "windows-sys 0.61.2",
6618 + "windows-sys 0.59.0",
6619 6619 ]
6620 6620
6621 6621 [[package]]
@@ -6637,7 +6637,7 @@
6637 6637 "parking_lot",
6638 6638 "rustix",
6639 6639 "signal-hook",
6640 - "windows-sys 0.61.2",
6640 + "windows-sys 0.60.2",
6641 6641 ]
6642 6642
6643 6643 [[package]]
@@ -7119,7 +7119,7 @@
7119 7119 "png 0.18.1",
7120 7120 "serde",
7121 7121 "thiserror 2.0.19",
7122 - "windows-sys 0.61.2",
7122 + "windows-sys 0.60.2",
7123 7123 ]
7124 7124
7125 7125 [[package]]
@@ -7154,7 +7154,7 @@
7154 7154 dependencies = [
7155 7155 "memoffset",
7156 7156 "tempfile",
7157 - "windows-sys 0.61.2",
7157 + "windows-sys 0.60.2",
7158 7158 ]
7159 7159
7160 7160 [[package]]
@@ -7566,7 +7566,7 @@
7566 7566 "webview2-com-macros",
7567 7567 "webview2-com-sys",
7568 7568 "windows",
7569 - "windows-core 0.61.2",
7569 + "windows-core",
7570 7570 "windows-implement",
7571 7571 "windows-interface",
7572 7572 ]
@@ -7590,7 +7590,7 @@
7590 7590 dependencies = [
7591 7591 "thiserror 2.0.19",
7592 7592 "windows",
7593 - "windows-core 0.61.2",
7593 + "windows-core",
7594 7594 ]
7595 7595
7596 7596 [[package]]
@@ -7693,7 +7693,7 @@
7693 7693 source = "registry+https://github.com/rust-lang/crates.io-index"
7694 7694 checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22"
7695 7695 dependencies = [
7696 - "windows-sys 0.61.2",
7696 + "windows-sys 0.59.0",
7697 7697 ]
7698 7698
7699 7699 [[package]]
@@ -7724,7 +7724,7 @@
7724 7724 checksum = "9babd3a767a4c1aef6900409f85f5d53ce2544ccdfaa86dad48c91782c6d6893"
7725 7725 dependencies = [
7726 7726 "windows-collections",
7727 - "windows-core 0.61.2",
7727 + "windows-core",
7728 7728 "windows-future",
7729 7729 "windows-link 0.1.3",
7730 7730 "windows-numerics",
@@ -7736,7 +7736,7 @@
7736 7736 source = "registry+https://github.com/rust-lang/crates.io-index"
7737 7737 checksum = "3beeceb5e5cfd9eb1d76b381630e82c4241ccd0d27f1a39ed41b2760b255c5e8"
7738 7738 dependencies = [
7739 - "windows-core 0.61.2",
7739 + "windows-core",
7740 7740 ]
7741 7741
7742 7742 [[package]]
@@ -7752,26 +7752,13 @@
7752 7752 "windows-strings 0.4.2",
7753 7753 ]
7754 7754
7755 - [[package]]
7756 - name = "windows-core"
7757 - version = "0.62.2"
7758 - source = "registry+https://github.com/rust-lang/crates.io-index"
7759 - checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb"
7760 - dependencies = [
7761 - "windows-implement",
7762 - "windows-interface",
7763 - "windows-link 0.2.1",
7764 - "windows-result 0.4.1",
7765 - "windows-strings 0.5.1",
7766 - ]
7767 -
7768 7755 [[package]]
7769 7756 name = "windows-future"
7770 7757 version = "0.2.1"
7771 7758 source = "registry+https://github.com/rust-lang/crates.io-index"
7772 7759 checksum = "fc6a41e98427b19fe4b73c550f060b59fa592d7d686537eebf9385621bfbad8e"
7773 7760 dependencies = [
7774 - "windows-core 0.61.2",
7761 + "windows-core",
7775 7762 "windows-link 0.1.3",
7776 7763 "windows-threading",
7777 7764 ]
@@ -7829,7 +7816,7 @@
7829 7816 source = "registry+https://github.com/rust-lang/crates.io-index"
7830 7817 checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1"
7831 7818 dependencies = [
7832 - "windows-core 0.61.2",
7819 + "windows-core",
7833 7820 "windows-link 0.1.3",
7834 7821 ]
7835 7822
@@ -8214,7 +8201,7 @@
8214 8201 "webkit2gtk-sys",
8215 8202 "webview2-com",
8216 8203 "windows",
8217 - "windows-core 0.61.2",
8204 + "windows-core",
8218 8205 "windows-version",
8219 8206 "x11-dl",
8220 8207 ]
@@ -1009,3 +1009,99 @@
1009 1009 ) AS s
1010 1010 WHERE tasks.id = s.id AND tasks.title = '';
1011 1011 ";
1012 +
1013 + /// Deleting a completed task clears its completion time.
1014 + ///
1015 + /// `delete()` used to set `status` alone, so a deleted task kept the
1016 + /// `completed_at` of a status it no longer had. That was harmless on its own, and
1017 + /// stops being harmless once `tasks` field-merges: the pair is declared dependent
1018 + /// in the sync manifest precisely because a `status` and a `completed_at` that
1019 + /// disagree describe a task nobody has.
1020 + #[tokio::test]
1021 + async fn test_delete_clears_completed_at() {
1022 + let pool = common::setup_test_db().await;
1023 + let user_id = common::create_test_user(&pool).await;
1024 + let repo = SqliteTaskRepository::new(pool.clone());
1025 +
1026 + let task = repo
1027 + .create(user_id, NewTask::builder("complete then delete").build())
1028 + .await
1029 + .expect("Failed to create");
1030 + let completed = repo
1031 + .complete(task.id, user_id)
1032 + .await
1033 + .expect("Failed to complete")
1034 + .expect("task should exist");
1035 + assert!(completed.completed_at.is_some(), "premise of the test");
1036 +
1037 + assert!(
1038 + repo.delete(task.id, user_id)
1039 + .await
1040 + .expect("Failed to delete")
1041 + );
1042 +
1043 + let (status, completed_at): (String, Option<String>) =
1044 + sqlx::query_as("SELECT status, completed_at FROM tasks WHERE id = ?")
1045 + .bind(task.id.to_string())
1046 + .fetch_one(&pool)
1047 + .await
1048 + .unwrap();
1049 + assert_eq!(status, "Deleted");
1050 + assert_eq!(
1051 + completed_at, None,
1052 + "a deleted task must not carry the completion time of a status it lost"
1053 + );
1054 + }
1055 +
1056 + /// The other half: deleting a task that was never completed must not invent a
1057 + /// completion time, and must leave the stored value alone.
1058 + #[tokio::test]
1059 + async fn test_delete_leaves_an_uncompleted_task_without_a_completion_time() {
1060 + let pool = common::setup_test_db().await;
1061 + let user_id = common::create_test_user(&pool).await;
1062 + let repo = SqliteTaskRepository::new(pool.clone());
1063 +
1064 + let task = repo
1065 + .create(user_id, NewTask::builder("delete while pending").build())
1066 + .await
1067 + .expect("Failed to create");
1068 + assert!(
1069 + repo.delete(task.id, user_id)
1070 + .await
1071 + .expect("Failed to delete")
1072 + );
1073 +
1074 + let (status, completed_at): (String, Option<String>) =
1075 + sqlx::query_as("SELECT status, completed_at FROM tasks WHERE id = ?")
1076 + .bind(task.id.to_string())
1077 + .fetch_one(&pool)
1078 + .await
1079 + .unwrap();
1080 + assert_eq!(status, "Deleted");
1081 + assert_eq!(completed_at, None);
1082 + }
1083 +
1084 + /// Starting a task leaves no completion time behind. `start()` is guarded to fire
1085 + /// only from `Pending`, so this is the rule stated at the site rather than
1086 + /// inherited from a guard somewhere else.
1087 + #[tokio::test]
1088 + async fn test_start_leaves_no_completion_time() {
1089 + let pool = common::setup_test_db().await;
1090 + let user_id = common::create_test_user(&pool).await;
1091 + let repo = SqliteTaskRepository::new(pool.clone());
1092 +
1093 + let task = repo
1094 + .create(user_id, NewTask::builder("start me").build())
1095 + .await
1096 + .expect("Failed to create");
1097 + assert!(repo.start(task.id, user_id).await.expect("Failed to start"));
1098 +
1099 + let (status, completed_at): (String, Option<String>) =
1100 + sqlx::query_as("SELECT status, completed_at FROM tasks WHERE id = ?")
1101 + .bind(task.id.to_string())
1102 + .fetch_one(&pool)
1103 + .await
1104 + .unwrap();
1105 + assert_eq!(status, "Started");
1106 + assert_eq!(completed_at, None);
1107 + }
@@ -176,7 +176,37 @@
176 176 ],
177 177 )
178 178 .references_unsynced()
179 - .group_scoped("group_id"),
179 + .group_scoped("group_id")
180 + // The one table field merge is really for: 29 synced columns, so two
181 + // devices editing the *same* one is the uncommon case and plain LWW
182 + // discards a whole edit (and stashes it) for touching a row it never
183 + // contested. Opted in after enumerating every `UPDATE tasks` site in the
184 + // repo; the two findings are the two arguments below.
185 + //
186 + // `actual_minutes` is a counter (`SET actual_minutes = actual_minutes + ?`
187 + // in time_session_repo.rs), and a merge works on values: two devices each
188 + // logging 30 minutes give a base of 0 and two absolutes of 30, which it
189 + // cannot tell from one overwrite. Declaring it makes that conflict fall
190 + // back to LWW-and-stash, where the loss is visible.
191 + //
192 + // `completed_at` is derived from `status`, so deciding them separately can
193 + // land a Started task carrying a completion time. Making every write site
194 + // set both does NOT fix that (an unchanged value is not a change to
195 + // contest), so the pair is declared and the engine takes it whole.
196 + //
197 + // `urgency` is derived too (from priority, status, due, tags, created_at)
198 + // and is deliberately NOT grouped. Grouping it would drag most of the row
199 + // in and leave nothing to merge. It is a sort score rather than a claim
200 + // about the task, it is recomputed on the next write, and a remote apply
201 + // already overwrites it with the sender's value under plain LWW, so field
202 + // merge makes it no staler than it is today.
203 + //
204 + // These groups were checked and came out clean, always written together:
205 + // waiting_for_response/waiting_since/expected_response_date,
206 + // scheduled_start/scheduled_duration, is_focus/focus_set_at,
207 + // recurrence/recurrence_rule.
208 + .field_merge(&["actual_minutes"])
209 + .dependent_columns(&[&["status", "completed_at"]]),
180 210 tg(
181 211 "time_sessions",
182 212 &[
@@ -477,6 +507,51 @@
477 507 /// last-write-wins discarded on *this device*, so syncing it would push one
478 508 /// member's rejected values into a shared group log. Local-only is a property
479 509 /// of not being listed here, which makes it a property worth asserting.
510 + /// Field merge is opt-in per table because a table with a counter or a
511 + /// derived column is broken by it. Freezing the set means adding one is a
512 + /// deliberate edit here rather than something a manifest refactor can do
513 + /// quietly, and the enumeration that justifies `tasks` is in the comment at
514 + /// its declaration.
515 + #[test]
516 + fn only_enumerated_tables_opt_into_field_merge() {
517 + const MERGING: &[&str] = &["tasks"];
518 + for table in goingson_schema().tables() {
519 + assert_eq!(
520 + table.merges_fields(),
521 + MERGING.contains(&table.name()),
522 + "{} field-merge opt-in changed; enumerate its UPDATE sites for \
523 + counters and derived columns before adding it here",
524 + table.name()
525 + );
526 + }
527 + }
528 +
529 + /// The two findings from that enumeration. Dropping either declaration leaves
530 + /// a merge that compiles and is silently wrong: a lost half-hour, or a Started
531 + /// task carrying a completion time.
532 + #[test]
533 + fn tasks_declares_its_counter_and_its_derived_pair() {
534 + let schema = goingson_schema();
535 + let tasks = schema
536 + .tables()
537 + .iter()
538 + .find(|t| t.name() == "tasks")
539 + .expect("tasks is in the manifest");
540 +
541 + assert_eq!(
542 + tasks.counters(),
543 + &["actual_minutes"],
544 + "actual_minutes is incremented in time_session_repo.rs; merging it by \
545 + value drops one device's addition"
546 + );
547 + assert_eq!(
548 + tasks.dependent_columns_groups(),
549 + &[&["status", "completed_at"]],
550 + "completed_at is derived from status; merging them separately can land \
551 + a status neither device paired with that timestamp"
552 + );
553 + }
554 +
480 555 #[test]
481 556 fn synckit_bookkeeping_tables_are_not_in_the_manifest() {
482 557 const NEVER_SYNCED: &[&str] = &[
@@ -878,27 +878,52 @@
878 878
879 879 #[tracing::instrument(skip_all)]
880 880 async fn delete(&self, id: TaskId, user_id: UserId) -> Result<bool> {
881 - let result =
882 - sqlx::query("UPDATE tasks SET status = 'Deleted' WHERE id = ? AND user_id = ?")
883 - .bind(id.to_string())
884 - .bind(user_id.to_string())
885 - .execute(&self.pool)
886 - .await
887 - .map_err(CoreError::database)?;
881 + // completed_at follows the status transition, the same rule update()
882 + // spells out at the top of this file: leaving Completed clears it. This
883 + // used to set status alone, so deleting a completed task left it carrying
884 + // the completion time of a status it no longer had.
885 + //
886 + // Expressed as a CASE rather than update()'s read-modify-write because the
887 + // new status is fixed here, so the derivation is a function of the stored
888 + // row and SQL can do it. One statement is also strictly safer than a
889 + // transaction around two: there is no window for a concurrent complete()
890 + // to land between the read and the write.
891 + let result = sqlx::query(
892 + r"
893 + UPDATE tasks
894 + SET status = 'Deleted',
895 + completed_at = CASE WHEN status = 'Completed' THEN NULL ELSE completed_at END
896 + WHERE id = ? AND user_id = ?
897 + ",
898 + )
899 + .bind(id.to_string())
900 + .bind(user_id.to_string())
901 + .execute(&self.pool)
902 + .await
903 + .map_err(CoreError::database)?;
888 904
889 905 Ok(result.rows_affected() > 0)
890 906 }
891 907
892 908 #[tracing::instrument(skip_all)]
893 909 async fn start(&self, id: TaskId, user_id: UserId) -> Result<bool> {
910 + // Same derivation as delete(). The `status = 'Pending'` guard means the
911 + // CASE cannot fire today, since a Pending task is not Completed; it is
912 + // written out anyway so this site states the rule rather than relying on a
913 + // guard elsewhere to make omitting it safe.
894 914 let result = sqlx::query(
895 - "UPDATE tasks SET status = 'Started' WHERE id = ? AND user_id = ? AND status = 'Pending'"
915 + r"
916 + UPDATE tasks
917 + SET status = 'Started',
918 + completed_at = CASE WHEN status = 'Completed' THEN NULL ELSE completed_at END
919 + WHERE id = ? AND user_id = ? AND status = 'Pending'
920 + ",
896 921 )
897 - .bind(id.to_string())
898 - .bind(user_id.to_string())
899 - .execute(&self.pool)
900 - .await
901 - .map_err(CoreError::database)?;
922 + .bind(id.to_string())
923 + .bind(user_id.to_string())
924 + .execute(&self.pool)
925 + .await
926 + .map_err(CoreError::database)?;
902 927
903 928 Ok(result.rows_affected() > 0)
904 929 }