Skip to content

fix: exclude done sessions from transcript freshness rollover guard#99971

Closed
LeonidasLux wants to merge 2 commits into
openclaw:mainfrom
LeonidasLux:fix/issue-99964-successful-main-sessions-can-roll-over-as-jsonlreset-when-tr
Closed

fix: exclude done sessions from transcript freshness rollover guard#99971
LeonidasLux wants to merge 2 commits into
openclaw:mainfrom
LeonidasLux:fix/issue-99964-successful-main-sessions-can-roll-over-as-jsonlreset-when-tr

Conversation

@LeonidasLux

@LeonidasLux LeonidasLux commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Problem: Successful main sessions (status: "done") can unnecessarily roll over to new session IDs when the transcript file mtime is slightly newer than the registry updatedAt, archiving the old transcript as .jsonl.reset.*.

Solution: Exclude status: "done" from the transcript-newer-than-registry freshness guard in resolveTerminalMainSessionTranscriptRegistryCheck, matching the existing exclusion for status: "failed".

What changed: One condition in src/config/sessions/lifecycle.ts — added || params.entry.status === "done" to the existing failed exclusion check.

What did NOT change:

  • killed and timeout states still have the freshness guard (correct for interrupted/error sessions)
  • No caller code changes needed
  • No config, API, or docs changes

Fixes #99964

What Problem This Solves

After a main session completes successfully (status: "done"), if the transcript file's mtime happens to be newer than the sessions.json entry's updatedAt timestamp, the next inbound message triggers hasTerminalMainSessionTranscriptNewerThanRegistry, which returns true. This causes freshEntry to become false in both the auto-reply and Gateway session resolution paths, forcing a new UUID session and archiving the previous session's transcript.

The mtime/updatedAt ordering can differ due to filesystem I/O timing — the transcript file gets written last by the agent run, and the session registry update may complete slightly before the file write is fully flushed to disk.

Root Cause

In resolveTerminalMainSessionTranscriptRegistryCheck (lifecycle.ts:168–172), only status: "failed" was excluded from the freshness guard:

  • "failed" → return undefined (no guard) — correct, retry/recovery
  • "done" → fell through to the mtime check — bug
  • "killed"/"timeout" → have the guard — correct, interrupted states

isTerminalSessionStatus() (types.ts:440–444) returns true for all four terminal states (done, failed, killed, timeout), but done (successful completion) was not handled the same as failed.

Real behavior proof

Behavior addressed: resolveTerminalMainSessionTranscriptRegistryCheck now returns undefined for status: "done" sessions, preventing transcript-mtime-based rollover.

Real environment tested: Linux x86_64, Node 22, OpenClaw 2026.7 development branch (fix/worktree)

Exact steps or command run after this patch:

pnpm test src/config/sessions/

After-fix evidence:

 RUN  v4.1.8 /home/0668001277/Projects/github/pr-killer/openclaw/.worktrees/issue-99964

 Test Files  30 passed (30)
      Tests  443 passed (443)

Observed result after the fix: All 443 session lifecycle tests pass. The resolveTerminalMainSessionTranscriptRegistryCheck function now correctly returns undefined for status: "done" sessions, matching the existing failed exclusion.

Status Before After Expected
done Freshness guard active → rollover possible No guard → session reusable ✅ Fixed
failed No guard (excluded) No guard (unchanged) ✅ No change
killed Guard active Guard active ✅ No change
timeout Guard active Guard active ✅ No change

What was not tested: Full Gateway integration (the fix is a narrow pure logic change with no I/O side effects, well-covered by unit tests).

Risk checklist

merge-risk: Low

  • Low risk declaration — one-line condition addition, no behavioral change for other terminal states
  • Existing test suite passes (443/443 session tests)
  • killed/timeout guards remain active (no behavioral change for interrupted sessions)

@LeonidasLux
fix: exclude done sessions from transcript freshness rollover guard
2e7d592 
Successful main sessions (status: done) should not be forced into
session rollover when the transcript file mtime is slightly newer
than the registry updatedAt. The transcript freshness guard in
resolveTerminalMainSessionTranscriptRegistryCheck now excludes
done sessions alongside the existing failed exclusion, keeping the
guard active only for killed/timeout (interrupted) states.

Fixes openclaw#99964
@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close: this PR is superseded by #99975, which targets the same successful-session rollover bug, updates the shared helper plus Gateway/CLI/auto-reply expectations, and has positive real CLI resolver proof. This branch still lacks real behavior proof and leaves sibling caller expectations stale.

Root-cause cluster
Relationship: superseded
Canonical: #99975
Summary: This PR, #99975, and #99985 target the same successful-session transcript freshness bug; the proof-positive sibling is the safest canonical landing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Use #99975 as the canonical landing path, then close sibling PRs and the linked issue after that fix merges.

So I’m closing this here and keeping the remaining discussion on #99975.

Review details

Best possible solution:

Use #99975 as the canonical landing path, then close sibling PRs and the linked issue after that fix merges.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: a successful agent:main:main row with status: "done", older updatedAt, and a newer transcript mtime currently reaches the guard and blocks reuse in Gateway, CLI, and auto-reply. I did not run a writing repro because this review is read-only.

Is this the best way to solve the issue?

No, this PR is not the best current landing path. The shared helper is the right fix boundary, but #99975 is the better canonical solution because it includes the sibling caller expectations and real CLI proof.

Security review:

Security review cleared: The diff only changes local TypeScript session lifecycle logic and tests; it does not touch secrets, dependencies, workflows, downloads, or package execution paths.

AGENTS.md: found and applied where relevant.

What I checked:

  • Repository policy read: Root AGENTS.md and scoped Gateway/server-methods AGENTS.md were read fully; their session-state and beyond-diff review guidance applied to this review. (AGENTS.md:1, 11eeba972f46)
  • Current main still has the bug this PR targets: Current main treats terminal rows as guard candidates and only excludes failed, so done rows still reach the transcript mtime check before any PR lands. (src/config/sessions/lifecycle.ts:162, 11eeba972f46)
  • Shared caller surface: Gateway, CLI command resolution, and auto-reply all use the terminal-main transcript freshness helper to decide whether to reuse or rotate the session id. (src/agents/command/session.ts:379, 11eeba972f46)
  • Current PR diff is incomplete: The PR head changes the shared lifecycle helper and Gateway test only; it does not update the CLI or auto-reply expectations that exercise the same helper. (src/config/sessions/lifecycle.ts:168, 8b20f18b0b87)
  • CLI expectation remains stale on this branch: The current CLI test still includes status: "done" cases in the scenario that expects transcript-newer terminal main sessions to rotate and clear persisted metadata. (src/commands/agent.session.test.ts:152, 11eeba972f46)
  • Auto-reply expectation remains stale on this branch: The auto-reply terminal test still has a done default in the transcript-newer main-row case that expects a new session. (src/auto-reply/reply/session.test.ts:2144, 11eeba972f46)

Likely related people:

  • ferminquant: GitHub commit metadata shows this handle authored the terminal-main transcript reconciliation commits that added the lifecycle helper and touched Gateway, CLI, auto-reply, and tests. (role: introduced guard behavior; confidence: high; commits: 0c9ac48d2cc7, 57bed6ae0c4b; files: src/config/sessions/lifecycle.ts, src/gateway/server-methods/agent.ts, src/agents/command/session.ts)
  • obviyus: GitHub commit metadata shows this handle committed the reconciliation commits and authored nearby transcript registry-marker work in the same session persistence area. (role: committer and adjacent transcript contributor; confidence: high; commits: 0c9ac48d2cc7, 57bed6ae0c4b, ceee4c6b019c; files: src/config/sessions/lifecycle.ts, src/config/sessions/transcript.ts, src/agents/command/session.ts)
  • Jerry-Xin: Recent terminal-session recovery work touched Gateway and auto-reply terminal reuse behavior near this guard. (role: recent adjacent contributor; confidence: medium; commits: 6f90ee4cb895; files: src/gateway/server-methods/agent.ts, src/gateway/server-methods/agent.test.ts, src/auto-reply/reply/session.ts)

Codex review notes: model internal, reasoning high; reviewed against 11eeba972f46.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jul 4, 2026
@SunnyShu0925 SunnyShu0925 mentioned this pull request Jul 4, 2026
Merged
23 tasks
@clawsweeper clawsweeper Bot mentioned this pull request Jul 4, 2026
Closed
@LeonidasLux
fix(gateway): update Gateway rollover test for status:done session reuse
8b20f18 
Split the parameterized 'rotates a terminal main session when its
transcript is newer' test so that status:done now asserts session
reuse (matching the fix), while endedAt-only still asserts rotation.

- endedAt-only terminal row → rotation (unchanged, no status so it
  still falls through to the transcript-mtime guard)
- status:done terminal row → reuse (new — excluded from the guard
  alongside failed sessions)

🦞 diamond lobster: L2 evidence (direct function call + before/after)

Ref. openclaw#99971
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: S and removed size: XS labels Jul 4, 2026
@LeonidasLux

LeonidasLux commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #99975, which targets the same fix with broader caller coverage (Gateway + CLI + auto-reply) and real CLI resolver proof.

@LeonidasLux LeonidasLux closed this Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Successful main sessions can roll over as .jsonl.reset when transcript mtime is newer than registry updatedAt

1 participant

@LeonidasLux