fix: address Codex review on PR #3200

application/state/useVaultState.ts

@@ -1922,12 +1922,12 @@ export const useVaultState = () => {
     (payload: Partial<ExportableVaultData>): Promise<void> => {
       const encryptedWrites: Promise<void>[] = [];
       if (payload.hosts) {
-        // Cloud payloads no longer carry `lastConnectedAt` (#2629), but any
-        // remote reconciliation replaces the whole local host array. Re-attach
-        // each matching local host's device-local timestamp so applying a
-        // remote snippet/settings change cannot clear this device's recent-
-        // hosts list. Timestamps present on the incoming hosts (local backups,
-        // legacy cloud snapshots) still win as before.
+        // `lastConnectedAt` is device-local (#2629), but any remote
+        // reconciliation replaces the whole local host array. Re-attach each
+        // matching local host's device-local timestamp so applying a remote
+        // snippet/settings change cannot clear this device's recent-hosts
+        // list, and ignore timestamps carried by the incoming payload (legacy
+        // cloud snapshots, other-device telemetry) — local recency wins.
         encryptedWrites.push(
           updateHosts(
             retainLocalHostLastConnectedAt(payload.hosts, hostsRef.current) ?? payload.hosts,

application/syncPayload.test.ts

@@ -225,6 +225,52 @@ test("retainLocalHostLastConnectedAt does not invent timestamps for unknown host
   assert.equal(retained?.[0]?.lastConnectedAt, undefined);
 });
 
+test("retainLocalHostLastConnectedAt prefers local recency over legacy incoming telemetry", () => {
+  const host = {
+    id: "host-keep",
+    label: "prod",
+    hostname: "prod.example.com",
+    username: "root",
+    tags: [],
+    os: "linux" as const,
+    protocol: "ssh" as const,
+  };
+  const local = [{ ...host, lastConnectedAt: 100 }];
+  const incoming = [{ ...host, label: "prod-renamed", lastConnectedAt: 900 }];
+
+  const retained = retainLocalHostLastConnectedAt(incoming, local);
+
+  assert.equal(retained?.[0]?.lastConnectedAt, 100);
+  assert.equal(retained?.[0]?.label, "prod-renamed");
+});
+
+test("retainLocalHostLastConnectedAt drops incoming telemetry for hosts unknown locally", () => {
+  const incoming = [{
+    id: "cloud-host",
+    label: "cloud",
+    hostname: "cloud.example.com",
+    username: "root",
+    tags: [],
+    os: "linux" as const,
+    protocol: "ssh" as const,
+    lastConnectedAt: 42,
+  }];
+  const local = [{
+    id: "local-host",
+    label: "local",
+    hostname: "local.example.com",
+    username: "root",
+    tags: [],
+    os: "linux" as const,
+    protocol: "ssh" as const,
+    lastConnectedAt: 7,
+  }];
+
+  const retained = retainLocalHostLastConnectedAt(incoming, local);
+
+  assert.equal(retained?.[0]?.lastConnectedAt, undefined);
+});
+
 test("buildSyncPayload includes reusable proxy profiles", () => {
   const proxyProfiles = [
     {

domain/convergentSync/payload.ts

@@ -1,4 +1,6 @@
 import {
+  isHostLocalOnlyField,
+  stripHostLocalOnlyFields,
   withHostsSanitizedForSync,
   type CloudSyncPayloadEntityKey,
   type SyncFileMeta,
@@ -218,7 +220,12 @@ export function materializeSyncPayloadFromConvergentState(
     ? materialized.settings as unknown as NonNullable<SyncPayload['settings']>
     : undefined;
   return {
-    hosts: typedCollection<import('../models').Host>(materialized.collections, 'hosts'),
+    // Replicas created before #2629 can still hold a device-local telemetry
+    // register (e.g. `lastConnectedAt`) on hosts. It must never reach a
+    // materialized cloud snapshot or v2 envelope, so strip it on the way out.
+    hosts: stripHostLocalOnlyFields(
+      typedCollection<import('../models').Host>(materialized.collections, 'hosts'),
+    ) ?? [],
     keys: typedCollection<import('../models').SSHKey>(materialized.collections, 'keys'),
     identities: typedCollection<import('../models').Identity>(materialized.collections, 'identities'),
     proxyProfiles: typedCollection<import('../models').ProxyProfile>(materialized.collections, 'proxyProfiles'),
@@ -345,6 +352,7 @@ export function createConvergentSyncEnvelope(
       const materialized = materializedEntity(materializedPayload, collectionName, id);
       const fields = createEmptyRecord<ConvergentEnvelopeRegister>();
       for (const [field, register] of Object.entries(entity.fields)) {
+        if (collectionName === 'hosts' && isHostLocalOnlyField(field)) continue;
         setOwnRecordValue(fields, field, compactRegister(register, materialized?.[field], true));
       }
       setOwnRecordValue(entities, id, {
@@ -522,7 +530,10 @@ export function hydrateConvergentSyncEnvelope(
   };
   assertValidConvergentSyncState(state);
   const canonical = canonicalizeConvergentSyncState(state);
-  assertMaterializedPayloadMatchesState(canonical, materializedPayload);
+  // Legacy snapshots can still carry host telemetry their registers were
+  // seeded from; materialization strips it, so compare against a sanitized
+  // snapshot instead of failing the invariant on pre-#2629 replicas.
+  assertMaterializedPayloadMatchesState(canonical, withHostsSanitizedForSync(materializedPayload));
   return canonical;
 }
 

domain/convergentSync/payloadMigration.test.ts

@@ -20,6 +20,7 @@ import {
   withConvergentSyncEnvelope,
   createConvergentSyncState,
 } from './index.ts';
+import type { JsonObject, ConvergentSyncStateV2 } from './types.ts';
 
 const NOW = 1_700_000_000_000;
 
@@ -288,6 +289,72 @@ test('diffLegacySyncPayload ignores lastConnectedAt-only host changes', () => {
   assert.deepEqual(diffLegacySyncPayload(baseline, legacy), []);
 });
 
+/** Build a state as a pre-#2629 writer would: register telemetry without sanitization. */
+function legacyStateWithHostTelemetry(): ConvergentSyncStateV2 {
+  return applyConvergentMutations(
+    createConvergentSyncState(),
+    'legacy-device',
+    [{
+      kind: 'entity-upsert',
+      collection: 'hosts',
+      entityId: 'host-1',
+      value: { ...payload().hosts[0], lastConnectedAt: NOW } as unknown as JsonObject,
+      position: 0,
+    }],
+    NOW,
+  );
+}
+
+test('materializeSyncPayloadFromConvergentState strips telemetry from legacy replicas', () => {
+  const state = legacyStateWithHostTelemetry();
+
+  const materialized = materializeSyncPayloadFromConvergentState(state, { syncedAt: NOW });
+
+  assert.equal(materialized.hosts[0]?.lastConnectedAt, undefined);
+  assert.equal('lastConnectedAt' in (materialized.hosts[0] ?? {}), false);
+});
+
+test('v2 envelopes from legacy replicas do not carry telemetry registers', () => {
+  const state = legacyStateWithHostTelemetry();
+  const materialized = materializeSyncPayloadFromConvergentState(state, { syncedAt: NOW });
+
+  const envelope = createConvergentSyncEnvelope(state, materialized);
+  const hostFields = envelope.state.collections['hosts']!.entities['host-1']!.fields;
+
+  assert.equal('lastConnectedAt' in hostFields, false);
+});
+
+test('legacy telemetry registers never reach materialized payloads or new envelopes', () => {
+  const state = legacyStateWithHostTelemetry();
+  const envelope = { ...withConvergentSyncEnvelope(state, { syncedAt: NOW }).convergentSync! };
+  // Simulate a pre-#2629 replica: the envelope and adjacent snapshot still
+  // carry the telemetry register the older writer stored. Reuse the register
+  // from raw CRDT mutations so its dots stay causally consistent.
+  const telemetryRegister = state
+    .collections['hosts']!.entities['host-1']!.fields['lastConnectedAt']!;
+  envelope.state.collections['hosts']!.entities['host-1']!.fields['lastConnectedAt'] = {
+    candidates: telemetryRegister.candidates.map((candidate) => ({
+      dot: { ...candidate.dot },
+      context: candidate.context.map((contextDot) => ({ ...contextDot })),
+      hlc: { ...candidate.hlc },
+      materialized: true as const,
+    })),
+  };
+  const snapshot: SyncPayload = {
+    ...materializeSyncPayloadFromConvergentState(state, { syncedAt: NOW }),
+    hosts: [{ ...payload().hosts[0], lastConnectedAt: NOW }],
+  };
+  const hydrated = hydrateConvergentSyncEnvelope(envelope, snapshot);
+  const roundTripped = materializeSyncPayloadFromConvergentState(hydrated, { syncedAt: NOW });
+
+  assert.equal('lastConnectedAt' in (roundTripped.hosts[0] ?? {}), false);
+  const nextEnvelope = createConvergentSyncEnvelope(hydrated, roundTripped);
+  assert.equal(
+    'lastConnectedAt' in nextEnvelope.state.collections['hosts']!.entities['host-1']!.fields,
+    false,
+  );
+});
+
 test('trusted legacy diff becomes causal CRDT writes without carrying transport metadata', () => {
   const baseline = payload('Before');
   const state = createConvergentSyncStateFromPayload(baseline, 'seed', NOW);

domain/sync.ts

@@ -402,6 +402,33 @@ export function sanitizeHostsForSync(
   return hosts.map((host) => ({ ...host, lastConnectedAt: undefined }));
 }
 
+/**
+ * Host fields that hold device-local telemetry. `lastConnectedAt` updates on
+ * every successful SSH connect and must never enter cloud payloads,
+ * convergent CRDT state, or v2 envelopes (#2629).
+ */
+export const HOST_LOCAL_ONLY_FIELDS = ['lastConnectedAt'] as const;
+
+export function isHostLocalOnlyField(field: string): boolean {
+  return (HOST_LOCAL_ONLY_FIELDS as readonly string[]).includes(field);
+}
+
+/**
+ * Copy hosts with device-local telemetry fields removed entirely. Unlike
+ * `sanitizeHostsForSync` (which masks values for equality), this drops the
+ * keys so materialized cloud snapshots never carry a telemetry register.
+ */
+export function stripHostLocalOnlyFields(
+  hosts: import('./models').Host[] | undefined,
+): import('./models').Host[] | undefined {
+  if (!hosts) return hosts;
+  return hosts.map((host) => {
+    const copy = { ...host };
+    for (const field of HOST_LOCAL_ONLY_FIELDS) delete (copy as Record<string, unknown>)[field];
+    return copy;
+  });
+}
+
 /** Copy a payload with device-local host telemetry stripped for compare/convert. */
 export function withHostsSanitizedForSync(payload: SyncPayload): SyncPayload {
   return {
@@ -411,9 +438,12 @@ export function withHostsSanitizedForSync(payload: SyncPayload): SyncPayload {
 }
 
 /**
- * Re-attach this device's Recently Connected timestamps when a cloud payload
- * omits `lastConnectedAt`. Incoming timestamps (local backups, legacy cloud
- * snapshots) still win when present.
+ * Re-attach this device's Recently Connected ordering when applying a remote
+ * payload. `lastConnectedAt` is device-local (#2629), so incoming telemetry —
+ * even from legacy v1 payloads or older convergent replicas — is never
+ * authoritative: this device's timestamp always wins when it has one, hosts
+ * unknown to this device get no invented value, and foreign timestamps are
+ * dropped rather than replacing the local Recently Connected ordering.
  */
 export function retainLocalHostLastConnectedAt(
   incoming: import('./models').Host[] | undefined,
@@ -423,9 +453,12 @@ export function retainLocalHostLastConnectedAt(
   if (!local?.length) return incoming;
   const localById = new Map(local.map((host) => [host.id, host.lastConnectedAt]));
   return incoming.map((host) => {
-    if (host.lastConnectedAt != null) return host;
     const localTs = localById.get(host.id);
-    if (localTs == null) return host;
+    if (localTs == null) {
+      // Not known locally: never adopt another device's activity timestamps.
+      return host.lastConnectedAt === undefined ? host : { ...host, lastConnectedAt: undefined };
+    }
+    if (host.lastConnectedAt === localTs) return host;
     return { ...host, lastConnectedAt: localTs };
   });
 }