fix(api): stop full-model updates from clobbering targeted-write columns

[otavio]

What

Several *Update methods in api/store/pg issued full-model bun updates
(db.NewUpdate().Model(x).WherePK() with no allowlist) that rewrote every
mapped column from an in-memory snapshot. When a table has a column maintained
by a separate targeted write, a full-model update carrying a stale value
silently overwrote the concurrently-maintained value.

Why

Closes #6637.

The failure is a lost update that only manifests under concurrency — a counter
increment or heartbeat landing between the resolve and the save of a
full-model update. It passes the mocked-store service tests and is easy to miss
in review.

Changes

• entity ,skipupdate tags: keep a column out of every generated UPDATE
  while still allowing INSERT and explicit .Set(...) writes. Applied to the
  four namespace device counters, device custom_fields / last_seen /
  disconnected_at, user preferred_namespace_id, active_sessions.created_at,
  and the user_invitations.invitations counter. Using the entity tag (rather
  than a per-method column allowlist) means every current and future update path
  is protected by one declaration.
• device liveness → targeted writes: last_seen/disconnected_at are now
  owned by DeviceHeartbeat. The agent-auth online transition routes through
  DeviceHeartbeat, and OfflineDevice uses a new targeted DeviceOffline
  store method instead of a full-model update (which also drops a redundant
  DeviceResolve).
• preferred namespace → targeted write: login persists it through a new
  UserUpdatePreferredNamespace store method; the membership-removal clear in
  member.go is now a single conditional Set(... = NULL) instead of a
  select-then-model-update.
• invitations allowlist: dropped from UserInvitationUpdate's Column(...)
  list (now redundant with the entity tag).
• regression tests: api/store/storetest/clobber_tests.go reproduces each
  clobber by interleaving a targeted write with a stale full-model update and
  asserting the targeted column survives.

Testing

go test ./store/pg/... ./services/... inside the api container. The new
store-level tests need testcontainers (a real PostgreSQL), which the
mocked-store service tests cannot substitute for this class of bug. Removing any
one ,skipupdate tag makes the corresponding regression test fail.

[github-actions]

Claude finished @otavio's task in 10m 30s —— View job

---

Code Review Complete

Reviewed 22 files across code quality, security, testing, language patterns, and architecture. The core fix is solid — the ,skipupdate approach is applied consistently, the targeted-write replacements (DeviceHeartbeat, DeviceOffline, UserUpdatePreferredNamespace) are wired at every converted call site, the online transition in AuthDevice correctly clears disconnected_at via DeviceHeartbeat, and the new clobber_tests.go regression suite exercises each column against real Postgres. No SQL injection surface (all values bound via ?), no access-control regressions.

One issue survived validation but could not be posted inline (the line isn't part of this PR's diff), plus one cross-repo heads-up:

Additional notes

1. Missed clobber site — api/services/member.go:343-344 (LeaveNamespace) — medium

LeaveNamespace still clears the preferred namespace through a full-model update:

user.Preferences.PreferredNamespace = ""
if err := s.store.UserUpdate(ctx, user); err != nil { ... }

Now that entity.UserPreferences.PreferredNamespace is ,skipupdate (api/store/pg/entity/user.go:33), this UserUpdate no longer writes preferred_namespace_id — and it returns no error, so the log.WithError branch never fires. This is the same clobber class the PR fixes; the two auth.go sites were converted but this one was missed.

In the common case it's masked by removeMember → NamespaceDeleteMembership, which does a targeted Set(preferred_namespace_id = NULL) guarded by WHERE id = ? AND preferred_namespace_id = ? (the tenant being left). But that only clears when the stored preference equals the namespace being left. If the user's preference points at a different namespace, the old code force-cleared it here; post-PR that reset is silently lost. Suggest routing it through the targeted write (user stays used by the nil check above):

if err := s.store.UserUpdatePreferredNamespace(ctx, req.UserID, ""); err != nil {
    log.WithError(err).
        WithField("tenant_id", req.TenantID).
        WithField("user_id", req.UserID).
        Error("failed to reset user's preferred namespace")
}

Fix this →

2. Cross-repo mock regeneration (cloud)

This PR adds DeviceOffline and UserUpdatePreferredNamespace to the store.Store interface. The production cloud store embeds the core store, so it satisfies them automatically — but the generated cloud/pkg/store/mocks/mock_store.go does not, so the cloud test build (which passes the mock as a cloudstore.Store) will fail to compile until the cloud mocks are regenerated. Worth landing a paired cloud change / calling it out so it isn't forgotten.

(Minor, not blocking: the leftover user.Preferences.PreferredNamespace = tenantID assignment in CreateUserToken at auth.go is now dead — the persist goes through UserUpdatePreferredNamespace(ctx, user.ID, tenantID) — mirroring the deletion you already made in AuthLocalUser.)

---

To request another review round, comment /review.

[otavio]

Addressed in ffad49c:

1. LeaveNamespace missed clobber site — routed the preferred-namespace clear through UserUpdatePreferredNamespace(ctx, req.UserID, "") so it actually clears now that the column is skipupdate.
2. Dead CreateUserToken assignment — removed the leftover user.Preferences.PreferredNamespace = tenantID, mirroring the AuthLocalUser cleanup.
3. Cloud mock regeneration — a paired cloud PR regenerating pkg/store/mocks/mock_store.go (adding DeviceOffline / UserUpdatePreferredNamespace) is being prepared.

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 2m 23s —— View job

---

Code Review Complete

Reviewed 24 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

The two items from the previous round were correctly addressed in ffad49c:

• LeaveNamespace now clears the preference through the targeted UserUpdatePreferredNamespace(ctx, req.UserID, "") (api/services/member.go:344), so it actually clears now that preferred_namespace_id is skipupdate.
• The dead user.Preferences.PreferredNamespace = tenantID assignment in CreateUserToken was removed, mirroring the AuthLocalUser cleanup.

Spot-checks that passed on re-review:

• The agent-auth online transition sets device.LastSeen = clock.Now() before DeviceHeartbeat, and DeviceHeartbeat both writes last_seen and clears disconnected_at = NULL — so the online transition survives both columns becoming skipupdate.
• No leftover write sites: assignments to PreferredNamespace / device LastSeen / DisconnectedAt / CustomFields outside the converted paths are gone, so every future update path is covered by the entity tags.
• CreateUserToken writes tenantID, which equals namespace.TenantID — consistent with the guarding condition.
• The clobber_tests.go regression suite exercises each skipupdate column against real Postgres by interleaving a targeted write with a stale full-model update.

Reminder (already tracked by the author): the paired cloud PR regenerating pkg/store/mocks/mock_store.go for DeviceOffline / UserUpdatePreferredNamespace needs to land so the cloud test build compiles.

---

To request another review round, comment /review.

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 6m 10s —— View job

---

Code Review Complete

Reviewed 24 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

The two items from the prior round remain correctly addressed in cef0d6a:

• LeaveNamespace clears the preference through the targeted UserUpdatePreferredNamespace(ctx, req.UserID, ""), so it actually clears now that preferred_namespace_id is skipupdate.
• The dead PreferredNamespace = tenantID assignment in CreateUserToken was removed.

Spot-checks that passed on this round:

• ,skipupdate semantics — confirmed the tag excludes columns from full-model NewUpdate().Model(x).WherePK() SET clauses while still populating them on NewInsert(), so the four namespace counters, device custom_fields/last_seen/disconnected_at, preferred_namespace_id, active_sessions.created_at, and user_invitations.invitations are all genuinely protected.
• Targeted writes — DeviceOffline, DeviceHeartbeat (online transition: writes last_seen and clears disconnected_at = NULL), and UserUpdatePreferredNamespace bind every value via ? placeholders, scope by PK, and return ErrNoDocuments on zero rows — matching each caller's error handling. No SQL-injection surface, no dropped access-control check.
• NamespaceDeleteMembership — the select-then-update was replaced by a single atomic UPDATE ... WHERE id = ? AND preferred_namespace_id = ?, removing a TOCTOU race while preserving the "only clear if it points at the removed namespace" guard.
• Cross-repo — the cloud checkout's cloud/pkg/store/mocks/mock_store.go already contains DeviceOffline and UserUpdatePreferredNamespace, and cloud only reads the now-skipupdate columns, so there is no cloud compile break or silent-drop regression outstanding.
• Regression suite — clobber_tests.go exercises each column against real Postgres, asserting both that the intended column is persisted and the targeted column survives a stale full-model update.

---

To request another review round, comment /review.
• fix/pg-full-model-update-clobber

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 7m 16s —— View job

---

Code Review Complete

Reviewed 24 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

Spot-checks that passed on this round:

• ,skipupdate semantics — verified against uptrace/bun v1.2.18 source that Field.SkipUpdate() (HasOption("skipupdate")) is honored in both single-model (appendSetStruct) and slice (updateSliceSet) update paths, with no insert-time counterpart. So the tagged columns (namespace device counters, device custom_fields/last_seen/disconnected_at, preferred_namespace_id, active_sessions.created_at, user_invitations.invitations) are genuinely excluded from full-model UPDATEs while still written on INSERT and explicit .Set(...). Tag syntax matches the existing multi-option pattern in the same structs.
• Targeted writes — DeviceOffline, DeviceHeartbeat (online transition: last_seen = ? and disconnected_at = NULL), and UserUpdatePreferredNamespace bind every value via ?, scope by PK, wrap driver errors via fromSQLError, and return store.ErrNoDocuments on zero rows — consistent with sibling methods and matching each caller's handling. No SQL-injection surface, no dropped tenant/access-control check.
• NamespaceDeleteMembership — the select-then-update is a single atomic UPDATE ... WHERE id = ? AND preferred_namespace_id = ?, preserving the "only clear if it points at the removed namespace" guard.
• No orphaned no-ops — all DeviceUpdate/UserUpdate/NamespaceUpdate call sites in api/services/* were checked; the only online-transition full-model path (AuthDevice reconnect) is compensated by DeviceHeartbeat, and every preferred_namespace write routes through the targeted method.
• Cross-repo (cloud) — cloudstore.Store embeds the core store and the concrete cloud Store embeds *shpg.Pg, so DeviceOffline/UserUpdatePreferredNamespace are promoted automatically; cloud/pkg/store/mocks/mock_store.go already contains both. No cloud full-model update writes a now-skipupdate column, so no compile break or silent-drop regression.
• Regression suite — clobber_tests.go uses deterministic fixed clocks and load-bearing assertions (WithinDuration on last_seen, Before(2021) on created_at) that fail if the stale full-model update reverts the targeted column; the service-test changes (auth_test.go, device_test.go, member_test.go, setup_test.go) enforce the new store calls via mockery .Once() + AssertExpectations rather than being weakened.

---

To request another review round, comment /review.
• fix/pg-full-model-update-clobber