Stabilize rewatch scheduling and integration tests

CHANGELOG.md

@@ -20,6 +20,8 @@
 
 #### :bug: Bug fix
 
+- Make rewatch compile independent modules after an unrelated failure and recompile blocked dependents when a changed interface survives a failed implementation, including across full watcher rebuilds. https://github.com/rescript-lang/rescript/pull/8667
+
 #### :memo: Documentation
 
 #### :nail_care: Polish

rewatch/AGENTS.md

@@ -217,5 +217,5 @@ When clippy suggests refactoring that could impact performance, consider the tra
 ## CI Gotchas
 
 - **`sleep` is fragile** — Prefer polling (e.g., `wait_for_file`) over fixed sleeps. CI runners are slower than local machines.
-- **`exit_watcher` is async** — It only signals the watcher to stop (removes the lock file), it doesn't wait for the process to exit. Avoid triggering config-change events before exiting, as the watcher may start a concurrent rebuild.
+- **Wait for watcher shutdown with `exit_watcher`** — It removes the lock file and waits for the recorded watcher process to exit. Check its return status before continuing when later mutations could race with the watcher.
 - **`sed -i` differs across platforms** — macOS requires `sed -i '' ...`, Linux does not. Use the `replace` / `normalize_paths` helpers from `rewatch/tests/utils.sh` instead of raw `sed`.

rewatch/src/build/compile.rs

@@ -440,7 +440,11 @@ pub fn compile(
     rayon::in_place_scope(|scope| {
         let mut in_flight: usize = 0;
         loop {
-            while in_flight < capacity && !has_errors {
+            // Keep dispatching work that was already independent of any
+            // failed module. Otherwise diagnostics vary with worker count:
+            // a warning-producing module may or may not have started before
+            // an unrelated error finishes.
+            while in_flight < capacity {
                 let Some(work) = ready_heap.pop() else { break };
                 let module_name = work.module_name.clone();
                 let is_dirty = dirty_set.contains(&module_name);
@@ -462,10 +466,6 @@ pub fn compile(
             }
 
             if in_flight == 0 {
-                if !ready_heap.is_empty() {
-                    // Errors suppressed new spawns; nothing left to drain.
-                    break;
-                }
                 if completed.len() < compile_universe_count && !has_errors {
                     stalled = true;
                 }
@@ -475,7 +475,8 @@ pub fn compile(
             let Ok(msg) = rx.recv() else { break };
             in_flight -= 1;
 
-            if msg.result.is_err() || msg.interface_result.as_ref().is_some_and(|r| r.is_err()) {
+            let failed = msg.result.is_err() || msg.interface_result.as_ref().is_some_and(|r| r.is_err());
+            if failed {
                 has_errors = true;
             }
 
@@ -493,16 +494,21 @@ pub fn compile(
                 if !compile_universe.contains(dep) {
                     continue;
                 }
+                // A successful interface can replace the CMI even when its
+                // implementation fails. Keep dependents dirty for recovery,
+                // but do not release them until the module succeeds.
                 if !is_clean {
                     dirty_set.insert(dep.clone());
                 }
-                let count = pending_deps.get_mut(dep).unwrap();
-                *count -= 1;
-                if *count == 0 && !completed.contains(dep) {
-                    ready_heap.push(WorkUnit {
-                        priority: priorities[dep],
-                        module_name: dep.clone(),
-                    });
+                if !failed {
+                    let count = pending_deps.get_mut(dep).unwrap();
+                    *count -= 1;
+                    if *count == 0 && !completed.contains(dep) {
+                        ready_heap.push(WorkUnit {
+                            priority: priorities[dep],
+                            module_name: dep.clone(),
+                        });
+                    }
                 }
             }
         }
@@ -523,9 +529,9 @@ pub fn compile(
     let mut num_compiled_modules = 0;
 
     // Persist propagated dirtiness back onto build_state. Modules that were
-    // marked dirty (because a predecessor's cmi changed) but never scheduled
-    // — e.g. the first compile error aborted further dispatch — must keep
-    // compile_dirty = true so the next incremental build recompiles them.
+    // marked dirty (because a predecessor's cmi changed) but blocked by a
+    // failed prerequisite must keep compile_dirty = true so the next
+    // incremental build recompiles them.
     // Successful recompiles in the result loop below override this back to
     // false for the modules that actually ran.
     for name in &dirty_set {

rewatch/src/watcher.rs

@@ -172,7 +172,7 @@ fn unregister_watches(watcher: &mut RecommendedWatcher, watch_paths: &[(PathBuf,
     }
 }
 
-fn carry_forward_compile_warnings(previous: &BuildCommandState, next: &mut BuildCommandState) {
+fn carry_forward_compile_state(previous: &BuildCommandState, next: &mut BuildCommandState) {
     for (module_name, next_module) in next.build_state.modules.iter_mut() {
         let Some(previous_module) = previous.build_state.modules.get(module_name) else {
             continue;
@@ -184,6 +184,12 @@ fn carry_forward_compile_warnings(previous: &BuildCommandState, next: &mut Build
         match (&previous_module.source_type, &mut next_module.source_type) {
             (SourceType::SourceFile(previous_source), SourceType::SourceFile(next_source)) => {
                 if previous_source.implementation.path == next_source.implementation.path {
+                    // A changed CMI can leave a dependent blocked behind a failed
+                    // implementation. Asset timestamps alone cannot recover this
+                    // dirtiness when a full watcher rebuild recreates the state.
+                    if previous_module.compile_dirty {
+                        next_module.compile_dirty = true;
+                    }
                     next_source.implementation.compile_warnings =
                         previous_source.implementation.compile_warnings.clone();
 
@@ -501,9 +507,8 @@ async fn async_watch(
                     .expect("Could not initialize build");
 
                     // Full rebuilds can be triggered by editor atomic saves that surface as rename events.
-                    // Preserve warning state for unchanged modules so their warnings are re-emitted after the
-                    // fresh build state replaces the previous one.
-                    carry_forward_compile_warnings(&build_state, &mut next_build_state);
+                    // Preserve warnings and blocked dirty modules when fresh state replaces the previous one.
+                    carry_forward_compile_state(&build_state, &mut next_build_state);
                     build_state = next_build_state;
 
                     // Re-register watches based on the new build state
@@ -804,7 +809,7 @@ mod tests {
         );
         let mut next = test_build_state("ModuleA", test_module("src/ModuleA.res", None, None, None));
 
-        carry_forward_compile_warnings(&previous, &mut next);
+        carry_forward_compile_state(&previous, &mut next);
 
         let module = next.get_module("ModuleA").expect("module should exist");
         let SourceType::SourceFile(source_file) = &module.source_type else {
@@ -826,7 +831,7 @@ mod tests {
         );
         let mut next = test_build_state("ModuleA", test_module("src/ModuleARenamed.res", None, None, None));
 
-        carry_forward_compile_warnings(&previous, &mut next);
+        carry_forward_compile_state(&previous, &mut next);
 
         let module = next.get_module("ModuleA").expect("module should exist");
         let SourceType::SourceFile(source_file) = &module.source_type else {
@@ -853,7 +858,7 @@ mod tests {
             test_module("src/ModuleA.res", None, Some("src/ModuleA.resi"), None),
         );
 
-        carry_forward_compile_warnings(&previous, &mut next);
+        carry_forward_compile_state(&previous, &mut next);
 
         let module = next.get_module("ModuleA").expect("module should exist");
         let SourceType::SourceFile(source_file) = &module.source_type else {
@@ -864,4 +869,24 @@ mod tests {
         assert_eq!(interface.compile_warnings.as_deref(), Some("warning: interface"));
         assert_eq!(interface.compile_state, CompileState::Warning);
     }
+
+    #[test]
+    fn carries_forward_blocked_dirtiness_only_for_matching_sources() {
+        let mut previous = test_build_state("ModuleA", test_module("src/ModuleA.res", None, None, None));
+        previous
+            .build_state
+            .modules
+            .get_mut("ModuleA")
+            .unwrap()
+            .compile_dirty = true;
+
+        let mut same_source = test_build_state("ModuleA", test_module("src/ModuleA.res", None, None, None));
+        carry_forward_compile_state(&previous, &mut same_source);
+        assert!(same_source.get_module("ModuleA").unwrap().compile_dirty);
+
+        let mut different_source =
+            test_build_state("ModuleA", test_module("src/Other.res", None, None, None));
+        carry_forward_compile_state(&previous, &mut different_source);
+        assert!(!different_source.get_module("ModuleA").unwrap().compile_dirty);
+    }
 }

rewatch/tests/clean/01-clean-single-project.sh

@@ -42,7 +42,7 @@ other_project_compiled_files=$(find packages/new-namespace -type f -name '*.mjs'
 if [ "$other_project_compiled_files" -gt 0 ];
 then
   success "Didn't clean other project files"
-  git restore .
+  git restore --worktree -- .
 else
   error "Expected files from new-namespace not to be cleaned"
   exit 1

rewatch/tests/clean/02-clean-dev-dependencies.sh

@@ -31,7 +31,7 @@ project_compiled_files=$(find packages/pure-dev -type f -name '*.mjs' | wc -l |
 if [ "$project_compiled_files" -eq 0 ];
 then
   success "pure-dev cleaned"
-  git restore .
+  git restore --worktree -- .
 else
   error "Expected 0 .mjs files in pure-dev after clean, got $project_compiled_files"
   printf "%s\n" "$error_output"

rewatch/tests/clean/03-clean-node-modules.sh

@@ -31,7 +31,7 @@ compiler_assets=$(find node_modules/rescript-nodejs/lib/ocaml -type f -name '*.*
 if [ $compiler_assets -eq 0 ];
 then
   success "compiler assets from node_modules cleaned"
-  git restore .
+  git restore --worktree -- .
 else
   error "Expected 0 files in node_modules/rescript-nodejs/lib/ocaml after clean, got $compiler_assets"
   printf "%s\n" "$error_output"

rewatch/tests/clean/04-clean-rebuild-no-compiler-update.sh

@@ -18,24 +18,25 @@ else
   exit 1
 fi
 
-# Rebuild with snapshot output
-snapshot_file=../tests/snapshots/clean-rebuild.txt
-rewatch build &> $snapshot_file
+# Rebuild with captured output. This test checks one lifecycle message; writing
+# into a tracked snapshot made the fixture dirty whenever work counts changed.
+output_file=$(mktemp "${TMPDIR:-/tmp}/rewatch-clean-rebuild.XXXXXX")
+trap 'rm -f "$output_file"' EXIT
+rewatch build &> "$output_file"
 build_status=$?
-normalize_paths $snapshot_file
 if [ $build_status -eq 0 ];
 then
   success "Repo Built"
 else
   error "Error Building Repo"
-  cat $snapshot_file >&2
+  cat "$output_file" >&2
   exit 1
 fi
 
 # Verify the undesired message is NOT present
-if grep -q "Cleaned previous build due to compiler update" $snapshot_file; then
+if grep -q "Cleaned previous build due to compiler update" "$output_file"; then
   error "Unexpected compiler-update clean message present in rebuild logs"
-  cat $snapshot_file >&2
+  cat "$output_file" >&2
   exit 1
 else
   success "No compiler-update clean message present after explicit clean"

rewatch/tests/compile/08-remove-file.sh

@@ -11,7 +11,7 @@ rewatch build &> /dev/null
 rm packages/dep02/src/Dep02.res
 rewatch build &> ../tests/snapshots/remove-file.txt
 normalize_paths ../tests/snapshots/remove-file.txt
-git checkout -- packages/dep02/src/Dep02.res
+git restore --worktree -- packages/dep02/src/Dep02.res
 
 rewatch build &> /dev/null
 

rewatch/tests/compile/09-dependency-cycle.sh

@@ -11,7 +11,7 @@ rewatch build &> /dev/null
 echo 'Dep01.log()' >> packages/new-namespace/src/NS_alias.res
 rewatch build &> ../tests/snapshots/dependency-cycle.txt
 normalize_paths ../tests/snapshots/dependency-cycle.txt
-git checkout -- packages/new-namespace/src/NS_alias.res
+git restore --worktree -- packages/new-namespace/src/NS_alias.res
 
 rewatch build &> /dev/null
 

rewatch/tests/compile/11-dev-dependency-non-dev-source.sh

@@ -13,7 +13,7 @@ rewatch build &> /dev/null
 echo 'open WebAPI' >> packages/with-dev-deps/src/FileToTest.res
 rewatch build &> ../tests/snapshots/dev-dependency-used-by-non-dev-source.txt
 normalize_paths ../tests/snapshots/dev-dependency-used-by-non-dev-source.txt
-git checkout -- packages/with-dev-deps/src/FileToTest.res
+git restore --worktree -- packages/with-dev-deps/src/FileToTest.res
 
 rewatch build &> /dev/null
 

rewatch/tests/compile/13-no-infinite-loop-with-cycle.sh

@@ -7,7 +7,7 @@ bold "Test: It should not loop when clean building with a cycle"
 
 rewatch clean &> /dev/null
 echo 'Dep01.log()' >> packages/new-namespace/src/NS_alias.res
-git checkout -- packages/new-namespace/src/NS_alias.res
+git restore --worktree -- packages/new-namespace/src/NS_alias.res
 rewatch build &> /dev/null
 
 success "No infinite loop detected"

rewatch/tests/compile/17-prod-flag.sh

@@ -17,7 +17,7 @@ then
   success "Build without --prod correctly failed with nonexistent dev-dependency"
 else
   error "Build without --prod should have failed with nonexistent dev-dependency"
-  git checkout -- rescript.json
+  git restore --worktree -- rescript.json
   exit 1
 fi
 
@@ -27,12 +27,12 @@ then
   success "Build with --prod succeeded despite nonexistent dev-dependency"
 else
   error "Build with --prod should have succeeded by skipping dev-dependencies"
-  git checkout -- rescript.json
+  git restore --worktree -- rescript.json
   exit 1
 fi
 
 rewatch clean &> /dev/null
-git checkout -- rescript.json
+git restore --worktree -- rescript.json
 
 # Test 2: --prod should skip dev source files
 rewatch clean &> /dev/null

rewatch/tests/compile/20-schedule-independent-modules-after-error.sh

@@ -0,0 +1,92 @@
+#!/bin/bash
+
+cd $(dirname $0)
+source "../utils.sh"
+
+bold "Test: Independent modules compile after an unrelated error"
+
+fixture=$(mktemp -d 2>/dev/null || mktemp -d -t rewatch-schedule-after-error)
+trap "rm -rf '$fixture'" EXIT
+
+mkdir -p "$fixture/src"
+
+cat > "$fixture/rescript.json" <<'EOF'
+{
+  "name": "rewatch-schedule-after-error",
+  "sources": { "dir": "src" },
+  "warnings": { "number": "+26+27+32", "error": false },
+  "package-specs": { "module": "esmodule", "in-source": true },
+  "suffix": ".mjs"
+}
+EOF
+
+cat > "$fixture/src/Failing.res" <<'EOF'
+let value = 1
+EOF
+
+cat > "$fixture/src/Blocked.res" <<'EOF'
+let mirrored = Failing.value
+EOF
+
+cat > "$fixture/src/IndependentWarning.res" <<'EOF'
+let unusedValue = 42
+
+let hello = () => Console.log("hello")
+EOF
+
+cd "$fixture"
+if ! RAYON_NUM_THREADS=1 RUST_BACKTRACE=1 "$REWATCH_EXECUTABLE" build > /dev/null 2>&1; then
+  error "Initial fixture build failed"
+  exit 1
+fi
+
+# Make the dependent's generated output an observable signal: the failing build
+# must leave it absent, and the recovery build must recreate it.
+rm -f src/Blocked.mjs
+
+cat > src/Failing.res <<'EOF'
+let value: int = "not an int"
+EOF
+
+cat > src/IndependentWarning.res <<'EOF'
+let unusedValue = 42
+
+let hello = () => {
+  let secondUnusedValue = 43
+  Console.log("hello")
+}
+EOF
+
+compiler_output=$(RAYON_NUM_THREADS=1 RUST_BACKTRACE=1 "$REWATCH_EXECUTABLE" build 2>&1)
+build_status=$?
+
+if [ $build_status -eq 0 ]; then
+  error "Build unexpectedly succeeded despite the type error"
+  printf "%s\n" "$compiler_output" >&2
+  exit 1
+fi
+
+if ! printf '%s\n' "$compiler_output" | grep -q "unused variable secondUnusedValue"; then
+  error "Independent warning was not emitted after the unrelated failure"
+  printf "%s\n" "$compiler_output" >&2
+  exit 1
+fi
+
+if [ -f src/Blocked.mjs ]; then
+  error "Blocked dependent was compiled despite its failed dependency"
+  exit 1
+fi
+
+printf 'let value = "recovered"\n' > src/Failing.res
+if ! compiler_output=$(RAYON_NUM_THREADS=1 RUST_BACKTRACE=1 "$REWATCH_EXECUTABLE" build 2>&1); then
+  error "Build did not recover after fixing the failed dependency"
+  printf "%s\n" "$compiler_output" >&2
+  exit 1
+fi
+
+if [ ! -f src/Blocked.mjs ]; then
+  error "Previously blocked dependent was not compiled after recovery"
+  exit 1
+fi
+
+success "Independent diagnostics and blocked dependents are scheduled correctly"

rewatch/tests/format/01-format-all-files.sh

@@ -11,7 +11,7 @@ git_diff_file_count=$(git diff --name-only ./ | wc -l | xargs)
 if [ $? -eq 0 ] && [ $git_diff_file_count -eq 8 ];
 then
     success "Test package formatted. Got $git_diff_file_count changed files."
-    git restore .
+    git restore --worktree -- .
 else
     error "Error formatting test package"
     echo "Expected 8 files to be changed, got $git_diff_file_count"