fix(mount): converge first empty-revision bootstrap (#362)

* fix(mount): converge first empty revision bootstrap

* fix(mount): localize empty bootstrap exception

* fix(mount): keep empty bootstrap non-destructive

Author: khaliqgant

internal/mountsync/bootstrap_test.go

@@ -366,6 +366,45 @@ func TestBootstrapResumesFromPersistedCursor(t *testing.T) {
 	}
 }
 
+// TestBootstrapConvergesOnFirstUnversionedObservation reproduces #360's
+// empty-revision bootstrap shape. A complete traversal with no remote
+// revision is still a legitimate first observation: it must not be counted as
+// a revision-gate refusal, and the persisted bootstrap must converge.
+func TestBootstrapConvergesOnFirstUnversionedObservation(t *testing.T) {
+	client := newBootstrapClient(12, 12)
+	for path, file := range client.files {
+		file.Revision = ""
+		client.files[path] = file
+	}
+	localDir := t.TempDir()
+	stateFile := filepath.Join(localDir, ".relayfile-mount-state.json")
+	if err := writeMountState(stateFile, mountState{
+		Files:             map[string]trackedFile{},
+		BootstrapComplete: false,
+	}); err != nil {
+		t.Fatalf("seed state: %v", err)
+	}
+
+	s := newBootstrapSyncer(t, client, localDir, SyncerOptions{
+		RootCtx:   context.Background(),
+		StateFile: stateFile,
+	})
+	if err := s.Reconcile(context.Background()); err != nil {
+		t.Fatalf("unversioned bootstrap reconcile: %v", err)
+	}
+
+	st := loadPersistedState(t, localDir)
+	if !st.BootstrapComplete {
+		t.Fatalf("empty-revision full traversal must persist BootstrapComplete")
+	}
+	if st.Counters.SnapshotDeleteBlocked != 0 {
+		t.Fatalf("first empty-revision observation was blocked %d time(s), want 0", st.Counters.SnapshotDeleteBlocked)
+	}
+	if got := countLocalFiles(t, localDir); got != 12 {
+		t.Fatalf("mirrored files = %d, want 12", got)
+	}
+}
+
 // TestBootstrapStallCycleGuardPersistsAndFailsHard reproduces the
 // non-converging unversioned partial-bootstrap shape: the first page is
 // persisted, then every resumed cycle fails at the same next-page cursor.

internal/mountsync/syncer.go

@@ -4784,15 +4784,18 @@ func (s *Syncer) applyRemoteSnapshotDeletes(remotePaths map[string]struct{}, con
 //  1. Refuse the destructive pass entirely when the cloud-error circuit
 //     is open (read-only mirror remains; the next healthy cycle catches
 //     up). Layout materialization is still safe to run.
-//  2. Refuse when the observedRevision does not strictly advance past
+//  2. Treat the first complete bootstrap observation with no revision as a
+//     non-destructive no-op. This lets the caller persist bootstrap completion
+//     without creating or confirming tombstones.
+//  3. Refuse when the observedRevision does not strictly advance past
 //     state.LastAppliedRevision — an older or equal listing must not
 //     authorize deletes.
-//  3. For every tracked path missing from the fresh listing, write or
+//  4. For every tracked path missing from the fresh listing, write or
 //     confirm a tombstone under .relay/pending-deletes. Only the second
 //     consecutive confirmation actually deletes; the first observation
 //     is recorded and skipped.
-//  4. After the pass, prune tombstones for paths that have reappeared.
-//  5. On a clean pass, advance state.LastAppliedRevision.
+//  5. After the pass, prune tombstones for paths that have reappeared.
+//  6. On a clean pass, advance state.LastAppliedRevision.
 func (s *Syncer) applyRemoteSnapshotDeletesRev(remotePaths map[string]struct{}, conflicted map[string]struct{}, observedRevision string) error {
 	if err := s.materializeProviderLayoutsFromPaths(remotePaths); err != nil {
 		return err
@@ -4805,10 +4808,21 @@ func (s *Syncer) applyRemoteSnapshotDeletesRev(remotePaths map[string]struct{},
 		return nil
 	}
 
+	// The first completed bootstrap traversal may legitimately have no
+	// revision. Let the caller record that completion, but do not allow this
+	// unversioned observation to enter tombstone creation or confirmation.
+	// BootstrapComplete is persisted immediately after the successful
+	// traversal, so later unversioned observations fall through to the normal
+	// revision gate and are counted as blocked.
+	if strings.TrimSpace(observedRevision) == "" &&
+		strings.TrimSpace(s.state.LastAppliedRevision) == "" &&
+		!s.state.BootstrapComplete {
+		s.logf("snapshot delete pass skipped: first bootstrap observation has no revision; destructive reconciliation deferred")
+		return nil
+	}
+
 	// Revision gate: refuse to act on a listing that does not strictly
-	// advance the highest-applied revision. revisionAdvances treats an
-	// empty observedRevision as "unknown" — which is also refused. An
-	// empty stored LastAppliedRevision allows the first advancement.
+	// advance the highest-applied revision.
 	if !revisionAdvances(s.state.LastAppliedRevision, observedRevision) {
 		s.state.Counters.SnapshotDeleteBlocked++
 		s.logf("snapshot delete pass refused: observed revision %q does not advance past last applied %q",
@@ -4870,7 +4884,10 @@ func (s *Syncer) applyRemoteSnapshotDeletesRev(remotePaths map[string]struct{},
 // Revisions in this codebase look like "rev_<int>" (see fakeClient) but
 // real cloud revisions may be opaque; we compare numerically when both
 // match the rev_<int> shape, and lexicographically otherwise. An empty
-// observed is never an advancement.
+// observed is never an advancement. Bootstrap callers that need to accept a
+// first unversioned observation must keep that exception local to their
+// persisted bootstrap-completion gate; this generic predicate is also used by
+// strict candidate ordering.
 func revisionAdvances(last, observed string) bool {
 	observed = strings.TrimSpace(observed)
 	last = strings.TrimSpace(last)

internal/mountsync/tombstones_test.go

@@ -156,6 +156,7 @@ func TestRevisionGateRefusesUnversionedListingAcrossRepeatedPulls(t *testing.T)
 			"/keep.md":  {ContentType: "text/markdown", Hash: hashString("# keep")},
 			"/other.md": {ContentType: "text/markdown", Hash: hashString("# other")},
 		},
+		BootstrapComplete: true,
 	}); err != nil {
 		t.Fatalf("seed state: %v", err)
 	}
@@ -195,6 +196,9 @@ func TestRevisionGateRefusesUnversionedListingAcrossRepeatedPulls(t *testing.T)
 
 // TestRevisionAdvances exercises the numeric and lexicographic paths.
 func TestRevisionAdvances(t *testing.T) {
+	if revisionAdvances("", "") {
+		t.Fatalf("generic revision ordering must not treat empty + empty as strictly newer")
+	}
 	if revisionAdvances("rev_5", "") {
 		t.Fatalf("empty observed must never advance")
 	}
@@ -277,3 +281,57 @@ func TestObservePendingDeleteAgedOutReobservesCurrentPass(t *testing.T) {
 		t.Fatalf("unexpected counters after reset: %#v", s.state.Counters)
 	}
 }
+
+func TestFirstUnversionedBootstrapDoesNotConfirmLegacyTombstone(t *testing.T) {
+	const remotePath = "/gone.md"
+	const contents = "# preserve me"
+
+	localDir := t.TempDir()
+	localPath := filepath.Join(localDir, "gone.md")
+	if err := os.WriteFile(localPath, []byte(contents), 0o644); err != nil {
+		t.Fatalf("seed tracked file: %v", err)
+	}
+	s, err := NewSyncer(&fakeClient{}, SyncerOptions{
+		WorkspaceID: "ws_first_unversioned_bootstrap_tombstone",
+		RemoteRoot:  "/",
+		LocalRoot:   localDir,
+	})
+	if err != nil {
+		t.Fatalf("new syncer: %v", err)
+	}
+	s.state.Files[remotePath] = trackedFile{
+		Revision:    "rev_legacy",
+		ContentType: "text/markdown",
+		Hash:        hashString(contents),
+	}
+	s.state.BootstrapComplete = false
+	s.state.LastAppliedRevision = ""
+	seeded := pendingDeleteTombstone{
+		Path:             remotePath,
+		FirstObservedAt:  time.Now().UTC(),
+		LastObservedAt:   time.Now().UTC(),
+		Attempts:         1,
+		ObservedRevision: "rev_legacy",
+	}
+	if err := s.writeTombstone(&seeded); err != nil {
+		t.Fatalf("seed legacy tombstone: %v", err)
+	}
+
+	if err := s.applyRemoteSnapshotDeletesRev(map[string]struct{}{}, nil, ""); err != nil {
+		t.Fatalf("first unversioned bootstrap delete pass: %v", err)
+	}
+
+	if got, err := os.ReadFile(localPath); err != nil || string(got) != contents {
+		t.Fatalf("first unversioned bootstrap changed tracked file: contents=%q err=%v", got, err)
+	}
+	if _, ok := s.state.Files[remotePath]; !ok {
+		t.Fatalf("first unversioned bootstrap removed tracked state")
+	}
+	got, err := s.loadTombstone(remotePath)
+	if err != nil {
+		t.Fatalf("load legacy tombstone: %v", err)
+	}
+	if got == nil || got.Attempts != seeded.Attempts || got.ObservedRevision != seeded.ObservedRevision {
+		t.Fatalf("first unversioned bootstrap mutated legacy tombstone: got=%#v want=%#v", got, seeded)
+	}
+}