Skip to content

Commit 64dba4b

Browse files
committed
Preserve legacy ownership across machine renames
1 parent f3fa344 commit 64dba4b

4 files changed

Lines changed: 126 additions & 26 deletions

File tree

internal/postgres/collision_pgtest_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestPushSessionGuardsAgainstCrossMachineCollision(t *testing.T) {
8484
// Execute pushSession.
8585
tx, err := pg.BeginTx(ctx, nil)
8686
require.NoError(t, err, "BeginTx")
87-
err = sync.pushSession(ctx, tx, sess, markerID)
87+
err = sync.pushSession(ctx, tx, sess, markerID, "")
8888
require.ErrorIs(t, err, errSessionOwnershipConflict, "pushSession should return ownership conflict sentinel")
8989
require.NoError(t, tx.Commit(), "Commit")
9090

@@ -156,7 +156,7 @@ func TestPushSessionAllowsMachineRenameForSameOwnerMarker(t *testing.T) {
156156

157157
tx, err := pg.BeginTx(ctx, nil)
158158
require.NoError(t, err, "BeginTx")
159-
require.NoError(t, sync.pushSession(ctx, tx, sess, markerID), "pushSession")
159+
require.NoError(t, sync.pushSession(ctx, tx, sess, markerID, ""), "pushSession")
160160
require.NoError(t, tx.Commit(), "Commit")
161161

162162
var machine, ownerMarker string
@@ -215,7 +215,7 @@ func TestPushSessionAdoptsLegacyLocalSentinelRow(t *testing.T) {
215215
require.NoError(t, err, "BeginTx")
216216
markerID, err := sync.pushMarkerID()
217217
require.NoError(t, err, "pushMarkerID")
218-
require.NoError(t, sync.pushSession(ctx, tx, sess, markerID), "pushSession")
218+
require.NoError(t, sync.pushSession(ctx, tx, sess, markerID, ""), "pushSession")
219219
require.NoError(t, tx.Commit(), "Commit")
220220

221221
var machine, ownerMarker string

internal/postgres/name_source_pgtest_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestPushSessionNameRoundTrip(t *testing.T) {
6262
// Push via pushSession directly.
6363
tx, err := pg.BeginTx(ctx, nil)
6464
require.NoError(t, err, "BeginTx")
65-
if err := sync.pushSession(ctx, tx, sess, markerID); err != nil {
65+
if err := sync.pushSession(ctx, tx, sess, markerID, ""); err != nil {
6666
_ = tx.Rollback()
6767
t.Fatalf("pushSession: %v", err)
6868
}
@@ -105,7 +105,7 @@ func TestPushSessionNameRoundTrip(t *testing.T) {
105105

106106
tx2, err := pg.BeginTx(ctx, nil)
107107
require.NoError(t, err, "BeginTx (second)")
108-
if err := sync.pushSession(ctx, tx2, sess, markerID); err != nil {
108+
if err := sync.pushSession(ctx, tx2, sess, markerID, ""); err != nil {
109109
_ = tx2.Rollback()
110110
t.Fatalf("pushSession (second): %v", err)
111111
}

internal/postgres/push.go

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ func (s *Sync) Push(
8585
if err != nil {
8686
return result, err
8787
}
88+
previousMarkerMachine, markerExists, err := s.pgPushMarkerMachine(ctx, markerID)
89+
if err != nil {
90+
return result, err
91+
}
8892
if full {
8993
lastPush = ""
9094
// Caller requested a full push — the PG schema
@@ -113,17 +117,14 @@ func (s *Sync) Push(
113117
// was reset (schema dropped, DB recreated, etc.). Force a full
114118
// push so all sessions are re-synced.
115119
if lastPush != "" {
116-
markerExists, cErr := s.pgPushMarkerExists(ctx, markerID)
117-
if cErr != nil {
118-
return result, cErr
119-
}
120120
if !markerExists {
121121
log.Printf(
122122
"pgsync: local watermark set but PG push marker " +
123123
"missing; PG was reset, forcing full push",
124124
)
125125
lastPush = ""
126126
full = true
127+
previousMarkerMachine = ""
127128
s.schemaMu.Lock()
128129
s.schemaDone = false
129130
s.schemaMu.Unlock()
@@ -278,7 +279,7 @@ func (s *Sync) Push(
278279
batch := sessions[i:end]
279280

280281
batchResult, err := s.pushBatch(
281-
ctx, batch, full, markerID, &pushed,
282+
ctx, batch, full, markerID, previousMarkerMachine, &pushed,
282283
)
283284
if err != nil {
284285
return result, err
@@ -293,7 +294,7 @@ func (s *Sync) Push(
293294
for _, sess := range batch {
294295
sr, retryErr := s.pushBatch(
295296
ctx, []db.Session{sess},
296-
full, markerID, &pushed,
297+
full, markerID, previousMarkerMachine, &pushed,
297298
)
298299
if retryErr != nil {
299300
return result, retryErr
@@ -369,29 +370,33 @@ func (s *Sync) Push(
369370
return result, nil
370371
}
371372

372-
// pgPushMarkerExists reports whether this host's push marker is present in PG.
373+
// pgPushMarkerMachine reports whether this host's push marker is present in PG
374+
// and returns the machine value stored with the marker.
373375
// A missing marker while the local watermark is set means PG was reset (schema
374376
// dropped or recreated) since this host last pushed, so a full re-push is
375377
// needed. Counting rows by machine cannot detect this reliably: another host
376378
// pushing to the same PG can repopulate rows under a machine value this host
377379
// also writes -- a remote host's sessions synced in over SSH, or this host's
378380
// own renamed identity -- masking the loss of this host's own rows. The marker
379381
// is per-local-DB, so no other pusher can satisfy this check.
380-
func (s *Sync) pgPushMarkerExists(ctx context.Context, markerID string) (bool, error) {
381-
var exists bool
382+
func (s *Sync) pgPushMarkerMachine(ctx context.Context, markerID string) (string, bool, error) {
383+
var machine string
382384
err := s.pg.QueryRowContext(ctx,
383-
`SELECT EXISTS (SELECT 1 FROM sync_metadata WHERE key = $1)`,
385+
`SELECT value FROM sync_metadata WHERE key = $1`,
384386
pushMarkerKeyPrefix+markerID,
385-
).Scan(&exists)
387+
).Scan(&machine)
386388
if err != nil {
389+
if errors.Is(err, sql.ErrNoRows) {
390+
return "", false, nil
391+
}
387392
if isUndefinedTable(err) {
388-
return false, nil
393+
return "", false, nil
389394
}
390-
return false, fmt.Errorf(
395+
return "", false, fmt.Errorf(
391396
"checking pg push marker: %w", err,
392397
)
393398
}
394-
return exists, nil
399+
return machine, true, nil
395400
}
396401

397402
// writePushMarker records this host's push marker in PG so a later push can
@@ -452,6 +457,7 @@ func (s *Sync) pushBatch(
452457
batch []db.Session,
453458
full bool,
454459
markerID string,
460+
previousMarkerMachine string,
455461
pushed *[]db.Session,
456462
) (batchResult, error) {
457463
tx, err := s.pg.BeginTx(ctx, nil)
@@ -466,7 +472,7 @@ func (s *Sync) pushBatch(
466472
skippedConflicts := 0
467473
for _, sess := range batch {
468474
if err := s.pushSession(
469-
ctx, tx, sess, markerID,
475+
ctx, tx, sess, markerID, previousMarkerMachine,
470476
); err != nil {
471477
if errors.Is(err, errSessionOwnershipConflict) {
472478
skippedConflicts++
@@ -798,7 +804,10 @@ func pushedSessionMachine(sess db.Session, fallbackMachine string) string {
798804
return fallbackMachine
799805
}
800806

801-
func sameSessionOwner(existingOwnerMarker, existingMachine, markerID, pushedMachine string) bool {
807+
func sameSessionOwner(
808+
existingOwnerMarker, existingMachine, markerID, pushedMachine,
809+
previousMarkerMachine string,
810+
) bool {
802811
if existingOwnerMarker != "" {
803812
return existingOwnerMarker == markerID
804813
}
@@ -808,6 +817,9 @@ func sameSessionOwner(existingOwnerMarker, existingMachine, markerID, pushedMach
808817
if existingMachine == "local" {
809818
return true
810819
}
820+
if previousMarkerMachine != "" && existingMachine == previousMarkerMachine {
821+
return true
822+
}
811823
return existingMachine == pushedMachine
812824
}
813825

@@ -872,7 +884,7 @@ func nilStrTS(s *string) any {
872884
// local-only and used solely by the sync engine to detect
873885
// re-parsed sessions.
874886
func (s *Sync) pushSession(
875-
ctx context.Context, tx *sql.Tx, sess db.Session, markerID string,
887+
ctx context.Context, tx *sql.Tx, sess db.Session, markerID, previousMarkerMachine string,
876888
) error {
877889
createdAt, _ := ParseSQLiteTimestamp(sess.CreatedAt)
878890
isAutomated := sess.IsAutomated
@@ -890,6 +902,7 @@ func (s *Sync) pushSession(
890902
existingMachine.String,
891903
markerID,
892904
pushedMachine,
905+
previousMarkerMachine,
893906
) {
894907
log.Printf(
895908
"pgsync: session %s: skipping — already owned by machine %q, "+
@@ -989,7 +1002,9 @@ func (s *Sync) pushSession(
9891002
WHERE ((
9901003
sessions.owner_marker = ''
9911004
AND (sessions.machine = EXCLUDED.machine
992-
OR sessions.machine = 'local')
1005+
OR sessions.machine = 'local'
1006+
OR sessions.machine = ''
1007+
OR sessions.machine = $48)
9931008
)
9941009
OR sessions.owner_marker = EXCLUDED.owner_marker)
9951010
AND (
@@ -1070,6 +1085,7 @@ func (s *Sync) pushSession(
10701085
sess.HealthScore, nilStr(sess.HealthGrade),
10711086
sess.HasToolCalls, sess.HasContextData,
10721087
sess.SecretLeakCount, sess.SecretsRulesVersion,
1088+
previousMarkerMachine,
10731089
)
10741090
if err != nil {
10751091
return err
@@ -1084,7 +1100,10 @@ func (s *Sync) pushSession(
10841100
currentOwnerMarker = existingOwnerMarker.String
10851101
currentMachine = existingMachine.String
10861102
}
1087-
if refreshErr == nil && !sameSessionOwner(currentOwnerMarker, currentMachine, markerID, pushedMachine) {
1103+
if refreshErr == nil && !sameSessionOwner(
1104+
currentOwnerMarker, currentMachine, markerID, pushedMachine,
1105+
previousMarkerMachine,
1106+
) {
10881107
log.Printf(
10891108
"pgsync: session %s: skipping — already owned by machine %q, this pusher is %q; sync from the origin machine to update",
10901109
sess.ID, currentMachine, pushedMachine,

internal/postgres/push_pgtest_test.go

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ func TestPushSessionTerminationStatus(t *testing.T) {
373373
t.Helper()
374374
tx, err := pg.BeginTx(ctx, nil)
375375
require.NoError(t, err, "BeginTx")
376-
if err := sync.pushSession(ctx, tx, s, markerID); err != nil {
376+
if err := sync.pushSession(ctx, tx, s, markerID, ""); err != nil {
377377
_ = tx.Rollback()
378378
t.Fatalf("pushSession: %v", err)
379379
}
@@ -439,7 +439,7 @@ func TestPushSessionPreservesSourceMachine(t *testing.T) {
439439
require.NoError(t, err, "BeginTx")
440440
markerID, err := sync.pushMarkerID()
441441
require.NoError(t, err, "pushMarkerID")
442-
require.NoError(t, sync.pushSession(ctx, tx, remoteSession, markerID), "pushSession")
442+
require.NoError(t, sync.pushSession(ctx, tx, remoteSession, markerID, ""), "pushSession")
443443
require.NoError(t, tx.Commit(), "Commit")
444444

445445
var got string
@@ -1101,6 +1101,87 @@ func TestPushUpdatesSentinelMachineWhenSyncMachineChanges(t *testing.T) {
11011101
"sentinel machine must follow the new fallback")
11021102
}
11031103

1104+
func TestPushAdoptsOwnerlessRowsFromPreviousMarkerMachine(t *testing.T) {
1105+
pgURL := testPGURL(t)
1106+
1107+
const schema = "agentsview_push_legacy_marker_machine_test"
1108+
pg, err := Open(pgURL, schema, true)
1109+
require.NoError(t, err, "Open")
1110+
defer pg.Close()
1111+
1112+
ctx := context.Background()
1113+
_, err = pg.Exec(`DROP SCHEMA IF EXISTS ` + schema + ` CASCADE`)
1114+
require.NoError(t, err, "drop schema")
1115+
require.NoError(t, EnsureSchema(ctx, pg, schema), "EnsureSchema")
1116+
1117+
localDB, err := db.Open(filepath.Join(t.TempDir(), "local.db"))
1118+
require.NoError(t, err, "db.Open")
1119+
defer localDB.Close()
1120+
1121+
const markerID = "legacy-marker-1"
1122+
require.NoError(t, localDB.SetSyncState("pg_push_marker_id", markerID),
1123+
"seed local push marker")
1124+
1125+
sync := &Sync{
1126+
pg: pg,
1127+
local: localDB,
1128+
machine: "host-b",
1129+
schema: schema,
1130+
schemaDone: true,
1131+
}
1132+
1133+
const sessID = "legacy-previous-machine-1"
1134+
require.NoError(t, localDB.UpsertSession(db.Session{
1135+
ID: sessID,
1136+
Project: "proj",
1137+
Machine: "host-b",
1138+
Agent: "claude",
1139+
MessageCount: 1,
1140+
CreatedAt: "2026-01-01T00:00:00Z",
1141+
}), "UpsertSession")
1142+
require.NoError(t, localDB.InsertMessages([]db.Message{{
1143+
SessionID: sessID,
1144+
Ordinal: 0,
1145+
Role: "assistant",
1146+
Content: "hello",
1147+
ContentLength: 5,
1148+
}}), "InsertMessages")
1149+
1150+
_, err = pg.ExecContext(ctx, `
1151+
INSERT INTO sync_metadata (key, value)
1152+
VALUES ($1, $2)
1153+
`, pushMarkerKeyPrefix+markerID, "host-a")
1154+
require.NoError(t, err, "seed previous marker machine")
1155+
_, err = pg.ExecContext(ctx, `
1156+
INSERT INTO sessions (
1157+
id, machine, owner_marker, project, agent, created_at
1158+
) VALUES ($1, $2, $3, $4, $5, NOW())
1159+
`, sessID, "host-a", "", "proj", "claude")
1160+
require.NoError(t, err, "seed ownerless legacy session")
1161+
1162+
res, err := sync.Push(ctx, true, nil)
1163+
require.NoError(t, err, "Push")
1164+
assert.Zero(t, res.Errors, "push should report no failed sessions")
1165+
assert.Zero(t, res.SkippedConflicts,
1166+
"previous-marker-machine legacy row should be adopted")
1167+
assert.Equal(t, 1, res.SessionsPushed,
1168+
"legacy row should be counted as pushed")
1169+
1170+
var machine, ownerMarker string
1171+
require.NoError(t, pg.QueryRow(
1172+
`SELECT machine, owner_marker FROM sessions WHERE id = $1`, sessID,
1173+
).Scan(&machine, &ownerMarker), "reading adopted row")
1174+
assert.Equal(t, "host-b", machine)
1175+
assert.Equal(t, markerID, ownerMarker)
1176+
1177+
var markerMachine string
1178+
require.NoError(t, pg.QueryRow(
1179+
`SELECT value FROM sync_metadata WHERE key = $1`,
1180+
pushMarkerKeyPrefix+markerID,
1181+
).Scan(&markerMachine), "reading marker machine")
1182+
assert.Equal(t, "host-b", markerMachine)
1183+
}
1184+
11041185
func TestPushReportsSkippedConflicts(t *testing.T) {
11051186
pgURL := testPGURL(t)
11061187

0 commit comments

Comments
 (0)