fix(benchmark): harden target monitoring

Closes #3637

## Summary

- Add a benchmark target monitor that starts at the measured benchmark
window and verifies the target before any Bencher payload files are
written.
- Preserve the pre-measurement target log to
`bench_results/target_startup_before_benchmark.log`, then blank the live
log so the existing workflow grep cannot match pre-benchmark `died
UNEXPECTEDLY` lines.
- Treat the node-renderer master's `died UNEXPECTEDLY` log line as an
explicit Pro worker restart signal and fail before uploading partial
metrics.

## Tests

- `bundle exec rspec benchmarks/spec/benchmark_target_monitor_spec.rb`
- `bundle exec rspec benchmarks/spec`
- `bundle exec rubocop benchmarks/bench.rb
benchmarks/bench-node-renderer.rb benchmarks/lib/benchmark_helpers.rb
benchmarks/lib/benchmark_target_monitor.rb
benchmarks/spec/benchmark_target_monitor_spec.rb`
- `bundle exec rubocop` (baseline fails on unrelated
`react_on_rails/spike/3313_prism_gemfile_rewriter/*` offenses;
changed-file RuboCop and hooks are clean)
- `/Users/justin/.agents/skills/autoreview/scripts/autoreview --mode
local`
- pre-commit and pre-push hooks reran changed-file RuboCop with no
offenses

## Design Calls

- Kept the implementation in benchmark scripts/tests and did not edit CI
workflow YAML.
- Used log windowing inside the benchmark scripts so #3579's existing
post-run workflow grep now sees only measured-run log content.
- Preserved successful-startup/pre-measure logs as an uploaded benchmark
artifact; startup crashes that happen before benchmark scripts run still
use the existing workflow log dump because improving that path further
would require workflow YAML changes.

Labels: benchmark, ci-tooling

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Changes are confined to benchmark scripts and CI env vars already in
use; they tighten when metrics are written without touching app runtime
or auth.
> 
> **Overview**
> Benchmark runs now **gate Bencher uploads** on target health, not only
on green Vegeta/k6 results.
> 
> A new **`BenchmarkTargetMonitor`** starts at the measured window (from
`TARGET_PID` / `TARGET_LOG`), copies pre-run logs to
`target_startup_before_benchmark.log`, overwrites the live log with
newlines so startup `died UNEXPECTEDLY` lines cannot pollute post-run
checks, then **verifies** the target PID is still alive and that no
unexpected Pro node-renderer worker restart was logged during the run.
**`write_benchmark_payload`** runs that verification before writing
BMF/display JSON; **`bench.rb`** and **`bench-node-renderer.rb`** call
the monitor and exit with a GitHub Actions `::error::` if monitoring
fails after an otherwise successful suite.
> 
> Specs cover log windowing, PID validation, failure ordering (no
payload write on monitor failure), and integration with the shared
helper.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
caf53a3. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Benchmarks now run a pre/post measurement monitor that preserves
startup logs, blanks live logs during measurement, verifies the target
process and log integrity, and prevents writing results if the monitor
detects failures (emits an error and exits).

* **Tests**
* Added comprehensive tests for log preservation,
verification-before-write ordering, payload-write gating, and multiple
failure scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: justin808

benchmarks/bench-node-renderer.rb

@@ -351,6 +351,8 @@ def run_vegeta_suite(test_cases, bundle, label, bmf_collector, runner: method(:r
 
   # Create output directory
   FileUtils.mkdir_p(OUTDIR)
+  target_monitor = BenchmarkTargetMonitor.from_env(output_dir: OUTDIR)
+  target_monitor.start_measurement!
 
   # Initialize BMF collector for Bencher output
   bmf_collector = BmfCollector.new(prefix: BMF_PREFIX)
@@ -366,15 +368,17 @@ def run_vegeta_suite(test_cases, bundle, label, bmf_collector, runner: method(:r
   # Display summary
   display_summary(SUMMARY_TXT)
 
-  # Write the Bencher payload only on a fully green run. Guarding the write here
-  # (rather than writing unconditionally before exit) makes the "never upload a
-  # partial-success payload" invariant self-enforcing instead of relying on the
-  # downstream Bencher step having no `if: always()`.
+  # Write the Bencher payload only on a fully green run against a healthy target.
+  # Guarding the write here makes the "never upload partial metrics" invariant
+  # self-enforcing instead of relying on later workflow steps to discard data.
   if failed_tests.empty?
-    # Append to existing Pro results.
-    bmf_collector.write_bmf_json(BENCHMARK_JSON, append: true)
-    # Display sidecar (summary table data) — append to match the BMF.
-    bmf_collector.write_display_json(DISPLAY_JSON, append: true)
+    begin
+      # Append to existing Pro results and display sidecar rows.
+      write_benchmark_payload(bmf_collector, target_monitor: target_monitor, append: true)
+    rescue BenchmarkTargetMonitor::MonitorFailure => e
+      $stdout.puts "::error::#{e.message}"
+      exit 1
+    end
   else
     $stdout.puts "::error::#{failed_tests.length} node renderer benchmark(s) failed: #{failed_tests.join(', ')}"
     exit 1

benchmarks/bench.rb

@@ -190,6 +190,8 @@ def run_benchmark_suite(routes, bmf_collector, runner: method(:benchmark_route))
   puts "Server is ready!"
 
   FileUtils.mkdir_p(OUTDIR)
+  target_monitor = BenchmarkTargetMonitor.from_env(output_dir: OUTDIR)
+  target_monitor.start_measurement!
 
   # Initialize BMF collector for Bencher output (suffix used for Core/Pro distinction)
   bmf_collector = BmfCollector.new(suffix: BMF_SUFFIX)
@@ -203,14 +205,16 @@ def run_benchmark_suite(routes, bmf_collector, runner: method(:benchmark_route))
   puts "\nSummary saved to #{SUMMARY_TXT}"
   system("column", "-t", "-s", "\t", SUMMARY_TXT)
 
-  # Write the Bencher payload only on a fully green run. Guarding the write here
-  # (rather than writing unconditionally before exit) makes the "never upload a
-  # partial-success payload" invariant self-enforcing instead of relying on the
-  # downstream Bencher step having no `if: always()`.
+  # Write the Bencher payload only on a fully green run against a healthy target.
+  # Guarding the write here makes the "never upload partial metrics" invariant
+  # self-enforcing instead of relying on later workflow steps to discard data.
   if failed_routes.empty?
-    bmf_collector.write_bmf_json(BENCHMARK_JSON)
-    # Display sidecar (summary table data) — written alongside the BMF.
-    bmf_collector.write_display_json(DISPLAY_JSON)
+    begin
+      write_benchmark_payload(bmf_collector, target_monitor: target_monitor)
+    rescue BenchmarkTargetMonitor::MonitorFailure => e
+      $stdout.puts "::error::#{e.message}"
+      exit 1
+    end
   else
     $stdout.puts "::error::#{failed_routes.length} of #{routes.length} benchmark route(s) failed: " \
                  "#{failed_routes.join(', ')}"

benchmarks/lib/benchmark_helpers.rb

@@ -4,6 +4,7 @@
 require "fileutils"
 require "net/http"
 require "uri"
+require_relative "benchmark_target_monitor"
 
 # Shared utilities for benchmark scripts
 # Note: env_or_default and validation helpers are in benchmark_config.rb
@@ -100,3 +101,10 @@ def display_summary(summary_file)
   puts "\nSummary saved to #{summary_file}"
   system("column", "-t", "-s", "\t", summary_file)
 end
+
+# Requires BENCHMARK_JSON and DISPLAY_JSON constants from benchmark_config.rb.
+def write_benchmark_payload(bmf_collector, target_monitor:, append: false)
+  target_monitor.verify_after_measurement!
+  bmf_collector.write_bmf_json(BENCHMARK_JSON, append: append)
+  bmf_collector.write_display_json(DISPLAY_JSON, append: append)
+end

benchmarks/lib/benchmark_target_monitor.rb

@@ -0,0 +1,140 @@
+# frozen_string_literal: true
+
+require "fileutils"
+
+# Guards benchmark payload writes against target process failures that happen
+# after the benchmark script starts measuring but before Bencher receives data.
+class BenchmarkTargetMonitor
+  class MonitorFailure < StandardError; end
+
+  STARTUP_LOG_FILENAME = "target_startup_before_benchmark.log"
+  UNEXPECTED_WORKER_RESTART = "died UNEXPECTEDLY"
+  BLANK_CHUNK_SIZE = 8 * 1024 # balances allocation cost against write-call count
+  BLANK_CHUNK = ("\n" * BLANK_CHUNK_SIZE).freeze
+
+  def self.from_env(output_dir:, env: ENV)
+    new(
+      target_pid: env["TARGET_PID"],
+      target_log: env["TARGET_LOG"],
+      output_dir: output_dir
+    )
+  end
+
+  def self.pid_alive?(pid)
+    # Signal 0 checks process existence without delivering a signal. Zombies can
+    # still answer as alive; CI parents reap quickly enough for this guard.
+    Process.kill(0, pid)
+    true
+  rescue Errno::EPERM
+    true
+  rescue Errno::ESRCH
+    false
+  end
+
+  def initialize(target_pid: nil, target_log: nil, output_dir: nil, pid_alive: nil)
+    @target_pid = present_string(target_pid)
+    @target_log = present_string(target_log)
+    @output_dir = output_dir
+    @pid_alive = pid_alive || ->(pid) { self.class.pid_alive?(pid) }
+    @measurement_started = false
+    @log_windowed = false
+  end
+
+  def start_measurement!
+    return if @measurement_started
+
+    @measurement_started = true
+    @log_windowed = target_log?
+    if @target_log && !@log_windowed
+      warn "BenchmarkTargetMonitor: TARGET_LOG=#{@target_log.inspect} set but log windowing disabled " \
+           "(file does not exist or output_dir is nil); worker-restart checks will be skipped."
+    end
+    preserve_and_blank_target_log if @log_windowed
+  end
+
+  def verify_after_measurement!
+    unless @measurement_started
+      raise MonitorFailure, "start_measurement! was never called; discarding benchmark metrics."
+    end
+
+    verify_target_pid!
+    verify_unexpected_worker_log!
+    true
+  end
+
+  private
+
+  def present_string(value)
+    string_value = value.to_s
+    string_value.empty? ? nil : string_value
+  end
+
+  def target_log?
+    @target_log && @output_dir && File.file?(@target_log)
+  end
+
+  def preserve_and_blank_target_log
+    FileUtils.mkdir_p(@output_dir)
+    File.binwrite(File.join(@output_dir, STARTUP_LOG_FILENAME), File.binread(@target_log))
+    blank_existing_log_bytes
+  end
+
+  # Truncating a file that a running process already has open can leave sparse NUL
+  # gaps when that process writes again at its old file offset. Overwriting the
+  # startup bytes with newlines keeps the existing workflow grep text-only while
+  # preventing pre-measurement events from matching the post-run scan.
+  def blank_existing_log_bytes
+    File.open(@target_log, "r+b") do |file|
+      # Capture size after opening, so bytes appended before the open are blanked.
+      # Bytes appended after this point belong to the measured window.
+      remaining = file.size
+      until remaining.zero?
+        chunk_size = [remaining, BLANK_CHUNK_SIZE].min
+        chunk = chunk_size == BLANK_CHUNK_SIZE ? BLANK_CHUNK : BLANK_CHUNK.byteslice(0, chunk_size)
+        file.write(chunk)
+        remaining -= chunk_size
+      end
+    end
+  end
+
+  def verify_target_pid!
+    return unless @target_pid
+
+    pid = Integer(@target_pid)
+    unless pid.positive?
+      raise MonitorFailure, "TARGET_PID=#{@target_pid.inspect} is not a positive integer; discarding " \
+                            "benchmark metrics because target liveness could not be verified."
+    end
+
+    return if @pid_alive.call(pid)
+
+    raise MonitorFailure, "Benchmark target PID #{pid} exited during the measured benchmark window; " \
+                          "discarding benchmark metrics because the target process did not survive."
+  rescue ArgumentError
+    raise MonitorFailure, "TARGET_PID=#{@target_pid.inspect} is not an integer; discarding benchmark metrics " \
+                          "because target liveness could not be verified."
+  end
+
+  def verify_unexpected_worker_log!
+    return unless @log_windowed
+
+    matches = unexpected_worker_restart_lines
+    return if matches.empty?
+
+    raise MonitorFailure, "Pro node-renderer worker restarted during the measured benchmark window. " \
+                          "The node-renderer master logged `#{UNEXPECTED_WORKER_RESTART}`, which means " \
+                          "a worker process died and was forked again; discarding benchmark metrics " \
+                          "instead of uploading partial data. Matching log line(s):\n#{matches.join("\n")}"
+  end
+
+  def unexpected_worker_restart_lines
+    File.readlines(@target_log, chomp: true).each_with_index.filter_map do |line, index|
+      next unless line.include?(UNEXPECTED_WORKER_RESTART)
+
+      "#{index + 1}:#{line}"
+    end
+  rescue Errno::ENOENT
+    raise MonitorFailure, "Target log #{@target_log.inspect} disappeared before post-measurement scan; " \
+                          "discarding benchmark metrics."
+  end
+end