Skip to content

fix(core): keep session I/O flat as history grows and resume through symlinks - #354

Merged
Max17190 merged 5 commits into
mainfrom
keep-the-session-index-off-the-hot-path
Oct 6, 2026
Merged

Max17190 merged 5 commits into
mainfrom
keep-the-session-index-off-the-hot-path

Conversation

@Max17190

@Max17190 Max17190 commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Why

Every model request and every transcript save read and parsed the whole session index just to confirm their session still existed. The index lists every session of every project and is never pruned, so each request got slower as history accumulated. Every turn start also reparsed the full transcript of a session this process already owned and had just written.

Separately, session lookup compared project paths byte for byte, while trust keys a project by its resolved path, so a lookup spelled through a symlink (such as /tmp for /private/tmp) found none of the sessions recorded in the directory it leads to.

Summary

  • A process reads the index once when it claims a session and remembers that the session is listed. Usage appends, transcript saves, and manifest saves then read no index bytes in steady state.
  • Deleting or discarding a session in this process records it as removed, so a late write after the delete is still dropped without reading the index. Releasing a claim forgets what was remembered before the session's lock is let go.
  • A session claimed while the index was damaged, or not listed in it, still checks the index on every write, so the warning that a save was dropped over a damaged index still fires.
  • Behaviour change: if the index is damaged after a session was claimed while it was healthy, that session keeps saving, and its later history is written but not listed by --continue, /resume, or --recall. The --check repair advice (close every openmax before moving the index aside) is unchanged, and its explanation now describes both cases.
  • A turn start on an owned session reparses the transcript only when its length, modification time, or status-change time changed since this process last validated or wrote it. The status-change time moves on every write and every timestamp change and cannot be set, so an outside edit that keeps the length and puts the modification time back is still reparsed. An append updates that record only when nothing else wrote the file in between, so damage written from outside is still refused at the next turn start.
  • Session lookup (latest, --continue, /resume, --recall) resolves the requested path and compares it, as given and as resolved, with each stored path as recorded. Frontends store the working directory already resolved, the form trust keys a project by. A stored path is never resolved again: when a project moves and a symlink to another project takes its place, that other project inherits none of the history recorded there, as trust treats the same move. A relative stored path such as . (stored when the working directory cannot be read) matches only its own spelling. index.json is unchanged.
  • Four existing deferred-hydration repair tests now release the session before damaging the index, so hydration claims it over the damage and the documented refuse-then-land flow is still exercised.

Test Plan

  • sessions::tests::a_steady_state_request_and_save_read_no_index_bytes: attach, first save, usage appends, transcript appends and rewrites, and a manifest save read 0 index bytes; after a delete mid-turn, the usage append, transcript save, and manifest save are all dropped without reading the index and no file comes back.
  • sessions::tests::a_turn_start_rereads_an_owned_transcript_only_after_it_changed: repeated attach on an unchanged owned transcript reads it 0 times; an outside damaging write is refused with "damaged at line 4"; a repaired file is read once and then trusted again.
  • sessions::tests::a_turn_start_rereads_a_transcript_written_from_outside_before_an_append: an append after outside damage reads the transcript 0 times, and the next attach still reads it once and refuses with "damaged at line 4".
  • sessions::tests::a_turn_start_rereads_a_transcript_edited_with_its_modification_time_restored (unix): a same-length damaging edit made on a later tick of the file system's clock, with its modification time put back, is refused at the next turn start with "damaged at line 2".
  • sessions::tests::a_project_lists_its_sessions_from_any_spelling_of_its_path (unix): sessions recorded under the resolved path list, newest first, and latest finds the newest, from the direct, symlinked, and resolved spellings.
  • sessions::tests::a_symlink_that_takes_a_projects_place_does_not_carry_its_history (unix): after a project moves and a symlink to a second project takes its place, the second project lists only its own session from its direct and resolved spellings, and latest returns it.
  • cli::a_symlink_that_takes_a_projects_place_does_not_carry_its_history (unix, end to end): in the same setup, --recall --json run from the second project scans 0 sessions and returns no hits, and --continue --stdio exits 2 with no prior session, without contacting the model endpoint.
  • sessions::tests::a_relative_project_path_is_no_other_projects_history: a session stored under . is not listed from the absolute working directory, an absolute-path session stays the latest, and list(".") still finds the . entry.
  • The steady-state, only-after-it-changed, and spelling tests fail on main and pass with the change. The others pin behavior main already has, since main reparsed the transcript at every turn start and never resolved a path at lookup: each fails if the stamp drops the status-change time, an append vouches for a file written from outside, or lookup resolves stored paths.
  • cargo test --workspace --locked: exit 0.
  • cargo +1.97.0 clippy --workspace --all-targets --locked -- -D warnings: exit 0.

RetriggerConfidence Score: 4/5

The outstanding loss of access to moved-project history should be fixed before merging.

Findings

  1. P1 Moved project history disappears ▶
  2. P2 Unchanged stamp skips validation ▶
Fix with agent prompt
### Issue 1
crates/core/src/sessions.rs:undefined-1109
If a project directory moves and its old path is symlinked to the new location, existing sessions still record the old path. This filter compares that path without resolving it, so opening the project at its new location leaves `--continue` with no latest session and hides those sessions from `/resume` and `--recall`. Sessions recorded under a symlink spelling are hidden too. Preserve access to that history without allowing a retargeted symlink to expose it to another project.

### Issue 2
crates/core/src/sessions.rs:183-185
If an outside edit damages an owned transcript without changing its length or modification time, the next turn start skips parsing it. A later save can append after the unreadable record, increasing the cost of recovering that session’s history. This is a non-blocking concern.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary

The latest changes explicitly release session file locks and add transcript compatibility coverage. Session lookup prevents a retargeted symlink from exposing another project's history, but it still cannot find sessions recorded before a project moved to a new path.

Reviews (3) · Last reviewed commit: "fix(core): reread a transcript edited wi..."

Comment thread crates/core/src/sessions.rs Outdated
Comment on lines +174 to +176
let seen = stamp(&messages_path(core, id));
if seen.is_some() && memo.transcripts.get(id) == seen.as_ref() {
return Ok(());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unchanged stamp skips validation

If an outside edit damages an owned transcript without changing its length or modification time, the next turn start skips parsing it. A later save can append after the unreadable record, increasing the cost of recovering that session’s history. This is a non-blocking concern.

Knowledge Base Used: Sessions and runtime state

Artifacts

Evidence from the check

  • This authored executable source damages a transcript without changing its length, varies its mtime, and exercises attach, direct parsing, and append; it reproduces the reported distinction.

Command output from the check

  • The captured rustc command compiled the harness against the repository's core library with exit code 0; the executable was ready for both runs.

Command output from the check

  • The captured changed-mtime run exited 0 and shows both attach and direct parsing reporting damage at line 2; a different stamp triggers validation.

Command output from the check

  • The captured restored-mtime run exited 0 and shows attach succeeding despite a direct parse error, followed by an append that retains the damaged prefix; the matching stamp bypasses validation.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/core/src/sessions.rs
Line: 174-176

Comment:
**Unchanged stamp skips validation**

If an outside edit damages an owned transcript without changing its length or modification time, the next turn start skips parsing it. A later save can append after the unreadable record, increasing the cost of recovering that session’s history. This is a non-blocking concern.

**Knowledge Base Used:** [Sessions and runtime state](https://app.greptile.com/trace-labs/-/custom-context/knowledge-base/max17190/open-max/-/docs/sessions-and-runtime-state.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

.into_iter()
.enumerate()
.filter(|(_, m)| m.project == project)
.filter(|(_, m)| m.project == project || Path::new(&m.project) == wanted.as_path())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Moved project history disappears

If a project directory moves and its old path is symlinked to the new location, existing sessions still record the old path. This filter compares that path without resolving it, so opening the project at its new location leaves --continue with no latest session and hides those sessions from /resume and --recall. Sessions recorded under a symlink spelling are hidden too. Preserve access to that history without allowing a retargeted symlink to expose it to another project.

Knowledge Base Used: Sessions and runtime state

Artifacts

Executed Rust source for moved-project session lookup

  • The authored probe creates sessions under two historical path spellings, moves the project, and checks listing and latest-session results from its new path, providing the exact executed source.

Command used to build and run each revision

  • The authored command builds the selected revision and executes the same Rust probe against its core library, making the comparison reproducible.

Parent revision finds both historical sessions

  • Running the probe against the parent SHA returned both stored sessions and a latest session from the moved project's real path, establishing the prior behavior.

Current revision finds no historical sessions

  • Running the same probe against the current checkout returned an empty list and no latest session from the moved project's real path, confirming the regression.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/core/src/sessions.rs
Line: 1074

Comment:
**Moved project history disappears**

If a project directory moves and its old path is symlinked to the new location, existing sessions still record the old path. This filter compares that path without resolving it, so opening the project at its new location leaves `--continue` with no latest session and hides those sessions from `/resume` and `--recall`. Sessions recorded under a symlink spelling are hidden too. Preserve access to that history without allowing a retargeted symlink to expose it to another project.

**Knowledge Base Used:** [Sessions and runtime state](https://app.greptile.com/trace-labs/-/custom-context/knowledge-base/max17190/open-max/-/docs/sessions-and-runtime-state.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

…symlinks

Every model request and every transcript save read and parsed the whole
session index to learn whether their session still existed, and every turn
start reparsed the whole transcript, though the index lists every session
of every project and is never pruned: each request cost more with all
history ever recorded. Project lookups also compared stored paths byte for
byte, so a session recorded through a symlink, or before its project moved
and was linked back, was invisible to --continue, /resume, and --recall.

A claim now reads the index once and remembers, under the store lock, that
its session is indexed. No other process can remove a session this one
holds, since every removal claims first, and this process's own removal
records the session as gone, so a late write is still dropped without
asking the index. A session claimed while the index was damaged or did
not list it still asks the index on every write, which keeps the loud
refusal for a damaged index. Each owned transcript's length and
modification time are remembered after every validation and write, and a
turn start reparses it only when they moved.

Session lookup resolves the stored and the requested path before comparing
them, once per distinct stored path; index.json is unchanged.
An append does not read the history it extends, so it may record the
transcript as known only when nothing else wrote the file since this
process last did. No test held that check: without it, damage written from
outside during a turn and then appended past by the turn's own save would
be trusted by the next turn start instead of refused there.

The comments that said moving the index aside under a running session
turns its later saves into silent no-ops now describe what happens: a
session claimed while the index was healthy keeps saving history no lookup
lists, and one claimed over the damage, or claimed again later, has its
saves dropped. Closing every openmax before the move stays the advice.
A frontend that cannot read its working directory stores the project as
".". Resolving every stored path to find a project's sessions resolved
that "." against the reader's directory, so one stray entry was listed
under every project: --continue in a project with no history resumed it,
and the turns saved to it then surfaced in every other project.

Only an absolute stored path is resolved now; a relative one matches only
the same spelling, as before.
…ts place

Session lookup resolved every stored project path again each time it ran.
When a project directory moved and a symlink to another project took its
place, the stored path resolved to that other project, so running there
listed the first project's sessions: `--recall` returned their text,
`--continue` resumed the wrong transcript, and `/resume` offered it.

Frontends store the working directory as the OS reports it, already
resolved, which is the form trust keys a project by. Lookup now resolves
only the requested path and compares it with each stored path as
recorded, so a lookup spelled through a symlink still finds the
directory's sessions, while a path that later leads elsewhere carries no
history with it, matching how trust treats the same move.
…back

A turn start skips reparsing an owned transcript whose length and
modification time match what this process last validated or wrote. An
outside edit that kept the length and restored the modification time, as
a copy or sync that preserves timestamps does, was therefore trusted, and
a later append landed after the damaged record.

The stamp now also records the status-change time, which every write and
every timestamp change moves and no caller can set, so such an edit is
read and refused at the next turn start.
@Max17190
Max17190 force-pushed the keep-the-session-index-off-the-hot-path branch from 376a04e to b66d9d8 Compare October 6, 2026 03:58
@Max17190
Max17190 merged commit 9293fc5 into main Oct 6, 2026
16 checks passed
@Max17190
Max17190 deleted the keep-the-session-index-off-the-hot-path branch October 10, 2026 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant