Introduce opentelemetry in rewatch and setup new test infrastructure #8241

nojaf · 2026-02-05T13:21:03Z

This is a subset of #8231.

I wish to introduce open telemetry in Rewatch and migrate the bash tests to a new test suite.

The new tests would look like:

describe("build", () => {
  it("builds all packages from root", () =>
    runRewatchTest(async ({ cli }) => {
      await cli.build();
    }));
});

Where we capture all telemetry traces produced inside runRewatchTest and extract a deterministic snapshot of this:

// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`build > builds all packages from root 1`] = `
[
  "rewatch.build[working_dir=.]",
  "  initialize_build",
  "    packages.make",
  "      build.load_package_sources[package=@rewatch-test/app]",
  "      build.load_package_sources[package=@rewatch-test/library]",
  "      build.load_package_sources[package=rewatch-test-fixture]",
  "    packages.parse_packages",
  "    clean.cleanup_previous_build",
  "  incremental_build[module_count=3]",
  "    build.parse[dirty_modules=3]",
  "    build.compile",
  "      build.compile_wave[file_count=1]",
  "        build.compile_file[module=Library]",
  "      build.compile_wave[file_count=1]",
  "        build.compile_file[module=App]",
  "      build.compile_wave[file_count=1]",
  "        build.compile_file[module=Root]",
]
`;

With this telemetry we can easily update snapshots and assert all still works as expected.
By using Vitest we also have a proper test runner, so can run individual tests.
All tests also run in a sandbox folder in the /tmp, we they cannot interfere with each other.
And they can run in parallel.

The goal is too replace and remove the existing bash tests.

pkg-pr-new · 2026-02-06T09:42:30Z

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@8241

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@8241

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@8241

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@8241

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@8241

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@8241

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@8241

commit: ab8a10a

Add optional OTLP tracing export to rewatch, controlled by the OTEL_EXPORTER_OTLP_ENDPOINT environment variable. When set, rewatch exports spans via HTTP OTLP; when unset, tracing is a no-op. Instrument key build system functions (initialize_build, incremental_build, compile, parse, clean, format, packages) with tracing spans and attributes such as module counts and package names. Restructure main.rs to support telemetry lifecycle (init/flush/shutdown) and fix show_progress to use >= LevelFilter::Info so -v/-vv don't suppress progress messages. Also print 'Finished compilation' in plain_output mode during watch full rebuilds. Introduce a new Vitest-based test infrastructure in tests/rewatch_tests/ that replaces the bash integration tests. Tests spawn rewatch with an OTLP endpoint pointing to an in-process HTTP receiver, collect spans, and snapshot the resulting span tree for deterministic assertions. Update CI, Makefile, and scripts/test.js to use the new test runner.

When stdin is a pipe (not a TTY), spawn a background thread that monitors for EOF. This allows a parent process (such as the test harness) to signal a graceful shutdown by closing stdin, without relying on signals or lock file removal.

Add mtime and content-hash based deduplication to filter out phantom and duplicate file system events. Normalize event kinds from atomic writes (temp file + rename) so they are treated as content modifications rather than create/remove cycles that trigger unnecessary full rebuilds. This fixes issues on macOS (Create events from atomic writes), Linux (duplicate inotify IN_MODIFY events), and Windows (Remove+Rename sequences from atomic writes).

On Windows, bsc writes CRLF to stdout in text mode. When the original source file uses LF line endings, the formatted output would introduce unwanted CRLF conversions. Detect the original file's line ending style and normalize the formatted output to match.

nojaf · 2026-02-06T10:57:04Z

@rolandpeelen and @jfrolich this is already a good first carve out worth reviewing.

Changes to rewatch are minimal but I need some small fixes for the new test suite.
As I mentioned with passion: I want to get rid of the rewatch bash tests, so I will be fixing what I need in this PR. Will try do have a somewhat clean git history so it is easy to review.

Next steps would be to port more tests, but you can already take a look at the rewatch changes.

Propagate parent span through rayon in build.parse so build.parse_file spans are properly nested under build.parse instead of appearing as orphaned root spans. Enrich build.compile_file span with package, suffix, module_system, and namespace attributes for better observability. Handle invalid config changes gracefully during watch mode: replace .expect() with match to report the error and continue watching, allowing the user to fix the config without restarting the watcher.

Add 7 new fixture packages to cover more configuration dimensions: - commonjs: CommonJS module output with .bs.js suffix - namespaced: namespace package with TestNS - noop-ppx: lightweight cross-platform no-op PPX for testing - with-deps: package depending on rescript-bun for clean tests - with-dev-deps: multi-source dirs with dev dependencies - with-jsx: JSX v4 with @rescript/react - with-ppx: PPX integration using noop-ppx Enhance test helpers: - Normalize CRLF line endings in process output for Windows - Support .bs.js artifacts in sandbox cleanup detection - Add createCli, readFileInSandbox, writeFileInSandbox helpers - Add OTEL config for build.parse_file and enriched compile_file spans - Exclude noop-ppx from biome linting (CommonJS required)

Add tests for core build functionality: - Build from a package subdirectory - No stale artifacts on second build - Filter flag to compile only matching modules Add build error tests: - Parse error reporting with file location - Type error reporting - Errors when a dependency module is deleted - Circular dependency detection

Add module operation tests: - File rename with and without dependents - Duplicate module name detection - Interface file compilation and error cases Add namespace package tests: - Build with namespace flag - Namespace in compiler args - File rename in namespaced package Add dev-dependency tests: - Dev source compiles with dev dependencies - Non-dev source cannot use dev dependencies - Clean removes dev source artifacts

Add build config tests: - Experimental feature flags (valid, invalid key, invalid format) - After-build hook execution (success and failure) - Warning configuration in compiler args - Warn-error CLI override - Deprecated and unknown config field warnings Add module system tests: - CommonJS package with .bs.js suffix - CommonJS in compiler args - Suffix change triggers rebuild - Duplicate package-spec suffix error Add PPX integration tests using lightweight noop-ppx: - PPX build produces output - PPX flags in parser args - PPX flags not in compiler args Add JSX tests: - JSX v4 build with @rescript/react - JSX flags in parser args - JSX preserve flag

Add tests for scoped clean, node_modules dependency cleanup, and verifying no false compiler-update message after clean+rebuild.

Add format tests: - Stdin formatting for .res and .resi - Single file and all-files formatting - Subdirectory-scoped formatting - Check mode (pass and fail cases) Add compiler-args tests: - CWD invariance (same output from root and subdirectory) - Warning flags in both parser and compiler args

Verify that a concurrent build is prevented while watch mode holds the lock file.

Add watch mode tests: - New file creation triggers compilation - Warning persistence across incremental builds - Config change triggers full rebuild - Changes outside source dirs are ignored - Missing source folder does not crash watcher - Invalid config change recovery (watcher keeps running) - File rename removes old artifacts and compiles new file - File deletion removes artifacts

Tracing spans are thread-local, so compile_file spans created inside Rayon's par_iter had no parent connection to the compile_wave span on the main thread. Pass the wave span explicitly via `parent: &wave_span` to establish the correct parent-child relationship.

Copilot

Pull request overview

Introduces OpenTelemetry span instrumentation into the rewatch CLI and replaces the legacy bash-based rewatch integration test suite with a Vitest-based harness that runs in isolated sandboxes and snapshots deterministic OTEL span summaries.

Changes:

Add OTEL tracing spans across rewatch command execution and build pipeline steps (parse/compile/clean/format).
Add new Vitest rewatch integration tests + snapshot infrastructure (sandbox + OTLP HTTP receiver).
Remove legacy rewatch/tests/* bash test suite and the old rewatch/testrepo fixture; update CI/Make targets to use the new runner.

Reviewed changes

Copilot reviewed 228 out of 235 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/rewatch_tests/vitest.config.mjs	Vitest configuration (timeouts, global setup, test discovery).
tests/rewatch_tests/package.json	Adds a dedicated workspace package for rewatch tests (Vitest + protobufjs).
tests/rewatch_tests/globalSetup.mjs	Logs resolved toolchain paths for debugging runs.
tests/rewatch_tests/helpers/bins.mjs	Resolves test toolchain binaries/runtime (installed package vs local dev build).
tests/rewatch_tests/helpers/sandbox.mjs	Creates per-test sandbox in `/tmp` and runs `yarn install` inside it.
tests/rewatch_tests/helpers/process.mjs	CLI wrapper for running `rescript` commands and spawning `watch`.
tests/rewatch_tests/helpers/otel-receiver.mjs	In-process OTLP/HTTP receiver to collect spans for assertions.
tests/rewatch_tests/helpers/test-context.mjs	`runRewatchTest` harness: sandbox + OTEL receiver + snapshot summary builder.
tests/rewatch_tests/tests/watch.test.mjs	Watch-mode integration tests (rebuilds, new files, config changes, etc.).
tests/rewatch_tests/tests/lock.test.mjs	Locking behavior test while `watch` is running.
tests/rewatch_tests/tests/format.test.mjs	`format` command tests (stdin, check mode, scoped formatting).
tests/rewatch_tests/tests/compiler-args.test.mjs	`compiler-args` command tests + cross-platform snapshot normalization.
tests/rewatch_tests/tests/clean.test.mjs	`clean` command tests (scoped clean, node_modules artifacts, rebuild behavior).
tests/rewatch_tests/tests/build.test.mjs	`build` command tests (root build, subdir build, filter behavior, no new artifacts).
tests/rewatch_tests/tests/build-ppx.test.mjs	PPX integration tests.
tests/rewatch_tests/tests/build-namespaces.test.mjs	Namespace package build + rename cleanup tests.
tests/rewatch_tests/tests/build-modules.test.mjs	Module rename/delete/interface/duplicate-name behavior tests.
tests/rewatch_tests/tests/build-module-system.test.mjs	CommonJS + suffix behavior tests.
tests/rewatch_tests/tests/build-jsx.test.mjs	JSX config propagation tests.
tests/rewatch_tests/tests/build-errors.test.mjs	Build error reporting tests (parse/type/dependency/cycle).
tests/rewatch_tests/tests/build-dev-deps.test.mjs	Dev-dependency compilation and enforcement tests.
tests/rewatch_tests/tests/build-config.test.mjs	Experimental features / after-build / warnings / unknown fields tests.
tests/rewatch_tests/tests/snapshots/lock.test.mjs.snap	Snapshot of OTEL span summary for lock test.
tests/rewatch_tests/tests/snapshots/format.test.mjs.snap	Snapshot of OTEL span summary for format tests.
tests/rewatch_tests/tests/snapshots/compiler-args.test.mjs.snap	Snapshots for compiler-args outputs + OTEL spans.
tests/rewatch_tests/tests/snapshots/build.test.mjs.snap	Snapshot of OTEL span summary for build tests.
tests/rewatch_tests/tests/snapshots/build-ppx.test.mjs.snap	Snapshot of OTEL span summary for PPX tests.
tests/rewatch_tests/tests/snapshots/build-namespaces.test.mjs.snap	Snapshot of OTEL span summary for namespace tests.
tests/rewatch_tests/tests/snapshots/build-module-system.test.mjs.snap	Snapshot of OTEL span summary for module system tests.
tests/rewatch_tests/tests/snapshots/build-jsx.test.mjs.snap	Snapshot of OTEL span summary for JSX tests.
tests/rewatch_tests/tests/snapshots/build-errors.test.mjs.snap	Snapshot of OTEL span summary for build error tests.
tests/rewatch_tests/tests/snapshots/build-dev-deps.test.mjs.snap	Snapshot of OTEL span summary for dev-deps tests.
tests/rewatch_tests/fixture/package.json	Defines the fixture workspace used as the sandbox template.
tests/rewatch_tests/fixture/yarn.lock	Locks fixture dependencies (e.g. react, rescript-bun).
tests/rewatch_tests/fixture/.yarnrc.yml	Forces node-modules linker and pins yarnPath for sandbox installs.
tests/rewatch_tests/fixture/.gitignore	Ignores generated artifacts within the fixture.
tests/rewatch_tests/fixture/rescript.json	Root fixture ReScript config.
tests/rewatch_tests/fixture/src/Root.res	Root fixture source entry.
tests/rewatch_tests/fixture/packages/app/rescript.json	Fixture package: app config.
tests/rewatch_tests/fixture/packages/app/package.json	Fixture package: app manifest.
tests/rewatch_tests/fixture/packages/app/src/App.res	Fixture package: app source.
tests/rewatch_tests/fixture/packages/library/rescript.json	Fixture package: library config.
tests/rewatch_tests/fixture/packages/library/package.json	Fixture package: library manifest.
tests/rewatch_tests/fixture/packages/library/src/Library.res	Fixture package: library source.
tests/rewatch_tests/fixture/packages/commonjs/rescript.json	Fixture package: commonjs config.
tests/rewatch_tests/fixture/packages/commonjs/package.json	Fixture package: commonjs manifest.
tests/rewatch_tests/fixture/packages/commonjs/src/CjsModule.res	Fixture package: commonjs source.
tests/rewatch_tests/fixture/packages/namespaced/rescript.json	Fixture package: namespaced config.
tests/rewatch_tests/fixture/packages/namespaced/package.json	Fixture package: namespaced manifest.
tests/rewatch_tests/fixture/packages/namespaced/src/Helper.res	Fixture package: namespaced source.
tests/rewatch_tests/fixture/packages/namespaced/src/Consumer.res	Fixture package: namespaced source.
tests/rewatch_tests/fixture/packages/with-deps/rescript.json	Fixture package: external dep usage config.
tests/rewatch_tests/fixture/packages/with-deps/package.json	Fixture package: external dep usage manifest.
tests/rewatch_tests/fixture/packages/with-deps/src/WithDeps.res	Fixture package: external dep usage source.
tests/rewatch_tests/fixture/packages/with-dev-deps/rescript.json	Fixture package: dev-deps config.
tests/rewatch_tests/fixture/packages/with-dev-deps/package.json	Fixture package: dev-deps manifest.
tests/rewatch_tests/fixture/packages/with-dev-deps/src/Main.res	Fixture package: dev-deps source.
tests/rewatch_tests/fixture/packages/with-dev-deps/test/Test.res	Fixture package: dev-deps test source.
tests/rewatch_tests/fixture/packages/with-jsx/rescript.json	Fixture package: JSX config.
tests/rewatch_tests/fixture/packages/with-jsx/package.json	Fixture package: JSX manifest (react deps).
tests/rewatch_tests/fixture/packages/with-jsx/src/Greeting.res	Fixture package: JSX source.
tests/rewatch_tests/fixture/packages/with-ppx/rescript.json	Fixture package: PPX config.
tests/rewatch_tests/fixture/packages/with-ppx/package.json	Fixture package: PPX manifest.
tests/rewatch_tests/fixture/packages/with-ppx/src/User.res	Fixture package: PPX source.
tests/rewatch_tests/fixture/packages/noop-ppx/package.json	Minimal PPX package used to avoid heavy native PPX deps.
tests/rewatch_tests/fixture/packages/noop-ppx/install.js	Postinstall creates a cross-platform `bin` entry point.
tests/rewatch_tests/fixture/packages/noop-ppx/ppx.js	No-op PPX implementation for tests.
tests/rewatch_tests/fixture/packages/noop-ppx/README.md	Documents why/how the noop PPX works.
tests/rewatch_tests/fixture/packages/noop-ppx/.gitignore	Ignores generated `bin` artifacts.
scripts/test.js	Adds `-rewatch` option to run the new Vitest rewatch tests via yarn workspace.
rewatch/src/telemetry.rs	New OTEL initialization + shutdown/flush guard.
rewatch/src/main.rs	Refactors CLI to return exit codes and adds `tracing::instrument` spans per command.
rewatch/src/lib.rs	Exposes the new `telemetry` module.
rewatch/src/format.rs	Adds tracing spans and normalizes CRLF behavior for Windows formatting output.
rewatch/src/build.rs	Adds tracing spans for initialize/incremental build and error/warning steps.
rewatch/src/build/packages.rs	Adds tracing spans around package discovery + parsing.
rewatch/src/build/parse.rs	Adds parse spans (overall + per file) with selected attrs for snapshots.
rewatch/src/build/compile.rs	Adds compile spans (overall + waves + per file) and js-post-build span.
rewatch/src/build/clean.rs	Adds tracing spans for clean stages.
rewatch/Cargo.toml	Adds tracing + OTEL dependencies; adjusts ctrlc feature flags.
Makefile	Switches `test-rewatch` to run the new Node/Vitest test runner.
.github/workflows/ci.yml	Updates CI to install deps and run the new Vitest-based rewatch integration suite.
package.json	Adds `tests/rewatch_tests` as a Yarn workspace.
biome.json	Excludes `noop-ppx` from Biome checks.
CHANGELOG.md	Documents OTEL support + test migration.
AGENTS.md	Updates developer docs for new Vitest-based rewatch tests + OTEL usage.
rewatch/tests/suite.sh	Removes legacy bash test suite entrypoint.
rewatch/tests/utils.sh	Removes legacy bash helpers.
rewatch/tests/get_bin_paths.js	Removes legacy bin-path helper.
rewatch/tests/lib/rewatch.lock	Removes legacy lockfile artifact.
rewatch/tests/watch/01-watch-recompile.sh	Removes legacy bash watch test.
rewatch/tests/watch/02-watch-warnings-persist.sh	Removes legacy bash watch test.
rewatch/tests/watch/03-watch-new-file.sh	Removes legacy bash watch test.
rewatch/tests/watch/04-watch-config-change.sh	Removes legacy bash watch test.
rewatch/tests/watch/05-watch-ignores-non-source.sh	Removes legacy bash watch test.
rewatch/tests/watch/06-watch-missing-source-folder.sh	Removes legacy bash watch test.
rewatch/tests/lock/01-lock-when-watching.sh	Removes legacy bash lock test.
rewatch/tests/suffix/01-custom-suffix.sh	Removes legacy bash suffix test.
rewatch/tests/format/01-format-all-files.sh	Removes legacy bash format test.
rewatch/tests/format/02-format-single-file.sh	Removes legacy bash format test.
rewatch/tests/format/03-format-stdin.sh	Removes legacy bash format test.
rewatch/tests/format/04-format-current-project.sh	Removes legacy bash format test.
rewatch/tests/clean/01-clean-single-project.sh	Removes legacy bash clean test.
rewatch/tests/clean/02-clean-dev-dependencies.sh	Removes legacy bash clean test.
rewatch/tests/clean/03-clean-node-modules.sh	Removes legacy bash clean test.
rewatch/tests/clean/04-clean-rebuild-no-compiler-update.sh	Removes legacy bash clean test.
rewatch/tests/experimental/01-experimental-features-emit.sh	Removes legacy bash experimental test.
rewatch/tests/experimental/02-experimental-features-parse-error.sh	Removes legacy bash experimental test.
rewatch/tests/experimental/03-watch-invalid-experimental.sh	Removes legacy bash experimental test.
rewatch/tests/experimental-invalid/01-invalid-experimental-key.sh	Removes legacy bash experimental-invalid test.
rewatch/tests/compiler-args/01-compiler-args-cwd-invariant.sh	Removes legacy bash compiler-args test.
rewatch/tests/compiler-args/02-warnings-in-parser-and-compiler.sh	Removes legacy bash compiler-args test.
rewatch/tests/compile/01-basic-compile.sh	Removes legacy bash compile test.
rewatch/tests/compile/02-standalone-package.sh	Removes legacy bash compile test.
rewatch/tests/compile/03-rename-file.sh	Removes legacy bash compile test.
rewatch/tests/compile/04-rename-file-internal-dep.sh	Removes legacy bash compile test.
rewatch/tests/compile/05-rename-file-namespace.sh	Removes legacy bash compile test.
rewatch/tests/compile/06-rename-interface-file.sh	Removes legacy bash compile test.
rewatch/tests/compile/07-rename-file-with-interface.sh	Removes legacy bash compile test.
rewatch/tests/compile/08-remove-file.sh	Removes legacy bash compile test.
rewatch/tests/compile/09-dependency-cycle.sh	Removes legacy bash compile test.
rewatch/tests/compile/10-duplicate-module-name.sh	Removes legacy bash compile test.
rewatch/tests/compile/11-dev-dependency-non-dev-source.sh	Removes legacy bash compile test.
rewatch/tests/compile/12-compile-dev-dependencies.sh	Removes legacy bash compile test.
rewatch/tests/compile/13-no-infinite-loop-with-cycle.sh	Removes legacy bash compile test.
rewatch/tests/compile/14-no-testrepo-changes.sh	Removes legacy bash compile test.
rewatch/tests/compile/15-no-new-files.sh	Removes legacy bash compile test.
rewatch/tests/compile/16-snapshots-unchanged.sh	Removes legacy bash compile test.
rewatch/tests/snapshots/rename-file.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/rename-interface-file.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/rename-file-with-interface.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/rename-file-internal-dep.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/rename-file-internal-dep-namespace.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/remove-file.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/duplicate-module-name.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/dev-dependency-used-by-non-dev-source.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/dependency-cycle.txt	Removes legacy snapshot text file.
rewatch/tests/snapshots/clean-rebuild.txt	Removes legacy snapshot text file.
rewatch/testrepo/**	Removes the old large bash-test fixture repo (replaced by `tests/rewatch_tests/fixture`).
rewatch/rewatch.cmd	Removes legacy Windows wrapper script.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-07T11:20:32Z

rewatch/src/build/compile.rs

+                            let root_config = build_state.get_root_config();
+                            let first_spec = root_config.get_package_specs().into_iter().next();
+                            let suffix = first_spec.as_ref().map(|s| root_config.get_suffix(s)).unwrap_or_default();
+                            let module_system = first_spec.as_ref().map(|s| s.module.as_str()).unwrap_or("esmodule");
+                            let namespace = package.namespace.to_suffix().unwrap_or_default();
+                            let _file_span =
+                                info_span!(parent: &wave_span, "build.compile_file", module = %module_name, package = %package.name, suffix, module_system, namespace).entered();
+


The telemetry attributes for build.compile_file are derived from root_config’s first package-spec, not the package currently being compiled. This can produce incorrect suffix/module_system values when building a workspace that contains mixed module systems or per-package suffix overrides. Prefer deriving these fields from package.config/the package’s resolved specs instead.

Copilot claims the telemetry attributes (suffix, module_system) should come from the per-package config, not the root config. But this is wrong — in ReScript's build system, the root rescript.json's package-specs governs the output format for all packages in the workspace. The actual bsc invocation in compiler_args() (line ~600 of compile.rs) also uses root_config.get_package_specs(). The telemetry span correctly mirrors what the compiler actually does. Changing it to use package.config would make the telemetry less accurate.

Minor nitpick: the span only reports the first package spec (.next()), so if the root has multiple specs (e.g., both ESM and CJS), only one is shown. But that's a telemetry simplification, not a correctness bug.

Copilot · 2026-02-07T11:20:32Z

rewatch/src/main.rs

+                cmd::run(args_after_build)
+            }
+            0


cmd::run(...) doesn’t propagate the subprocess exit status (and run_build always returns 0 on success). If --after-build fails, the CLI still reports success. Consider having cmd::run return a Result/exit code and propagate it here so CI scripts can fail correctly.

Suggested change

cmd::run(args_after_build)

}

0

// Propagate the exit status of the after-build command

cmd::run(args_after_build)

} else {

0

}

cmd::run()returns()— it callscmd.wait().unwrap()but discards theExitStatus. The caller in run_buildalways returns0regardless. So if--after-buildspawns a command that fails, the CLI still exits with success. This is a real bug, though it predates this PR (the old code had the same behavior withstd::process::exit(0)aftercmd::run`). Not a regression introduced by this refactor — the behavior was already broken before.

Copilot · 2026-02-07T11:20:32Z

rewatch/src/main.rs

+            let _lock = match get_lock(&build_args.folder) {
+                Ok(lock) => lock,
+                Err(code) => return code,
            };
+            run_build(build_args, show_progress, plain_output)


BuildArgs.folder is a FolderArg (see cli.rs), but this code passes &build_args.folder to get_lock, which expects a &str. This won’t compile and should use &build_args.folder.folder (and similarly anywhere the folder path is needed).

Rust's deref coercion converts &FolderArg → &str automatically. The as &str cast in Path::new(&build_args.folder as &str) works for the same reason. This code compiles and works fine — these are three wrong comments from not understanding Rust's type system.

Copilot · 2026-02-07T11:20:33Z

rewatch/src/main.rs

+    match build::build(
+        &build_args.filter,
+        Path::new(&build_args.folder as &str),
+        show_progress,
+        build_args.no_timing,
+        true, // create_sourcedirs is now always enabled
+        plain_output,


Path::new(&build_args.folder as &str) is invalid because build_args.folder is a FolderArg, not a String. Use the inner string (e.g. Path::new(build_args.folder.folder.as_str())) to avoid a compile error.

Rust's deref coercion converts &FolderArg → &str automatically. The as &str cast in Path::new(&build_args.folder as &str) works for the same reason. This code compiles and works fine — these are three wrong comments from not understanding Rust's type system.

Copilot · 2026-02-07T11:20:33Z

rewatch/src/main.rs

+            let _lock = match get_lock(&watch_args.folder) {
+                Ok(lock) => lock,
+                Err(code) => return code,
+            };
+            run_watch(watch_args, show_progress, plain_output)


Same issue as build: watch_args.folder is passed to get_lock and watcher::start as if it were a String/&str, but the CLI types use FolderArg. Use the inner folder string (e.g. &watch_args.folder.folder) consistently.

Rust's deref coercion converts &FolderArg → &str automatically. The as &str cast in Path::new(&build_args.folder as &str) works for the same reason. This code compiles and works fine — these are three wrong comments from not understanding Rust's type system.

Copilot · 2026-02-07T11:20:33Z

rewatch/src/main.rs

+    // Only set up custom logger if OTEL is not enabled (OTEL sets up its own subscriber)
+    if !telemetry_guard.otel_enabled {
+        let stdout_logger = env_logger::Builder::new()
+            .format(|buf, record| writeln!(buf, "{}:\n{}", record.level(), record.args()))
+            .filter_level(log_level_filter)
+            .target(env_logger::fmt::Target::Stdout)
+            .build();
+
+        let stderr_logger = env_logger::Builder::new()
+            .format(|buf, record| writeln!(buf, "{}:\n{}", record.level(), record.args()))
+            .filter_level(log_level_filter)
+            .target(env_logger::fmt::Target::Stderr)
+            .build();
+
+        log::set_max_level(log_level_filter);
+        log::set_boxed_logger(Box::new(SplitLogger {
+            stdout: stdout_logger,
+            stderr: stderr_logger,
+        }))
+        .expect("Failed to initialize logger");
+    }


When OTEL is enabled, the code skips installing the log logger entirely, so all existing log::info!/debug!/warn! output becomes a no-op (including warnings in telemetry.rs). Consider bridging log -> tracing (e.g. tracing_log::LogTracer) and/or adding a tracing_subscriber::fmt layer so current logging continues to work alongside OTEL export.

When OTEL_EXPORTER_OTLP_ENDPOINT is set, the code enters the OTEL path in telemetry.rs which installs a tracing subscriber but never installs a log crate logger and never sets up a tracing-log bridge. There are ~40+ log::* calls across the codebase (watcher.rs, packages.rs, compiler_info.rs, helpers.rs, etc.) that all become silent no-ops.

Ironically, telemetry.rs itself uses log::warn! to report OTEL flush/shutdown errors — those warnings are invisible when OTEL is enabled.

That said, this is acceptable for now. OTEL is only used during tests to collect spans for snapshot assertions — nobody is relying on log::* output when OTEL is active. The log::* macros work correctly in production (non-OTEL) mode. A tracing-log bridge would be a nice improvement but isn't blocking.

Copilot · 2026-02-07T11:20:33Z

tests/rewatch_tests/helpers/sandbox.mjs

+  try {
+    execSync("node .yarn/releases/yarn-4.12.0.cjs install", {
+      cwd: realDir,
+      stdio: "pipe",
+    });


execSync defaults to a small maxBuffer (~1MB). yarn install output can exceed this and make tests fail nondeterministically. Prefer spawnSync/execFileSync with stdio: "inherit" (or increase maxBuffer) so installs don’t fail due to output volume.

Copilot warns that yarn install output could exceed execSync's default maxBuffer (~200KB). The test fixture is a small workspace with ~9 packages and minimal dependencies. Actual measured yarn install output is ~615 bytes — 0.3% of the default buffer. This is a generic "best practice" warning with no relevance to this specific use case.

jfrolich · 2026-02-07T19:28:44Z

yarn.lock

  languageName: node
  linkType: hard

+"@rollup/rollup-win32-arm64-msvc@npm:4.57.1":


why do we need rollup?

jfrolich · 2026-02-07T19:30:14Z

tests/rewatch_tests/tests/build-modules.test.mjs

@@ -0,0 +1,126 @@
+import { describe, expect, it } from "vitest";


It would be nice for these tests to be rescript. And perhaps we can use bun test, so we don't need any extra dependencies, we can potentially also use bun for installing packages - it's really fast.

Vitest snapshots work better than bun to my knowledge and experience.

Other maintainers would have pitch forks if we used bun.

jfrolich · 2026-02-07T19:32:54Z

Diffs of files are gone. I feel like this caught bugs - not checking the end result (did the files compile in the exact same way) might make the tests less resilient.

jfrolich · 2026-02-07T19:34:22Z

I get that we are not using bash anymore, but I liked the simplicity of it. With Vitest we add a large test runner to the repo with a lot of dependencies. I'm not sure if we need all of it. Perhaps we can move the repository to use bun for package management while using bun:test for tests, this would make it quite a bit simpler.

If we decide on writing the tests in JavaScript I think it would be nicer to actually use rescript and eat our own dogfood.

jfrolich · 2026-02-07T19:36:36Z

Finally there is probably some overhead to having otel. Did you measure it? I like that it gives us some tracing/insights, but it wouldn't be great if we pay a performance price for it.

Not super familiar, maybe the macro compiles away all the tracing code in release mode! :)

nojaf · 2026-02-07T19:54:17Z

Diffs of files are gone. I feel like this caught bugs - not checking the end result (did the files compile in the exact same way) might make the tests less resilient.

I agree that checking end results is important, but the intent has shifted slightly here.

Previously, snapshots were largely used to capture end results implicitly (stdout, files on disk, etc.), with most intent encoded indirectly. In this refactor, those expectations are moved into explicit assertions in the tests themselves, so the “what must hold” is now much more clearly defined and reviewable.

The new snapshots are intentionally higher‑level: they capture the sequence of telemetry events and the path the system took. That makes them useful for regression detection when behaviour changes, and for understanding how a result was produced rather than just what the final output was.

In practice, this has been a big help both for humans and for LLMs when reasoning about failures, since you can see exactly what happened instead of inferring it from side effects.

(We can still add targeted end‑result assertions where they add signal; they just don’t need to live in snapshots anymore.)

I get that we are not using bash anymore, but I liked the simplicity of it.

The trade‑off here is moving to a more standard setup where it’s easy to:

run a subset of tests
re‑run failing cases quickly
update snapshots intentionally
debug failures locally
No cross platform headaches, I can tell you fixed few Windows related problems in Rewatch 😉

That convenience has already paid off for me while iterating on this, especially as the test surface grows.

Finally there is probably some overhead to having otel.

In practice there isn’t much. If no otel collector is configured, nothing is emitted and the overhead is effectively negligible. It’s designed to be cheap when disabled and only pay for itself when you opt in.

If we decide on writing the tests in JavaScript I think it would be nicer to actually use rescript and eat our own dogfood.

Nothing in this setup prevents moving tests to ReScript later if we decide that’s the right direction. For now, I was aiming to stabilise the testing approach first before layering more on top.

cknitt · 2026-02-10T09:16:25Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd3b38beb5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-02-10T09:23:30Z

rewatch/src/watcher.rs

+                if let Some(mtime) = current_mtime
+                    && last_mtime.get(&path_buf) == Some(&mtime)
+                {


Don't drop file events solely on unchanged mtime

This early return treats identical modified() timestamps as a phantom event, but some filesystems have coarse mtime granularity, so two real edits can share the same timestamp. In that case the watcher skips the event before content/hash checks, the module is never marked dirty, and watch can leave stale JS output until a later edit. The mtime fast-path should not unconditionally continue without a secondary change signal.

Useful? React with 👍 / 👎.

The mtime check is just a fast path to avoid the more expensive file read + hash. Right below it (lines 327–342), there's a content hash check that acts as the secondary signal.

If two real edits somehow share the same mtime (coarse-grained filesystem), that implies the content actually changed — but for that to happen, the second edit would need to produce a different mtime than what's cached (since we update the cache after each processed event). So the mtime guard wouldn't fire.

The only scenario where same-mtime skips an event is when the content is also identical, which is correct behavior — no recompilation needed.

chatgpt-codex-connector · 2026-02-10T09:23:30Z

rewatch/src/watcher.rs

+            let _ = std::io::stdin().read(&mut buf);
+            stdin_closed.store(true, Ordering::Relaxed);


Exit watch only after stdin EOF, not any stdin read

In non-TTY mode this thread sets stdin_closed after a single read call regardless of result, but Read::read returns successfully for normal input bytes too. That means any parent process that writes to stdin (without closing it) will trigger a false shutdown on the next loop and stop watch unexpectedly. The EOF signal should only be set when read returns Ok(0) (and continue reading otherwise).

Useful? React with 👍 / 👎.

Good catch!

The stdin monitor thread set the shutdown flag after a single read call, regardless of the result. If a parent process wrote to stdin without closing it, read returned Ok(1) and falsely triggered a graceful shutdown. Now the thread loops until actual EOF (Ok(0)) or an error before signaling shutdown.

nojaf force-pushed the migrate-rewatch-tests branch 2 times, most recently from 6fc10da to 0ce96cc Compare February 6, 2026 10:45

nojaf added 4 commits February 6, 2026 11:48

Gracefully shutdown watcher via stdin EOF

e632783

When stdin is a pipe (not a TTY), spawn a background thread that monitors for EOF. This allows a parent process (such as the test harness) to signal a graceful shutdown by closing stdin, without relying on signals or lock file removal.

nojaf force-pushed the migrate-rewatch-tests branch from 0ce96cc to 9ba38d7 Compare February 6, 2026 10:48

nojaf added 12 commits February 6, 2026 20:37

Add clean tests

a63d1e5

Add tests for scoped clean, node_modules dependency cleanup, and verifying no false compiler-update message after clean+rebuild.

Add lock test

239fd42

Verify that a concurrent build is prevented while watch mode holds the lock file.

SLEEP NOW IN THE FIRE

e7210b9

Add changelog entry

281064a

nojaf marked this pull request as ready for review February 6, 2026 20:51

nojaf requested a review from Copilot February 7, 2026 11:14

Copilot started reviewing on behalf of nojaf February 7, 2026 11:15 View session

Copilot AI reviewed Feb 7, 2026

View reviewed changes

jfrolich reviewed Feb 7, 2026

View reviewed changes

yarn.lock

languageName: node

linkType: hard

"@rollup/rollup-win32-arm64-msvc@npm:4.57.1":

Copy link

Collaborator

jfrolich Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need rollup?

jfrolich reviewed Feb 7, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Feb 10, 2026

View reviewed changes

		@@ -0,0 +1,126 @@
		import { describe, expect, it } from "vitest";

		let _ = std::io::stdin().read(&mut buf);
		stdin_closed.store(true, Ordering::Relaxed);

Introduce opentelemetry in rewatch and setup new test infrastructure #8241

Are you sure you want to change the base?

Introduce opentelemetry in rewatch and setup new test infrastructure #8241

Conversation

nojaf commented Feb 5, 2026

Uh oh!

pkg-pr-new bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nojaf commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfrolich commented Feb 7, 2026

Uh oh!

jfrolich commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jfrolich commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nojaf commented Feb 7, 2026

Uh oh!

cknitt commented Feb 10, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Feb 10, 2026

Choose a reason for hiding this comment

pkg-pr-new bot commented Feb 6, 2026 •

edited

Loading

jfrolich commented Feb 7, 2026 •

edited

Loading

jfrolich commented Feb 7, 2026 •

edited

Loading