test: full test-suite refactor sweep (#61)

* chore(test): start test-suite refactor sweep

* test(ltl): pin exact multi-obligation residual AST

* test(ltl): table-test finalize Kleene connective combinations

* test(ltl): pin reduce over pending inner for bound, Or, Not

* test(ltl): marshal bounded Always steps/duration/deadline

* test(verifier): cover LTL combinator verdict transitions and within unit panic

* test(verifier): table-test DecodeAction kinds and lastAction field exposure

* test(verifier): assert WithPlatform(ios) reaches the picker host and key pool

* test(verifier): widen weighted-selection assertion to a 5x skew margin

* test(verifier): un-skip ax-find round trip with a committed tree fixture

* test(runner): pin isWDADrop to sidecar reconnect-failed message origin

* test(runner): assert PressKey/Wait trace encoding records kind-specific fields

* test(runner): cover RenderSummary unsupported-verbs surfacing branch

* test(trace): set Hierarchy in round-trip and lock lossy Tree contract

Also add a -race concurrent WriteStep test that asserts N well-formed JSONL lines, catching torn lines if the writer mutex is dropped.

* test(trace): round-trip witnesses/changes/metrics/exceptions, pin step-0 witness

* test(trace): document ViolationsAreGreppable grep contract and lock-free WriteScreenshot

* test(hierarchy): cover invalid-JSON and malformed-bounds parser paths

* test(trace): guard writer mutex via WriteStep/Close race on w.file

* test(replay): drop unfailable assets and devproxy assertions

* test(replay): cache reuses on equal mtime, reparses after append

* test(replay): violation marker falls back to detection step when attributed missing

* test(replay): corrupt meta/trace dirs return 500 with error body

* test(replay): SSE client receives runs.changed after a broadcast

* test(replay): Run coalesces creates, ignores write/chmod, closes subs on cancel

* fix(sidecar): synchronize health fixture writes and exercise healthError

* test(sidecar): cover swipe/longpress/doubletap/erase/presskey/metrics/logs translations

* test(sidecar): cover DoubleTapSelector composition and mid-gesture cancel

* test(sidecar): assert gRPC error status surfaces from action RPC

* fix(chrome): route action methods through runCtx so caller cancellation aborts CDP

* fix(chrome): route hierarchy/screenshot/waitidle/metrics through runCtx

* refactor(ios): extract pure simctl JSON parsers

* refactor(ios): add command-runner seams for EnsureSimulator

* test(ios): table-test simctl parsers and EnsureSimulator seams

* test(sidecarassets): cover placeholder build path

* test(sidecarassets): assert reuse via sentinel bytes not mtime

* test(bundler): cover properties-only spec registration

* refactor(testrun): extract prepareBundleInputs from Execute

* test(testrun): cover prepareBundleInputs aliases and missing-runtime error

* test(testrun): table-test resolveRuntimeSibling search edges

* test(testrun): exact-output tests for progressHandler line format

* fix(cmd): point bundle-check aliases at pkg/spec/src

* test(cmd): smoke-test bundle-check resolves spec aliases

* test(cmd): table-test hier-check parse and FindAll on fixture

* test(cmd): unit-test buildBrowseURL deep-link vs root

* test(cmd): drop flaky TestRun_Doctor that launched real Chromium

* test(cmd): pin pipeline error to bundle resolution on web platform

* test(replay-ui): add bun test script

* ci(replay-ui): run bun test via make web-test target

* ci(replay-ui): point bun cache key at replay-ui/bun.lock

* test(replay-ui): exercise real URL encoding and non-ok throw in getJson

* refactor(replay-ui): extract snapshot flatten/getAtPath into lib module

* test(replay-ui): pin snapshot flatten/getAtPath path round-trip

* refactor(replay-ui): extract action selector/format into lib module

* test(replay-ui): pin action selector parse and row formatting

* refactor(replay-ui): share one statusFor between panels

* refactor(replay-ui): extract run-history derivation into lib module

* test(replay-ui): pin shared statusFor precedence and ordering

* test(replay-ui): pin run-history derivation alignment

* refactor(replay-ui): export clampIndex for testing

* refactor(replay-ui): extract keyboard-nav dispatch into pure module

* refactor(replay-ui): extract metrics formatters into lib module

* test(replay-ui): pin clampIndex step boundaries

* test(replay-ui): pin keyboard-nav ownership and key routing

* test(replay-ui): pin metrics formatters and path gap handling

* refactor(sidecar): expose device-output parsers as internal for testing

* test(sidecar): table-test device-output parsers against malformed input

* test(sidecar): cover logcat parsing year inference and line skipping

* test(sidecar): pin pressKey keycode mapping and unknown-key rejection

* test(sidecar): metrics bundleId falls back to launched app and honors override

* test(sidecar): loosen deadline upper bound to tolerate slow CI scheduling

* test(web-runtime): export selector builders for unit tests

* test(web-runtime): guard sanitize cycle, function, and depth limits

* test(web-runtime): table-test selector builder quoting and escaping

* test(sidecar): collapse scalar-forwarding RPC tests into a table

* test(replay-ui): dedup step/summary fixtures into shared module

* test(ios): collapse pickSimulator point-tests into a table

internal/trace/writer.go

@@ -158,6 +158,9 @@ func (w *Writer) WriteStep(step Step) error {
 	return w.encoder.Encode(step)
 }
 
+// WriteScreenshot is lock-free: each call writes a distinct, uniquely-named
+// file via os.WriteFile and touches no field of Writer, so concurrent calls
+// never contend.
 func (w *Writer) WriteScreenshot(stepIndex int, png []byte) error {
 	return w.writePNG(fmt.Sprintf("step-%05d.png", stepIndex), png)
 }

internal/trace/writer_test.go

@@ -6,8 +6,11 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
+
+	"github.com/priyanshujain/sanderling/internal/hierarchy"
 )
 
 func TestWriteMeta_RoundTrip(t *testing.T) {
@@ -99,6 +102,12 @@ func TestWriteStep_HierarchyAndResidualsRoundTrip(t *testing.T) {
 	writer, _ := NewWriter(directory)
 	defer writer.Close()
 
+	tree, err := hierarchy.Parse(`{"attributes":{"resource-id":"root"},"children":[
+		{"attributes":{"resource-id":"child","text":"hi"},"children":[]}]}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	step := Step{
 		Index:     1,
 		Timestamp: time.Now().UTC(),
@@ -108,6 +117,7 @@ func TestWriteStep_HierarchyAndResidualsRoundTrip(t *testing.T) {
 			ResolvedBounds: &BoundsRecord{X: 10, Y: 20, Width: 100, Height: 50},
 			TapPoint:       &PointRecord{X: 60, Y: 45},
 		},
+		Hierarchy: tree,
 		Residuals: map[string]json.RawMessage{
 			"prop1": json.RawMessage(`{"op":"true"}`),
 		},
@@ -132,6 +142,24 @@ func TestWriteStep_HierarchyAndResidualsRoundTrip(t *testing.T) {
 	if string(got.Residuals["prop1"]) != `{"op":"true"}` {
 		t.Errorf("residuals round-trip wrong: %s", got.Residuals["prop1"])
 	}
+
+	// Intentionally-lossy contract: Tree marshals only Elements (Root and
+	// Node.Children are json:"-"). The flat element list survives; tree
+	// structure does not. Lock both halves so a regression that drops the
+	// element list, or one that silently starts persisting structure the
+	// replay UI would then depend on, is caught.
+	if got.Hierarchy == nil {
+		t.Fatal("hierarchy dropped from trace")
+	}
+	if len(got.Hierarchy.Elements) != 2 {
+		t.Fatalf("hierarchy elements not preserved: got %d", len(got.Hierarchy.Elements))
+	}
+	if got.Hierarchy.Elements[1].Text != "hi" {
+		t.Errorf("element field lost: %+v", got.Hierarchy.Elements[1])
+	}
+	if got.Hierarchy.Root != nil {
+		t.Errorf("Root is json:\"-\" and must decode nil, got %+v", got.Hierarchy.Root)
+	}
 }
 
 func TestWriteStep_OmitsEmptyHierarchyAndResiduals(t *testing.T) {
@@ -182,6 +210,66 @@ func TestWriteStep_AppendsOneJsonLine(t *testing.T) {
 	}
 }
 
+// Bug class: a property that first violates at step 0 carries Witness.Step==0,
+// which omitempty drops from JSON. Decode must still yield Step 0 (the true
+// origin) rather than confusing it with a later step. Also pins that the
+// Witnesses/ExtractorChanges/Metrics/Exceptions sidecars survive the round-trip
+// rather than silently vanishing on decode.
+func TestWriteStep_DiagnosticsRoundTrip(t *testing.T) {
+	directory := t.TempDir()
+	writer, _ := NewWriter(directory)
+	defer writer.Close()
+
+	step := Step{
+		Index: 4,
+		Witnesses: map[string]Witness{
+			"balanceNonNegative": {
+				Reason:  "balance went negative",
+				IsError: true,
+				Step:    0,
+				Extractors: map[string]json.RawMessage{
+					"balance": json.RawMessage(`-5`),
+				},
+			},
+		},
+		ExtractorChanges: map[string]ExtractorChange{
+			"balance": {Prev: json.RawMessage(`10`), Curr: json.RawMessage(`-5`)},
+		},
+		Metrics:    &Metrics{CPUPercent: 12.5, HeapBytes: 4096},
+		Exceptions: []Exception{{Class: "NullPointerException", Message: "boom"}},
+	}
+	if err := writer.WriteStep(step); err != nil {
+		t.Fatal(err)
+	}
+	body, _ := os.ReadFile(filepath.Join(directory, "trace.jsonl"))
+	if strings.Contains(string(body), `"step":0`) {
+		t.Errorf("Witness.Step==0 should be omitted from JSON, got: %s", body)
+	}
+	var got Step
+	if err := json.Unmarshal(body, &got); err != nil {
+		t.Fatalf("bad jsonl: %v\n%s", err, body)
+	}
+	w, ok := got.Witnesses["balanceNonNegative"]
+	if !ok {
+		t.Fatal("step-0 witness vanished on decode")
+	}
+	if w.Step != 0 || w.Reason != "balance went negative" || !w.IsError {
+		t.Errorf("witness round-trip wrong: %+v", w)
+	}
+	if string(w.Extractors["balance"]) != `-5` {
+		t.Errorf("witness extractors lost: %s", w.Extractors["balance"])
+	}
+	if c := got.ExtractorChanges["balance"]; string(c.Prev) != `10` || string(c.Curr) != `-5` {
+		t.Errorf("extractor change round-trip wrong: %+v", c)
+	}
+	if got.Metrics == nil || got.Metrics.CPUPercent != 12.5 || got.Metrics.HeapBytes != 4096 {
+		t.Errorf("metrics round-trip wrong: %+v", got.Metrics)
+	}
+	if len(got.Exceptions) != 1 || got.Exceptions[0].Class != "NullPointerException" {
+		t.Errorf("exceptions round-trip wrong: %+v", got.Exceptions)
+	}
+}
+
 func TestWriteStep_MultipleStepsAppend(t *testing.T) {
 	directory := t.TempDir()
 	writer, err := NewWriter(directory)
@@ -201,6 +289,47 @@ func TestWriteStep_MultipleStepsAppend(t *testing.T) {
 	}
 }
 
+// Bug class: dropping the writer mutex unsynchronizes the w.file field that
+// WriteStep reads and Close nils out. Run under -race with WriteStep racing
+// Close: a missing mutex is a reported data race on w.file, and any step that
+// survives Close must still be a complete, parseable JSONL line.
+func TestWriteStep_RacesCloseSafely(t *testing.T) {
+	directory := t.TempDir()
+	writer, err := NewWriter(directory)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	const n = 50
+	var wg sync.WaitGroup
+	for index := 0; index < n; index++ {
+		wg.Add(1)
+		go func(index int) {
+			defer wg.Done()
+			// Tolerate "writer is closed": the contract under test is that the
+			// w.file access is synchronized, not that every write lands.
+			_ = writer.WriteStep(Step{Index: index, Screen: "s"})
+		}(index)
+	}
+	if err := writer.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+	wg.Wait()
+
+	for _, line := range readLines(t, filepath.Join(directory, "trace.jsonl")) {
+		var got Step
+		if err := json.Unmarshal([]byte(line), &got); err != nil {
+			t.Fatalf("torn JSONL line: %v\n%s", err, line)
+		}
+	}
+}
+
+// Grep contract (intentional substring assertion, not a JSON round-trip):
+// operators and CI scripts locate failing steps by grepping raw trace.jsonl for
+// `"violations":["<prop>"]` without a JSON parser. Bug class: a serialization
+// change (whitespace from indenting, renamed/reordered field, pointer slice)
+// that keeps the Step parseable but breaks that exact on-disk byte shape would
+// silently blind every grep-based tool.
 func TestWriteStep_ViolationsAreGreppable(t *testing.T) {
 	directory := t.TempDir()
 	writer, _ := NewWriter(directory)