[hemv-857]

Summary

Fixes #1671

Two appends to one session ran concurrently in the database session
service. They raced over the session row and over the in-memory event
list, and on SQLite one of them was aborted with a lock error. This
brings the Go backend in line with adk-python, which keeps one lock per
session and holds it across the stale check, the commit and the update of
the last update time.

What changed

• A per-session lock around AppendEvent, keyed by app, user and
  session. The lock is dropped once nobody holds or waits for it, so a
  long-lived service does not accumulate one lock per session it has seen.
  This matches adk-python's reference-counted per-session locks
  (database_session_service.py, _with_session_lock).
• The in-memory append moved after the commit. The event used to be
  added to the local session before it was written, so an append the
  database rejected still showed up in Events() and in the session state.
  The in-memory session still receives the untrimmed event, so temp:
  keys stay readable for the rest of the invocation and do not reach
  storage, the way the in-memory and the Vertex AI session services do.
• The session row is locked FOR UPDATE for the rest of the
  transaction, like adk-python, so an append from another process cannot
  write the row in between the read and the commit. The app and user state
  rows were already locked this way; the session row was not.

SQLite has no row-level locking and ignores the clause, so the FOR UPDATE is a no-op there; SQLite serializes writers anyway.

What behaves differently on the current release

• Two concurrent AppendEvent calls for one session no longer interleave.
  On SQLite the loser previously failed with database table is locked: database is deadlocked (6); it now fails with the stale-session error.
  Appends that used to collide now all succeed when they share one handle.
• An append that the database rejects no longer leaves its event in the
  in-memory session: neither in Events() nor in the session state.
• The in-memory event list is now in the order the events were committed
  in, rather than the order the calls were made in. Get returns them in
  timestamp order, which is a different order, as before.
• An append from a different handle to the same session still fails with
  the stale-session error, and that append's state key is still lost — the
  same as adk-python. Only the error changes: a stale-session error
  instead of a database lock error. On SQLite that was already loud; on
  Postgres it was silent.
• The data race between AppendEvent and LastUpdateTime() over
  updatedAt is unchanged; fix(session/database): read and write updatedAt under the session lock #1244 still tracks it. main wrote the field
  twice, once inside the transaction and once after it, and both writes
  were unguarded; this change leaves one unguarded write, so the race
  surface shrinks but the race remains. The per-session lock is what
  keeps a concurrent append from seeing the window between the commit and
  the write.

Delete, Get and List take no lock, which is what adk-python does
as well (delete_session, get_session and list_sessions take none).
They were considered and left alone here.

For #1244

This change removes the in-transaction updatedAt write that #1244
converts to setUpdatedAt. Whichever of the two lands second has to keep
exactly one write to updatedAt — a setUpdatedAt call before the
commit, or the remaining post-commit write converted. If both are
removed, updatedAt never advances and every append after the first on a
handle fails as stale.

Testing

• TestDatabaseService_AppendEvent_LocksTheSessionRow — the session row
  is read for update.
• TestDatabaseService_AppendEvent_FailedAppendLeavesNoEventInMemory — a
  rejected append leaves no event and no state key behind.
• TestDatabaseService_AppendEvent_KeepsTempStateReadableInMemory — a
  temp: key stays readable in the live session and does not reach
  storage.
• TestDatabaseService_AppendEvent_SerializesAppendsToOneSession — four
  appends to one session never have two of them inside a transaction at
  the same time, all four are stored, and every state key survives.
• TestKeyedMutex_DropsTheEntryWhenItIsUnused and
  TestKeyedMutex_SerializesOneKeyAtATime — the lock helper itself.

TDD: with session/database/service.go reverted to main, three of the
four behaviour tests fail and the suite is red.
TestDatabaseService_AppendEvent_KeepsTempStateReadableInMemory passes on
main on purpose: it guards a behaviour main already had and that an
earlier version of this PR broke, and the mutant that turns it red is
trimming the event before the in-memory append.

Each guard was also reverted on its own: dropping the per-session lock,
moving the in-memory append back before the write, trimming the event
before the in-memory append, and dropping the FOR UPDATE each one turns
its own test red. The lock entry is dropped in a separate test, which
fails when the entry is never dropped.

• go test -race -mod=readonly -count=1 -shuffle=on work — green
• golangci-lint run (v2.3.1, the version CI pins) — 0 issues
• gofumpt and goimports — clean

[caohy1988]

Same-session appends are now serialized, failed writes no longer change the in-memory event list, and temporary state stays readable after a successful commit. Two problems with the new locking should be fixed before merge.

AppendEvent now locks the session row before taking the app and user state locks, while Create still takes app and user state first and inserts the session last. The new comment notes that a Create for an existing session ID can block on the session-row lock; on MySQL that opposite order can deadlock, and the database may abort the valid append as the victim. I reproduced this against MySQL 8: a concurrent duplicate Create caused AppendEvent to fail with a deadlock error and left the event uncommitted. The same run succeeded when only service.go was restored to the base. Please give Create and AppendEvent a consistent lock order (for example app state, user state, then session), keep the stale-session check under the session-row lock, and add this concurrent duplicate-create case to the tests.

The new per-session mutex also waits in sync.Mutex.Lock without consulting the request context, and the holder keeps it for the whole transaction. A second AppendEvent whose context is already canceled, or whose deadline expires while it is queued, cannot return until the first request finishes. On the base implementation the canceled call reached the database layer and returned context.Canceled immediately. Please make lock acquisition context-aware (and drop the waiter's ref if it cancels), so a canceled append can leave the queue before the earlier transaction commits.

[hemv-857]

@caohy1988 — both problems are fixed and pushed in ec5ade9.

1. Lock order. AppendEvent now takes the app state row, then the user state row, then the session row — the same order Create already acquires them — and the stale-session check stays under the session-row lock.

Tests added:

• TestDatabaseService_AppendEvent_LocksStateRowsBeforeTheSessionRow records the statement order and fails on the old order.
• TestDatabaseService_AppendEvent_SurvivesConcurrentDuplicateCreate runs a duplicate Create of an existing session ID against an append with app, user, and session deltas; the append commits and the Create fails. The cycle the old order allows is what the reviewer repro showed; the test asserts the append side never loses.

2. Context-aware acquisition. Waiting is a select on ctx.Done(); a canceled or timed-out append returns ctx.Err() without taking the lock and drops its reference to the entry, so an unused entry is still reclaimed. TestDatabaseService_AppendEvent_CanceledAppendLeavesTheQueue holds one append inside its transaction, cancels a queued second append, and checks it returns context.Canceled while the holder is still running. TestKeyedMutex_CanceledRequestDoesNotTakeAFreeLock covers the canceled-before-acquisition case.

The local suite is SQLite-only, so the order test pins the sequence rather than reproducing the MySQL deadlock directly. go test -race -shuffle=on ./... and golangci-lint run are clean on the new head.

[hemv-857]

Synced with main at 5b3d897 (merge commit 6696817). The merge was clean — no conflicts. Re-ran go test ./session/database/... and go vet on the merged tree: green.

[hemv-857]

Lint follow-up: golangci-lint v2.15.0 enables the modernize waitgroupgo analyzer, which flagged the Add(1)/go func/defer Done pair in the new concurrency test. Switched to wg.Go; behavior unchanged. Head now d9a613c, all 11 checks pass, MERGEABLE.