AgentWorkforce · khaliqgant · Aug 7, 2026 · Aug 9, 2026 · Aug 9, 2026 · coderabbitai
diff --git a/cmd/relayfile-cli/relay_state_counter_test.go b/cmd/relayfile-cli/relay_state_counter_test.go
@@ -0,0 +1,92 @@
+package main
+
+import (
+	"sync"
+	"testing"
+)
+
+// TestFailedWritebacksSurvivesAlternation checks the specific claim that the
+// failedWritebacks counter is zeroed when the two writers alternate.
+//
+// It is NOT zeroed. Both writers deliberately read the counter back before
+// writing it: mountsync at internal/mountsync/syncer.go:7020
+// (readPublicFailedWritebacks) and the CLI mirror at
+// cmd/relayfile-cli/main.go:10986 (max against the persisted value). This test
+// documents that hardening so a future refactor does not remove it.
+func TestFailedWritebacksSurvivesAlternation(t *testing.T) {
+	localDir := t.TempDir()
+	syncer := newPublicStateWriter(t, localDir)
+
+	for i := 0; i < 5; i++ {
+		if err := incrementFailedWritebacksInState(localDir); err != nil {
+			t.Fatalf("increment %d: %v", i, err)
+		}
+	}
+	if got := readPersistedFailedWritebacks(localDir); got != 5 {
+		t.Fatalf("precondition: counter = %d, want 5", got)
+	}
+
+	// Alternate the two writers several times.
+	for i := 0; i < 3; i++ {
+		writeMountsyncPublicState(t, syncer)
+		if got := readPersistedFailedWritebacks(localDir); got != 5 {
+			t.Errorf("after mountsync write %d: counter = %d, want 5 (mountsync dropped the counter)", i, got)
+		}
+		writeCLIMirrorState(t, localDir)
+		if got := readPersistedFailedWritebacks(localDir); got != 5 {
+			t.Errorf("after CLI mirror write %d: counter = %d, want 5 (CLI mirror dropped the counter)", i, got)
+		}
+	}
+}
+
+// TestFailedWritebacksLostUpdateUnderConcurrency probes the real hazard: both
+// writers preserve the counter via read-modify-write, but neither holds a lock
+// the other respects, so an increment landing between mountsync's read
+// (syncer.go:7020) and its write (syncer.go:7200) is silently overwritten with
+// the stale value.
+//
+// failedWritebacksStateMu (cmd/relayfile-cli/main.go) serializes the CLI-side
+// increments against each other, but mountsync lives in another package and
+// cannot take it. The mutex looks synchronized and is not.
+//
+// N increments must yield a counter of N.
+func TestFailedWritebacksLostUpdateUnderConcurrency(t *testing.T) {
+	const increments = 200
+
+	localDir := t.TempDir()
+	syncer := newPublicStateWriter(t, localDir)
+
+	var wg sync.WaitGroup
+	stop := make(chan struct{})
+
+	// mountsync republishing public state, as the live daemon does ~1x/sec.
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for {
+			select {
+			case <-stop:
+				return
+			default:
+				_ = syncer.FlushOutboxOnce(t.Context())
+			}
+		}
+	}()
+
+	for i := 0; i < increments; i++ {
+		if err := incrementFailedWritebacksInState(localDir); err != nil {
+			t.Fatalf("increment %d: %v", i, err)
+		}
+	}
+
+	close(stop)
+	wg.Wait()
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
-		for {
-			select {
-			case <-stop:
-				return
-			default:
-				_ = syncer.FlushOutboxOnce(t.Context())
-			}
-		}
-	}()
-
-	for i := 0; i < increments; i++ {
-		if err := incrementFailedWritebacksInState(localDir); err != nil {
-			t.Fatalf("increment %d: %v", i, err)
-		}
-	}
-
-	close(stop)
-	wg.Wait()
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for {
+			select {
+			case <-stop:
+				return
+			default:
+				_ = syncer.FlushOutboxOnce(t.Context())
+			}
+		}
+	}()
+	defer func() {
+		close(stop)
+		wg.Wait()
+	}()
+
+	for i := 0; i < increments; i++ {
+		if err := incrementFailedWritebacksInState(localDir); err != nil {
+			t.Fatalf("increment %d: %v", i, err)
+		}
+	}
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
-		for {
-			select {
-			case <-stop:
-				return
-			default:
-				_ = syncer.FlushOutboxOnce(t.Context())
-			}
-		}
-	}()
-
-	for i := 0; i < increments; i++ {
-		if err := incrementFailedWritebacksInState(localDir); err != nil {
-			t.Fatalf("increment %d: %v", i, err)
-		}
-	}
-
-	close(stop)
-	wg.Wait()
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for {
+			select {
+			case <-stop:
+				return
+			default:
+				_ = syncer.FlushOutboxOnce(t.Context())
+			}
+		}
+	}()
+	defer func() {
+		close(stop)
+		wg.Wait()
+	}()
+
+	for i := 0; i < increments; i++ {
+		if err := incrementFailedWritebacksInState(localDir); err != nil {
+			t.Fatalf("increment %d: %v", i, err)
+		}
+	}
+
+	if got := readPersistedFailedWritebacks(localDir); got != increments {
+		t.Errorf("failedWritebacks = %d after %d increments: %d writeback failures were silently lost. "+
+			"failedWritebacksStateMu (cmd/relayfile-cli) does not cross into internal/mountsync, so "+
+			"savePublicState's read at syncer.go:7020 / write at syncer.go:7200 overwrites increments "+
+			"that land in between",
+			got, increments, increments-got)
+	}
+}
diff --git a/cmd/relayfile-cli/relay_state_two_writers_test.go b/cmd/relayfile-cli/relay_state_two_writers_test.go
@@ -0,0 +1,209 @@
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/agentworkforce/relayfile/internal/mountsync"
+)
+
+// newPublicStateWriter builds a real mountsync.Syncer rooted at localDir.
+// Its LocalRoot is the same directory the CLI mirror writer is handed in
+// cmd/relayfile-cli/main.go:6852-6892, so both writers derive the identical
+// <localDir>/.relay/state.json path — that shared derivation is the defect.
+//
+// No network call is made: FlushOutboxOnce with an empty outbox goes straight
+// to markSyncSuccess -> saveStateWithoutLocalScan -> savePublicState.
+func newPublicStateWriter(t *testing.T, localDir string) *mountsync.Syncer {
+	t.Helper()
+	client := mountsync.NewHTTPClient("http://127.0.0.1:1", "test-token", &http.Client{
+		Timeout: time.Second,
+	})
+	websocketDisabled := false
+	syncer, err := mountsync.NewSyncer(client, mountsync.SyncerOptions{
+		WorkspaceID: "rw_twowriters",
+		RemoteRoot:  "/linear",
+		LocalRoot:   localDir,
+		WebSocket:   &websocketDisabled,
+		RootCtx:     context.Background(),
+	})
+	if err != nil {
+		t.Fatalf("NewSyncer: %v", err)
+	}
+	return syncer
+}
+
+// writeMountsyncPublicState drives WRITER 1 (internal/mountsync/syncer.go:7200,
+// path derived at syncer.go:1608).
+func writeMountsyncPublicState(t *testing.T, syncer *mountsync.Syncer) {
+	t.Helper()
+	if err := syncer.FlushOutboxOnce(context.Background()); err != nil {
+		t.Fatalf("FlushOutboxOnce (writer 1): %v", err)
+	}
+}
+
+// writeCLIMirrorState drives WRITER 2 (cmd/relayfile-cli/main.go:10998), with a
+// snapshot shaped like the one buildSyncStateSnapshot produces for a live
+// mount: providers from the cloud feed, a daemon block, guards, a stall reason.
+func writeCLIMirrorState(t *testing.T, localDir string) {
+	t.Helper()
+	snapshot := syncStateFile{
+		WorkspaceID: "rw_twowriters",
+		RemoteRoot:  "/linear",
+		Mode:        defaultMountMode,
+		Status:      "ready",
+		// Provider-feed clock. Deliberately old: this is the field an
+		// operator misread as "the feed moved".
+		LastEventAt: "2026-08-03T07:26:26.334Z",
+		IntervalMs:  30000,
+		Providers: []syncStateProvider{
+			{Provider: "linear", Status: "ready", LastEventAt: "2026-08-03T07:26:26.334Z"},
+		},
+		StallReason: "provider feed frozen",
+		Daemon:      &syncStateDaemon{PID: 63173},
+		Guards: &syncStateGuards{
+			CircuitOpenEvents:   7,
+			TombstonesConfirmed: 3,
+		},
+	}
+	if err := writeMirrorStateFile(localDir, snapshot); err != nil {
+		t.Fatalf("writeMirrorStateFile (writer 2): %v", err)
+	}
+}
+
+func readStateDocument(t *testing.T, localDir string) map[string]any {
+	t.Helper()
+	payload, err := os.ReadFile(filepath.Join(localDir, ".relay", "state.json"))
+	if err != nil {
+		t.Fatalf("read .relay/state.json: %v", err)
+	}
+	document := map[string]any{}
+	if err := json.Unmarshal(payload, &document); err != nil {
+		t.Fatalf("unmarshal .relay/state.json: %v", err)
+	}
+	return document
+}
+
+// TestRelayStateJSONHasExactlyOneWriter is the contract this file exists to
+// pin: <localDir>/.relay/state.json must be a single document with a single
+// owner. Today two writers in the same binary emit two disjoint schemas to
+// that one path, so whichever wrote last defines what every consumer sees.
+//
+// The test drives writer 1 then writer 2 then writer 1 again against one
+// localDir and asserts, after each write, that the document still satisfies
+// BOTH consumer contracts. It fails on HEAD at the first assertion.
+func TestRelayStateJSONHasExactlyOneWriter(t *testing.T) {
+	localDir := t.TempDir()
+	syncer := newPublicStateWriter(t, localDir)
+
+	// --- writer 1 (mountsync) writes first ---------------------------------
+	writeMountsyncPublicState(t, syncer)
+	afterWriter1 := readStateDocument(t, localDir)
+	if _, ok := afterWriter1["localRoot"]; !ok {
+		t.Fatalf("precondition failed: writer 1 did not emit its own document; keys=%v", sortedKeys(afterWriter1))
+	}
+
+	// --- writer 2 (CLI mirror) writes second --------------------------------
+	writeCLIMirrorState(t, localDir)
+	afterWriter2 := readStateDocument(t, localDir)
+
+	// Writer 1's fields must survive writer 2. They do not: writeMirrorStateFile
+	// marshals a syncStateFile from scratch, so every publicState-only field is
+	// dropped from the file on disk.
+	for _, field := range []string{"localRoot", "states", "counters", "files", "outbox", "credExpiresInSecs"} {
+		if _, ok := afterWriter2[field]; !ok {
+			t.Errorf("writer 2 clobbered writer 1: field %q is absent from .relay/state.json after the CLI mirror write (keys now: %v)",
+				field, sortedKeys(afterWriter2))
+		}
+	}
+
+	// --- writer 1 writes again (this is what happens ~1x/second live) -------
+	writeMountsyncPublicState(t, syncer)
+	afterWriter1Again := readStateDocument(t, localDir)
+
+	// Writer 2's fields must survive writer 1. They do not.
+	for _, field := range []string{"providers", "daemon", "guards", "stallReason"} {
+		if _, ok := afterWriter1Again[field]; !ok {
+			t.Errorf("writer 1 clobbered writer 2: field %q is absent from .relay/state.json after the mountsync write (keys now: %v)",
+				field, sortedKeys(afterWriter1Again))
+		}
+	}
+
+	// The production readers in this package must keep working across both
+	// writes. They do not: each reads a field only one writer emits.
+	// readGuardCounters is the subtle one. It does not return nil here — it
+	// falls through to writer 1's `counters`/`circuit` block and returns a
+	// *different* guards document than the one writer 2 persisted. The CLI
+	// surface silently swaps its data source depending on who wrote last.
+	guards := readGuardCounters(localDir)
+	if guards == nil {
+		t.Errorf("readGuardCounters (main.go:10912) returned nil after a mountsync write")
+	} else {
+		if guards.CircuitOpenEvents != 7 {
+			t.Errorf("readGuardCounters (main.go:10912) circuitOpenEvents = %d, want 7 — it silently switched from writer 2's persisted `guards` block to writer 1's `counters` block",
+				guards.CircuitOpenEvents)
+		}
+		if guards.TombstonesConfirmed != 3 {
+			t.Errorf("readGuardCounters (main.go:10912) tombstonesConfirmed = %d, want 3 — writer 2's guard telemetry is gone from the file",
+				guards.TombstonesConfirmed)
+		}
+	}
+	if reason := readPersistedStallReason(localDir); reason != "provider feed frozen" {
+		t.Errorf("readPersistedStallReason (main.go:8556) = %q, want %q — the stall reason is erased every time writer 1 wins the race",
+			reason, "provider feed frozen")
+	}
+	state, err := readWritebackState(localDir)
+	if err != nil {
+		t.Fatalf("readWritebackState: %v", err)
+	}
+	if len(state.Providers) == 0 {
+		t.Errorf("readWritebackState (main.go:5487) sees 0 providers — the TypeScript SDK readiness check (packages/sdk/typescript/src/mount-launcher.ts:355) treats an empty providers list as ready, so this races into a false ready")
+	}
+}
+
+// TestRelayStateJSONLastEventAtHasOneMeaning pins the field that produced the
+// real-world misreport. Both writers emit `lastEventAt`, with different
+// semantics: writer 1 means "mountsync's own event clock" and writer 2 means
+// "the cloud provider feed". A consumer polling this one field gets a value
+// whose meaning changes between reads.
+func TestRelayStateJSONLastEventAtHasOneMeaning(t *testing.T) {
+	localDir := t.TempDir()
+	syncer := newPublicStateWriter(t, localDir)
+
+	writeCLIMirrorState(t, localDir)
+	fromProviderFeed, _ := readStateDocument(t, localDir)["lastEventAt"].(string)
+	if fromProviderFeed != "2026-08-03T07:26:26.334Z" {
+		t.Fatalf("precondition failed: writer 2 lastEventAt = %q", fromProviderFeed)
+	}
+
+	writeMountsyncPublicState(t, syncer)
+	afterMountsync := readStateDocument(t, localDir)
+	fromMountsync, present := afterMountsync["lastEventAt"].(string)
+
+	if !present {
+		t.Errorf("lastEventAt vanished from .relay/state.json after a mountsync write: a consumer polling this field sees the provider-feed timestamp %q disappear and reappear as the mount loop and the CLI mirror alternate",
+			fromProviderFeed)
+	}
+	if present && fromMountsync != fromProviderFeed {
+		t.Errorf("lastEventAt changed meaning without changing name: %q (cloud provider feed, writer 2) then %q (mountsync event clock, writer 1)",
+			fromProviderFeed, fromMountsync)
+	}
+}
+
+func sortedKeys(document map[string]any) []string {
+	keys := make([]string, 0, len(document))
+	for key := range document {
+		keys = append(keys, key)
+	}
+	for i := 1; i < len(keys); i++ {
+		for j := i; j > 0 && keys[j] < keys[j-1]; j-- {
+			keys[j], keys[j-1] = keys[j-1], keys[j]
+		}
+	}
+	return keys
+}
diff --git a/docs/evidence/mount-latency-20260807/CLEANUP.md b/docs/evidence/mount-latency-20260807/CLEANUP.md
@@ -0,0 +1,74 @@
+# Cleanup for the 2026-08-07 mount latency run
+
+Everything this run created is disposable. Nothing here touches the
+pre-existing `.dev-collab-stack/` or `.salvaged-from-minis/` directories,
+their processes, their ports, or the existing sf-mini mounts — those were
+deliberately left alone and must stay that way.
+
+## What this run started
+
+**On the sender host (`khaliqs-macbook-pro`, Tailscale `100.89.219.17`)**
+
+| Thing | Where |
+|---|---|
+| `relayfile-server` | bound to `100.89.219.17:18299` (Tailscale address only, not `0.0.0.0`) |
+| `dev-authd.py serve` (JWKS) | `127.0.0.1:19091`, loopback only |
+| Server state file | `<scratch>/latency-run/state/state.json` — outside the repo |
+| Throwaway RSA private key + minted tokens | `<scratch>/latency-run/keys/` — outside the repo, mode 0600 |
+
+**On the receiver host (`sf-mac-mini`, Tailscale `100.102.30.76`)**
+
+| Thing | Where |
+|---|---|
+| `relayfile-cli mount ws_latency_20260807` | mirror at `~/relayfile-latency-mount-20260807` |
+| `receiver-watch.py` | writing `~/.relayfile-latency-harness/raw/` |
+| `clock-offset.py server` | port `19299` |
+| Deployed harness + receiver token | `~/.relayfile-latency-harness/` |
+
+## Teardown
+
+Receiver:
+
+```sh
+ssh sf-mini '
+  pkill -f "relayfile-cli mount ws_latency_20260807"
+  pkill -f receiver-watch.py
+  pkill -f "clock-offset.py server"
+  rm -rf ~/relayfile-latency-mount-20260807
+  rm -rf ~/.relayfile-latency-harness
+'
+```
+
+Sender:
+
+```sh
+pkill -f "bin/relayfile-server"
+pkill -f "dev-authd.py serve"
+rm -rf <scratch>/latency-run
+```
+
+`pkill -f "relayfile-cli mount ws_latency_20260807"` is deliberately matched on
+the full workspace name. sf-mini also runs unrelated pre-existing mounts
+(`relayfile-dev-collab`, `relay-dev-collab`); a looser pattern would kill them.
+
+## Verifying nothing else was disturbed
+
+```sh
+lsof -nP -iTCP:8299 -sTCP:LISTEN     # dev-collab server port: expected untouched
+ssh sf-mini 'pgrep -fl "relayfile-cli.*dev-collab"'   # pre-existing mounts still up
+git -C <repo> status --short         # .dev-collab-stack/ and .salvaged-from-minis/ still untracked, unmodified
+```
+
+## Credentials
+
+The RSA key and the bearer tokens minted for this run are throwaway, scoped to
+workspace `ws_latency_20260807`, short-lived, and were never written into any
+committed artifact or sent over Relay. Deleting the scratch directory and
+`~/.relayfile-latency-harness` on the receiver destroys them.
+
+Separately, and unrelated to this run: a routine `ps` on sf-mini exposes live
+`RELAY_API_KEY` and agent-token values in broker process argv, because they are
+passed as command-line arguments. Those are pre-existing production credentials,
+readable by any local process, and were reported for rotation. This run
+deliberately passed its own receiver token via the `RELAYFILE_TOKEN` environment
+variable rather than `--token` so as not to add to that exposure.