fix(store): mongo update_data version-CAS must return retryable Ok(None), not Err

.github/workflows/main.yaml

@@ -1,17 +1,23 @@
 name: CI
 
 # Controls when the workflow will run.
+#
+# The only live job here (integration-tests) builds the trace_machina/nativelink
+# OCI image from the docker-compose Dockerfile and runs run_integration_tests.sh.
+# That is a per-PR OCI build — the same class of heavy work #image-off-pr moved
+# off PRs for image.yaml and #2877 moved off PRs for the nix integration job.
+# This lane fell in the crack between those two, so defer it the same way:
+# nightly cross-check + on every merge to main, not on every PR. Per-PR
+# build/test signal is carried by the Nix "Bazel Dev" lane.
 on:
   push:
     branches: [ main ]
     paths-ignore:
       - '.github/styles/**'
       - 'web/**'
-  pull_request:
-    branches: [ main ]
-    paths-ignore:
-      - '.github/styles/**'
-      - 'web/**'
+  schedule:
+    - cron: '0 7 * * *' # 07:00 UTC daily (off-peak)
+  workflow_dispatch:
 
 permissions: read-all
 

nativelink-store/src/mongo_store.rs

@@ -975,7 +975,7 @@ impl SchedulerStore for ExperimentalMongoStore {
             let result = match self
                 .scheduler_collection
                 .find_one_and_update(filter, update_doc)
-                .upsert(true)
+                .upsert(current_version == 0)
                 .return_document(ReturnDocument::After)
                 .await
             {

nativelink-store/tests/mongo_store_test.rs

@@ -839,7 +839,10 @@ async fn test_scheduler_store_operations() -> Result<(), Error> {
         data.version = version1; // Using old version
         let result = helper.store.update_data(data.clone(), None).await;
 
-        assert!(result.is_err(), "Update with old version should fail");
+        assert!(
+            matches!(result, Ok(None)),
+            "Stale-version update must return retryable Ok(None), got {result:?}"
+        );
         eprintln!("Correctly rejected update with stale version");
     }
 
@@ -1056,3 +1059,50 @@ async fn incomplete_upload_preserves_mongo_value() -> Result<(), Error> {
     }
     Ok(())
 }
+
+#[nativelink_test]
+async fn update_data_stale_version_returns_retryable_none_not_err() -> Result<(), Error> {
+    // N12 regression: ExperimentalMongoStore::update_data used upsert(true), so a
+    // version-CAS miss (a concurrent writer already bumped the version) hit the
+    // unique _id and surfaced as a DuplicateKeyError (Err) instead of the
+    // retryable Ok(None) the scheduler contract (and the Redis backend) require.
+    let helper = TestMongoHelper::new(None).await?;
+
+    let mut data = TestSchedulerData {
+        key: "test:n12_version_cas".to_string(),
+        content: "v1".to_string(),
+        version: 0,
+    };
+
+    // Seed at version 0 -> creates version 1 (genuine first create still upserts).
+    let v1 = helper
+        .store
+        .update_data(data.clone(), None)
+        .await
+        .err_tip(|| "create should succeed")?
+        .ok_or_else(|| make_err!(Code::Internal, "expected a version on create"))?;
+
+    // One writer wins with the correct version -> version 2.
+    data.content = "v2".to_string();
+    data.version = v1;
+    let v2 = helper
+        .store
+        .update_data(data.clone(), None)
+        .await
+        .err_tip(|| "correct-version update should succeed")?
+        .ok_or_else(|| make_err!(Code::Internal, "expected a version on update"))?;
+    assert!(v2 > v1, "version should increment ({v1} -> {v2})");
+
+    // The losing concurrent writer carries the now-stale version. It MUST get a
+    // retryable Ok(None), not Err. Before the fix this returned Err(Internal)
+    // (duplicate _id), destroying the retry signal.
+    data.content = "stale".to_string();
+    data.version = v1;
+    let stale = helper.store.update_data(data.clone(), None).await;
+    assert!(
+        matches!(stale, Ok(None)),
+        "stale-version update must return retryable Ok(None), got {stale:?}"
+    );
+
+    Ok(())
+}