Cross-process race in TranscriptLocator generation writes (session identity)

[senamakel]

Context

PR #197 (durable session identity) added a per-path, in-process mutex
(path_lock in crates/tinyagents-session/src/transcript/history.rs)
serializing FileTranscriptHistory::append_turn/append_turn_with_partial/
append/replace/clear for a given resolved path, and made
Session::persist (in tinyagents-runtime) commit a compaction's successor
generation to target/self.transcript only after the append into it
succeeds (pending_generation in crates/tinyagents-runtime/src/session.rs).

Both of those are real improvements over the pre-#197 baseline, where there
was no locking of any kind and the append-vs-compaction diff was computed
purely against in-memory state — a second process could already emit a
replacement record that erased the first process's turns, with no mitigation
at all.

What remains open

Two related gaps remain, both cross-process (two separate OS processes,
not two threads/handles within one process — the in-process case above is
already closed):

1. TranscriptLocator::begin_generation's existence check and the
   handle construction it returns are not atomic across processes.
   FileTranscriptHistory::new performs no I/O — it only resolves a path —
   so two processes compacting the same session at the same instant can both
   observe the successor generation's path as absent (via session_exists)
   and both return handles bound to the same not-yet-existing file. Neither
   process's own view is invalidated by the other having done the same
   check.

2. The eventual first write into that shared successor path is not
   mutually exclusive across processes either. The writer's create-fresh
   branch (append_transcript_turn_with_partial's !file_exists path) uses
   a plain fs::write, which has no OS-level exclusivity: whichever of the
   two processes' writes lands last silently wins, discarding the other's
   retained (post-compaction) message set with no error and no trace beyond
   the lost data itself.

Why this needs its own design, not a quick patch

These two pull in opposite directions and have to be resolved together:

• Making generation reservation atomic (e.g. an exclusive create_new on
  the successor's .jsonl path, claiming the filename before any content is
  known) would satisfy (1), but by itself would violate the "commit the
  successor only after its opening append succeeds" invariant this PR just
  established — a process that reserves the file and then fails or crashes
  before completing its append would leave an empty, "existing" generation
  that head_generation might select over the one that should actually be
  current.
• Making the content write atomic (e.g. via the same create-if-absent +
  fs::hard_link publish pattern write_transcript_if_absent already uses
  for adoption in this PR) closes (2) but does nothing for (1): two
  processes could still both build a complete, valid successor payload in
  parallel and race to publish it, with the loser's compaction silently
  discarded rather than retried or reported.

A correct fix needs a single cross-process protocol that reserves the
generation slot and commits its content atomically together — most likely an
OS-level advisory file lock (flock/LockFileEx, ideally via a small,
audited dependency) spanning both the existence check and the write, or a
compare-and-swap style append protocol. That is a deliberate,
separately-reviewable design decision, not something to fold into a session
identity PR.

Scope

• crates/tinyagents-session/src/transcript/history.rs:
  TranscriptLocator::begin_generation's default and
  FileTranscriptLocator's override.
• crates/tinyagents-session/src/transcript/writer.rs:
  append_transcript_turn_with_partial's create-fresh branch.
• Any fix must preserve the existing invariants from Durable session identity: resume a conversation by id, and stop losing history #197: deterministic,
  timestamp-free stems; a sealed generation stays byte-identical; adoption
  never touches a legacy file; a root stem never contains __.

References

• PR Durable session identity: resume a conversation by id, and stop losing history #197 (introduces path_lock and the in-process fix), see its
  "Known limitations" section and the review threads on
  history.rs:494/history.rs:806 for the original reports.

[tinysweeper]

Cross-process race in TranscriptLocator generation writes causes silent data loss when two OS processes compact the same session concurrently.

Labelled priority: p1.

[senamakel]

Addendum: the same class of gap also applies within a single process

A CodeRabbit review on PR #197 (round 5, history.rs:818) identified a
related but distinct gap in the same family as this issue's two points:

FileTranscriptHistory::append_turn/append_turn_with_partial (guarded by
path_lock as of PR #197) forward the caller-supplied TranscriptTurn::prev
baseline directly to the writer, which uses it to decide append-vs-compaction
without comparing it against the file's actual current content. path_lock
serializes the write call itself (so two handles can no longer race on the
writer's create-fresh branch or interleave bytes), but it does not validate
that prev still matches disk state inside the critical section.

crates/tinyagents-runtime's Session::persist is the caller that matters
here: it deliberately keeps its own in-memory persisted baseline across
turns rather than re-reading the transcript file every time (see the doc
comment on TranscriptTurn::prev in history.rs — this is an intentional
performance choice, not an oversight, since a disk re-read would also be
lossy relative to the in-memory representation for other reasons documented
there).

The consequence: two separate Session objects in one process bound to
the same transcript file (not a cross-process scenario at all) can each
compute a diff against their own stale persisted view. path_lock prevents
their writes from corrupting each other at the byte level, but does not
prevent one of them from legitimately-per-its-own-stale-view deciding "this is
a compaction" and emitting a replacement record that makes the other's
already-durable turn logically unreachable through the canonical (last
compaction wins) read path — even though the bytes are still physically on
disk.

Why this belongs here rather than as a separate issue

It is the same underlying question this issue already poses for the
cross-process case: whichever protocol reserves/validates a write also has to
either invalidate a stale caller's view or force a fresh read before it can
decide append-vs-compaction. A fix for the cross-process reservation race and
a fix for this in-process stale-baseline gap will likely share the same
answer (a compare-and-swap style append, or validating/refreshing the
baseline under the same lock that guards the write) — so they should be
designed together rather than patched independently.

Scope addition

• crates/tinyagents-session/src/transcript/history.rs:
  FileTranscriptHistory::append_turn_locked/append_turn_with_partial_locked.
• crates/tinyagents-runtime/src/session.rs: Session::persist's use of its
  in-memory persisted field as the diff baseline.

Reference: PR #197, CodeRabbit thread on history.rs:818 (round 5).