fix(octo): lease acquire/release must not bump updated_at (fixes restart loop)

server/internal/integrations/octo/db_test.go

@@ -603,6 +603,55 @@ func TestOctoWSLease_CAS(t *testing.T) {
 	}
 }
 
+// TestOctoWSLease_DoesNotBumpUpdatedAt is a regression guard: lease
+// acquire/renew and release must NOT advance updated_at. The hub treats an
+// advancing updated_at as a reconfigure signal and restarts the supervisor —
+// if a lease renewal bumped updated_at, every renewal would look like a
+// reconfigure and the connection would churn in a perpetual restart loop,
+// never holding the lease (caught in E2E acceptance after the reconfigure fix).
+func TestOctoWSLease_DoesNotBumpUpdatedAt(t *testing.T) {
+	requireDB(t)
+	q := db.New(testPool)
+	wsID, userID, agentID := fixture(t)
+	inst := newInstallation(t, q, wsID, userID, agentID)
+	ctx := context.Background()
+	token := pgtype.Text{String: "node-1:" + randToken(), Valid: true}
+	future := pgtype.Timestamptz{Time: time.Now().Add(90 * time.Second), Valid: true}
+
+	baseline := inst.UpdatedAt.Time
+
+	// Acquire then renew (same token re-acquires) — updated_at must stay put.
+	for i := 0; i < 2; i++ {
+		if _, err := q.AcquireOctoWSLease(ctx, db.AcquireOctoWSLeaseParams{
+			NewToken: token, NewExpiresAt: future, ID: inst.ID,
+		}); err != nil {
+			t.Fatalf("acquire/renew %d: %v", i, err)
+		}
+		got, err := q.GetOctoInstallation(ctx, inst.ID)
+		if err != nil {
+			t.Fatalf("reload: %v", err)
+		}
+		if !got.UpdatedAt.Time.Equal(baseline) {
+			t.Fatalf("acquire/renew advanced updated_at (%v -> %v); the hub would see a phantom reconfigure and restart-loop",
+				baseline, got.UpdatedAt.Time)
+		}
+	}
+
+	// Release must also leave updated_at untouched.
+	if err := q.ReleaseOctoWSLease(ctx, db.ReleaseOctoWSLeaseParams{
+		ID: inst.ID, CurrentToken: token,
+	}); err != nil {
+		t.Fatalf("release: %v", err)
+	}
+	got, err := q.GetOctoInstallation(ctx, inst.ID)
+	if err != nil {
+		t.Fatalf("reload after release: %v", err)
+	}
+	if !got.UpdatedAt.Time.Equal(baseline) {
+		t.Errorf("release advanced updated_at (%v -> %v)", baseline, got.UpdatedAt.Time)
+	}
+}
+
 // TestOctoUserBinding_NoSteal verifies the identity-binding boundary in
 // CreateOctoUserBinding: the same user re-binds idempotently (bound_at bumps),
 // a different user CANNOT steal an already-bound uid (zero rows), and a

server/pkg/db/queries/octo.sql

@@ -88,10 +88,13 @@ DELETE FROM octo_installation WHERE id = $1 AND workspace_id = $2;
 -- accepts the lease when (a) no current holder exists, (b) the holder's lease
 -- has expired, or (c) the holder is us (renewal). Returns the row when claimed;
 -- returns no rows when another live holder still owns it.
+-- Deliberately does NOT touch updated_at: lease acquire/renew is high-frequency
+-- operational churn, not a config change. The hub treats an advancing
+-- updated_at as a reconfigure signal (and restarts the supervisor), so bumping
+-- it here would make every renewal look like a reconfigure and loop forever.
 UPDATE octo_installation
 SET ws_lease_token      = sqlc.arg('new_token'),
-    ws_lease_expires_at = sqlc.arg('new_expires_at'),
-    updated_at          = now()
+    ws_lease_expires_at = sqlc.arg('new_expires_at')
 WHERE id = sqlc.arg('id')
   AND status = 'active'
   AND (
@@ -103,11 +106,11 @@ RETURNING *;
 
 -- name: ReleaseOctoWSLease :exec
 -- Drops the lease iff we're still the holder. A racing acquirer that already
--- took over will not have its lease cleared.
+-- took over will not have its lease cleared. Like AcquireOctoWSLease, this does
+-- NOT bump updated_at (lease churn is not a config change).
 UPDATE octo_installation
 SET ws_lease_token      = NULL,
-    ws_lease_expires_at = NULL,
-    updated_at          = now()
+    ws_lease_expires_at = NULL
 WHERE id = $1
   AND ws_lease_token = sqlc.arg('current_token');