Skip to content

fix: archive role-ordering reset transcripts#97544

Merged
vincentkoc merged 2 commits into
openclaw:mainfrom
yungchentang:codex/fix-97529-session-reset-archive
Jun 28, 2026
Merged

fix: archive role-ordering reset transcripts#97544
vincentkoc merged 2 commits into
openclaw:mainfrom
yungchentang:codex/fix-97529-session-reset-archive

Conversation

@yungchentang

@yungchentang yungchentang commented Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #97529

AI-assisted by Codex.

What Problem This Solves

Fixes an issue where users whose sessions hit an automatic role-ordering-conflict reset would silently lose the previous session transcript. The reset replayed only a bounded message tail into the new session, then hard-deleted the full previous transcript instead of preserving it as a reset archive.

Why This Change Was Made

The role-ordering reset lifecycle now uses the existing reset transcript archive path instead of the unlink-only cleanup path. This keeps the behavior aligned with adjacent reset and rollover lifecycle flows while preserving the bounded replay into the new session.

User Impact

Users retain a recoverable .jsonl.reset.<timestamp> archive for the full pre-reset transcript, so long conversations are not silently reduced to the replayed tail after this automatic recovery path runs.

Evidence

  • node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts
    • Passed: 1 test file, 58 tests.
  • node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json src/config/sessions/session-accessor.test.ts src/config/sessions/session-accessor.ts
    • Passed with no output.
  • git diff --check
    • Passed with no output.
  • git diff --numstat origin/main...HEAD
    • src/config/sessions/session-accessor.test.ts: 11 1
    • src/config/sessions/session-accessor.ts: 5 31
  • Branch-level real behavior proof on the fixed branch, driving the real exported persistSessionResetLifecycle from src/config/sessions/session-accessor.ts against an actual file-backed agents/main/sessions/ layout:
PR #97544 behavior proof: role-ordering reset archive path
workdir: /private/tmp/openclaw-97529-session-reset
tempRoot: /var/folders/ms/9qxgldzs4dd9f7hv74vrgxfh0000gn/T/openclaw-pr-97544-proof-TnZ4Gg
previous file exists after reset: false
reset archive files: 1 ["aaaaaaaa-1111-2222-3333-prevprevprev.jsonl.reset.2026-06-28T18-07-17.495Z"]
archive preserved previous session header: true
archive preserved user messages: 25
new transcript exists: true
replayed records returned: 6
new transcript replayed user messages: 3
new transcript includes newest user tail: true
  • .agents/skills/autoreview/scripts/autoreview --mode local --prompt 'Review the fix for GitHub issue #97529. Scope: preserve previous session transcript during role-ordering-conflict reset by archiving instead of unlinking, with focused session-accessor regression test. Focus on data-loss/session lifecycle regressions and whether proof covers the touched surface.'
    • Clean: no accepted/actionable findings reported.

No live provider reproduction was run. The real behavior proof drives the fixed file-backed session lifecycle directly: it writes a 25-turn previous transcript in the real sessions directory shape, runs the reset lifecycle with cleanup enabled, verifies the previous transcript is preserved as exactly one .jsonl.reset.<timestamp> archive, and verifies the replayed tail still lands in the new transcript.

@clawsweeper

clawsweeper Bot commented Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 28, 2026, 2:20 PM ET / 18:20 UTC.

Summary
This PR replaces role-ordering reset transcript cleanup with the existing reset archive flow and updates session accessor regression coverage.

PR surface: Source -26, Tests +10. Total -16 across 2 files.

Reproducibility: yes. at source level: current main routes role-ordering conflicts into cleanupPreviousResetTranscripts, which unlinks transcript candidates after bounded replay. I did not run a live provider-triggered reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Reset cleanup behavior: 1 unlink-only path replaced with archive. This is the session-retention behavior change maintainers should notice before merge.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/session-accessor.test.ts, serialized state: src/config/sessions/session-accessor.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #97529
Summary: This PR is the candidate fix for the canonical role-ordering reset unlink-without-archive issue; broader transcript archive policy and stalled-session retry work are related but not substitutes.

Members:

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

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] This intentionally changes persistent reset artifacts: role-ordering conflict recovery will now retain a .jsonl.reset archive instead of removing the previous transcript.
  • [P1] The PR body proves the file-backed lifecycle directly, but it does not include a full live provider-triggered role-ordering error run.

Maintainer options:

  1. Land with the archive contract (recommended)
    Accept that role-ordering reset recovery should retain the same reset archive artifact as adjacent reset flows once normal merge gates are satisfied.
  2. Ask for live trigger proof
    Require one provider-triggered role-ordering reset proof if maintainers want end-to-end assurance beyond the exercised file-backed lifecycle boundary.
  3. Defer to broader archive policy
    Pause this PR only if maintainers want this concrete fix folded into the broader transcript archive naming and retention decision tracked separately.

Next step before merge

  • No automated repair is needed; maintainers should review the intentional session artifact change and normal merge gates before landing.

Security
Cleared: The diff only changes session transcript archival behavior and a focused regression test; it does not touch dependencies, workflows, secrets, permissions, downloads, or supply-chain surfaces.

Review details

Best possible solution:

Land the narrow archive-based storage-boundary fix after maintainer review accepts the session-state artifact behavior and required CI is acceptable.

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

Yes, at source level: current main routes role-ordering conflicts into cleanupPreviousResetTranscripts, which unlinks transcript candidates after bounded replay. I did not run a live provider-triggered reproduction in this read-only review.

Is this the best way to solve the issue?

Yes. Reusing archivePreviousSessionTranscript is the narrowest maintainable fix because it preserves the existing reset archive contract instead of creating a parallel cleanup path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 session-state: Merging this changes persistent transcript lifecycle behavior for role-ordering resets and could affect recovery artifacts if wrong.

Label justifications:

  • P1: The PR fixes a source-proven automatic reset path that can silently lose previous session transcript data.
  • merge-risk: 🚨 session-state: Merging this changes persistent transcript lifecycle behavior for role-ordering resets and could affect recovery artifacts if wrong.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a real file-backed sessions layout showing one reset archive, preserved previous transcript content, and replay into the new transcript.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a real file-backed sessions layout showing one reset archive, preserved previous transcript content, and replay into the new transcript.
Evidence reviewed

PR surface:

Source -26, Tests +10. Total -16 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 31 -26
Tests 1 11 1 +10
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 16 32 -16

What I checked:

Likely related people:

  • steipete: Commit 6fa4376 added role-ordering reset transcript deletion, and commit bcd61f0 added the filesystem archive helpers reused by this PR. (role: introduced behavior and archive-runtime contributor; confidence: high; commits: 6fa437613b5e, bcd61f0a382d; files: src/auto-reply/reply/agent-runner.ts, src/gateway/session-transcript-files.fs.ts)
  • jalehman: Merged PR refactor(auto-reply): add lifecycle storage seams #93685 moved reset persistence, replay, and cleanup into persistSessionResetLifecycle, the seam touched here. (role: recent session lifecycle seam contributor; confidence: high; commits: 49e6f5a524bc; files: src/config/sessions/session-accessor.ts, src/auto-reply/reply/agent-runner-session-reset.ts, src/config/sessions/transcript-replay.ts)
  • Neerav Makwana: Commit 3de5476 added bounded transcript-tail replay for silent session rotations, which determines how much survives when the old transcript is not archived. (role: adjacent reset continuity contributor; confidence: medium; commits: 3de5476f5145; files: src/auto-reply/reply/agent-runner-session-reset.ts, src/config/sessions/transcript-replay.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. labels Jun 28, 2026
@yungchentang

yungchentang commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 28, 2026
@harjothkhara

harjothkhara commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

CI triage note after the current-head re-review: I checked the only red current check, checks-node-compact-large-1 (job 83929918721). It fails in src/agents/agent-bundle-mcp-runtime.test.ts:2424:

disposeSession timeout > retires timed-out shared MCP sessions before later catalog retries

with AssertionError: expected true not to be true.

That shard is agents-core; the files touched here are src/config/sessions/session-accessor.ts and src/config/sessions/session-accessor.test.ts. The session-specific gates are green: check-session-accessor-boundary, check-session-transcript-reader-boundary, check-lint, check-test-types, and the PR Real behavior proof. I do not see this failure as evidence against the reset-archive change unless it reproduces in a retry on the same agent runtime test.

@yungchentang

yungchentang commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for checking. Could a maintainer rerun checks-node-compact-large-1? I also think it looks unrelated to this PR since the touched session checks are green.

@vincentkoc
vincentkoc merged commit fcff01c into openclaw:main Jun 28, 2026
184 of 192 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 28, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jun 28, 2026
Closed
7 tasks
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 29, 2026
@yungchentang
fix: archive role-ordering reset transcripts (openclaw#97544)
71404f8 
* fix: archive role-ordering reset transcripts

* test: fix session reset archive lint
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@yungchentang @QiuYuang
fix: archive role-ordering reset transcripts (openclaw#97544)
7ed8646 
* fix: archive role-ordering reset transcripts

* test: fix session reset archive lint
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@yungchentang chenyangjun-xy
fix: archive role-ordering reset transcripts (openclaw#97544)
fddb63f 
* fix: archive role-ordering reset transcripts

* test: fix session reset archive lint
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@yungchentang @chenyangjun-xy
fix: archive role-ordering reset transcripts (openclaw#97544)
d102807 
* fix: archive role-ordering reset transcripts

* test: fix session reset archive lint
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

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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Role-ordering-conflict auto-reset hard-deletes the previous session transcript with no archive (fs.unlinkSync), keeping only the replayed tail

3 participants

@yungchentang @harjothkhara @vincentkoc