Fix: append a session event in a single database transaction

[AmaadMartin]

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):
   No existing public issue.
2. Or, if no issue exists, describe the change:

Problem: DatabaseSessionService.appendEvent wraps its work in
em.transactional(...) but calls await txEm.commit() inside the callback,
before the callback has finished. MikroORM's transactional() already flushes
and commits when the callback returns, so the explicit commit ends the
transaction early and the trailing flush runs in a second, implicit
transaction. Three consequences:

• The append is not atomic. The event insert commits in transaction 1; the
  sessions.update_time write happens in transaction 2. If the second write
  fails, appendEvent rejects while the event row is durably persisted with a
  stale update_time.
• The PESSIMISTIC_WRITE row lock is released mid-critical-section. The
  stale-session check reads storageSession.updateTime under the lock taken by
  the initial findOne, but the update_time write happens after that lock was
  released — on the server drivers, on a different pooled connection. Two
  concurrent appends can interleave between the two transactions and clobber the
  value the stale check depends on.
• A doubled COMMIT is issued (knex tolerates it on SQLite today; nothing
  promises that for every dialect) and the caller's in-memory session object
  was mutated before the durable write finished, so a failed append left it
  partially updated.

Observed statement log for one appendEvent, captured with
debug: ['query'] against SQLite (the update sessions … create_time, update_time inside the first transaction is MikroORM's flush rewriting the row
with its existing timestamps):

Before

begin                                          <- em.transactional() opens TX1
select ... from `sessions` ... limit ?
select ... from `app_states` ...
select ... from `user_states` ...
select ... from `events` ...
insert into `events` ...
update `user_states` set `update_time` = ?
update `app_states` set `update_time` = ?
update `sessions` set `create_time` = ?, `update_time` = ? ...
commit                                         <- txEm.commit(): TX1 ends here
begin                                          <- implicit TX2 (trailing flush)
update `sessions` set `update_time` = ? ...    <- the event timestamp write
commit                                         <- TX2 ends
commit                                         <- TX1 committed a second time

After

begin
select ... from `sessions` ... limit ?
select ... from `app_states` ...
select ... from `user_states` ...
select ... from `events` ...
insert into `events` ...
update `user_states` set `update_time` = ?
update `app_states` set `update_time` = ?
update `sessions` set `create_time` = ?, `update_time` = ? ...
commit

Solution:

• Delete await txEm.commit() and let em.transactional() flush and commit
  when the callback returns.
• Keep storageSession.updateTime = new Date(event.timestamp) inside the
  callback so it is part of the single flush, and keep
  LockMode.PESSIMISTIC_WRITE exactly where it is — it now covers the whole
  read-check-write critical section, which is the point of the fix.
• Return the values the post-commit block needs (mergedState, updateTime,
  reloadedEvents) from the callback and mutate the caller's session only
  after em.transactional(...) resolves.

Notes for the reviewer:

• session.state = mergedState in the stale-session branch was removed
  deliberately, as provably dead code. The final mergeStates call
  unconditionally overwrote session.state with a merge over the same three
  state objects after the deltas were applied, so the intermediate value was
  never observable. The branch now only carries the reloaded events out to the
  post-commit block.
• The caller's session object is now mutated only after a successful
  commit. No call site depends on the old partial-mutation behaviour:
  core/src/runner/runner.ts uses try/finally with no catch;
  dev/src/cli/cli_run.ts catches, logs and returns without touching session;
  core/src/a2a/agent_executor.ts catches but reads a different Session
  object. On the success path the mutations are identical and still happen
  before appendEvent resolves, so the runner's reliance on in-place mutation
  of the shared InvocationContext.session is preserved.
• The transaction result is held in a local result object rather than
  destructured inline at the await. Inline destructuring pushes the assignment
  past 80 columns, and Prettier then re-indents the entire 100-line callback
  body — a 219-line diff that is 90% whitespace instead of the 51-line
  behavioural diff here. Semantics are identical.
• Public API surface is unchanged: same class, constructor, method signature,
  return value and exports. Not a breaking change.
• Collision check. gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 (392 open PRs) surfaced three that touch
  core/src/sessions/database_session_service.ts: Fix: remove private-member casts from the DatabaseSessionService test and add a public close() #386 (adds a public
  close(), removes private-member casts from the test), Fix: align the listSessions() state contract across all session services (stacked on #376) #477 and Fix: make ListSessionsRequest.userId optional (adk-python parity) #376
  (listSessions filter/state contract). gh pr diff <n> --name-only plus the
  hunks confirm none of them touch appendEvent, so this branches from
  main rather than stacking. Fix: remove private-member casts from the DatabaseSessionService test and add a public close() #386 may conflict textually with the test file if it
  lands first. The new tests reach the service's MikroORM handle through a
  single documented ormOf() seam in the test file rather than repeating the
  file's existing per-site cast, and deliberately do not widen private orm or
  add a test-only getter.
• Follow-up, deliberately out of scope: adk-python additionally carries an
  explicit storage revision marker plus an in-process per-session lock, and
  rejects stale writers instead of silently reloading. That is a behaviour
  change needing its own design review; this PR only restores the atomicity and
  lock-window guarantees the existing code already intends.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

Five new tests in core/test/sessions/database_session_service_test.ts, under a
new appendEvent atomicity describe. No existing test was modified or deleted —
in particular "should align session updateTime with event timestamp" and
"should append event and update state" still pass verbatim.

npx vitest run --project unit:core core/test/sessions/database_session_service_test.ts
  -> Tests  31 passed (31)
npx vitest run --project unit:core core/test/sessions/ core/test/runner/
  -> Tests  190 passed (190)
npx vitest run --project integration tests/integration/lazy_load_db_drivers
  -> Tests  5 passed (5)
npm run build / npm run lint / npm run format:check / npm run docs:check  -> all clean

Every new test was run against the unfixed code and confirmed to fail
(mutation → failure message):

1. should append an event in a single transaction — reverts the whole source
   hunk: AssertionError: expected [ 'begin', 'begin' ] to have a length of 1 but got 2 (pre-fix also emits three commits, with the update_time write
   after the first one).
2. should not persist the event when the session update fails — same
   revert: AssertionError: expected [ …(1) ] to have a length of +0 but got 1
   (the event row is leaked: durably inserted while the caller sees a
   rejection).
3. should leave the in-memory session untouched when the append fails — same
   revert: AssertionError: expected [ { timestamp: 1785638820267, …(6) } ] to have a length of +0 but got 1 (the in-memory session.events had already
   grown before the trailing flush threw).
4. should reload events from storage when the in-memory session is stale —
   passes before and after the fix (it covers the restructured branch), so it
   was proved by mutation instead: dropping session.events = result.reloadedEvents ?? session.events; gives AssertionError: expected [ 'zgZnoFHZ' ] to deeply equal [ 'hpBcfxSF', 'zgZnoFHZ' ].
5. should replace an event that was already appended — covers the moved
   index >= 0 post-commit branch; replacing it with an unconditional
   session.events.push(event) gives AssertionError: expected [ …(2) ] to have a length of 1 but got 2.

Tests 2 and 3 install a SQLite trigger that aborts the update_time write. The
trigger carries a WHEN NEW.update_time <> OLD.update_time clause on purpose:
MikroORM's flush also rewrites the session row with its existing timestamps
inside the transaction, and blocking that write would abort the append before it
ever reaches the update_time change under test — which would make both tests
pass against the buggy code.

Test 1 strips ANSI escapes from the captured query log even though the service
is constructed with colors: false. MikroORM's colors flag is process-global
(process.env.MIKRO_ORM_COLORS, see @mikro-orm/core/logging/colors.js), so
another service in the same suite re-enables it; this was observed, not
assumed.

Coverage (npx vitest run --project unit:core --coverage core/test/sessions/database_session_service_test.ts): the restructured
stale-session branch (previously 18 uncovered lines) and the post-commit
mutation block are now fully covered. Four blocks inside appendEvent remain
uncovered, all of them pre-existing gaps that this change only re-indents or
does not touch at all: the event.partial early return (unchanged by this
diff), and the StorageAppState / StorageUserState row-creation branches,
which are unreachable through the public API because createSession always
creates both rows.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

Run against a real file-backed SQLite store through the public
getSessionServiceFromUri entry point (no mocks), with a service restart in the
middle:

1. Create a session via getSessionServiceFromUri('sqlite:///tmp/adk.sqlite').
2. Append 4 events alternating user/agent, each with a stateDelta.
3. Construct a second service against the same file and getSession(...).
4. Confirm all 4 events come back in timestamp order, the merged state reflects
   every delta, lastUpdateTime equals the last event's timestamp, and
   listSessions returns the session.

This was run locally and passes. Equivalently, with an agent directory:

npx adk run <agent-dir> --session_service_uri "sqlite://./adk_manual.sqlite"

Hold a 3-4 turn conversation, exit, and re-run against the same URI: the prior
turns are restored in order. With debug: ['query'] each turn now emits exactly
one begin/commit pair per append.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.