fix(agents): persist user turn before attempt failures by TurboTheTurtle · Pull Request #86764 · openclaw/openclaw

TurboTheTurtle · 2026-05-26T05:10:39Z

Real behavior proof

Behavior addressed: Internal ACP/CLI session-effects turns now persist the accepted user message into the active internal attempt transcript before the external runner can fail, and later successful transcript persistence reuses that same internal file without duplicating the user row or leaking the internal prompt into the visible session transcript.
Real environment tested: Local OpenClaw runtime modules from /Users/andy/openclaw/openclaw-86592 on macOS, writing real temporary OpenClaw session JSONL files through the production transcript persistence helpers.
Exact steps or command run after this patch: node --import tsx - <<'EOF' ... EOF runtime smoke that imports persistUserTurnTranscript and persistAcpTurnTranscript, starts with a visible session transcript, routes the current internal turn through sessionFileOverride, then appends the assistant reply with userAlreadyPersisted: true.
Evidence after fix:

{
  "internalSessionFileEndsWith": "internal-agent-runs/run-proof.jsonl",
  "visibleSessionFileEndsWith": "visible-session.jsonl",
  "afterUserSessionFileIsInternal": true,
  "internalMessages": [
    "user:sanitized internal user prompt",
    "assistant:sanitized internal assistant reply"
  ],
  "visibleMessages": [
    "user:existing visible session prompt"
  ],
  "visibleContainsInternalPrompt": false,
  "internalUserMessageCount": 1
}

I also ran the targeted ClawSweeper regression suites after this patch:

Focused validation before the final current-main compatibility amendment:
- attempt/command/model-switch suites: 209 tests passed
- agent-runner/followup suites: 403 tests passed
- total: 612 focused tests passed

Exact-head GitHub CI on 4b60514d98: 68 checks passed with no failures. The final compatibility amendment updates test mock return metadata for current-main types and removes a redundant initializer; independent review found no behavioral change.

Observed result after fix: The pre-run user message lands in internal-agent-runs/run-proof.jsonl, the ACP assistant append uses the same internal transcript with userAlreadyPersisted: true, the visible transcript retains only its original visible prompt, and the internal user message appears exactly once.
What was not tested: I did not launch a live external Claude/Codex CLI subprocess or a live ACP backend; this was a local OpenClaw runtime transcript smoke using real session persistence code, supplemented by targeted regression tests.

Summary

add a shared user-only transcript persistence helper
persist ACP and CLI user turns before the runner can throw
route internal session-effects persistence to the active internal transcript
skip duplicate user-row mirroring when the current turn was already persisted

Tests

node --import tsx - <<'EOF' ... EOF local OpenClaw runtime transcript smoke
node scripts/run-vitest.mjs src/agents/command/attempt-execution.cli.test.ts src/commands/agent.test.ts src/agents/agent-command.live-model-switch.test.ts
node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-execution.test.ts src/auto-reply/reply/followup-runner.test.ts

Fixes #86592

AI-assisted: Codex was used to rebase this PR, resolve current-main conflicts, run focused checks, and perform independent review.

clawsweeper · 2026-05-26T05:13:04Z

Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 10:01 PM ET / July 16, 2026, 02:01 UTC.

Summary
The PR persists accepted CLI and ACP user turns before runner failures, routes internal session-effects writes to the active internal transcript, and prevents duplicate user rows after successful attempts.

PR surface: Source +106, Tests +273. Total +379 across 4 files.

Reproducibility: yes. at source level: current success-path persistence occurs after the attempt, so a thrown CLI or ACP attempt exits before the user row is written; the linked issue and branch tests exercise that boundary. I did not independently run the failing current-main scenario during this read-only review.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/agents/command/attempt-execution.cli.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #86592
Summary: The open issue is the canonical bug report and this PR is its active candidate fix; the earlier overlapping CLI-only PR closed unmerged.

Members:

canonical: Inbound user messages are not persisted to session JSONL when the agent attempt throws #86592 - It documents the same post-attempt persistence ordering and is explicitly linked for closure by this PR.
superseded: fix(agents): persist CLI user turns before attempts #86624 - It proposed an earlier overlapping CLI-focused implementation but closed unmerged; this branch covers both CLI and ACP paths.
partial_overlap: Session transcript integrity: .jsonl silently drops early user messages; .trajectory.jsonl FIFO window makes neither file independently authoritative #104323 - It concerns broader transcript-authority integrity and early-message loss, but does not replace the specific attempt-failure fix tracked here.

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

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

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

Rank-up moves:

Rebase onto current main, resolve the dirty merge without weakening transcript isolation or deduplication, and refresh exact-head runtime proof and focused tests.

Risk before merge

[P1] The patch intentionally changes transcript semantics from success-only persistence to durable persistence once a turn is accepted; failed runs will now leave unmatched user rows that replay and compaction treat as authoritative.
[P1] Internal session-effects prompts must always resolve to the internal attempt transcript; a merge-resolution error in this routing could expose internal prompt content in the visible session history.
[P1] The current dirty merge state means the supplied proof and green checks do not yet validate the actual three-way merge result against current main.

Maintainer options:

Rebase and preserve the invariant (recommended)
Resolve the current merge conflict without changing exactly-once accepted-turn persistence or internal transcript isolation, then refresh proof on the resulting head.
Pause for a persistence redesign
Do not land the branch if maintainers do not want failed accepted turns to become authoritative transcript history.

Next step before merge

[P2] A maintainer should accept or reject the pre-run durability boundary and review the rebased merge result; there is no narrow automated defect to repair on the current patch.

Maintainer decision needed

Question: Should accepted CLI and ACP turns become durable transcript state before the external runner starts, including internal session-effects turns written only to their internal transcript?
Rationale: This timing change fixes real transcript loss but defines when a user turn becomes authoritative state and affects replay, compaction, recovery, and internal-prompt isolation; that durable semantic boundary needs explicit owner acceptance.
Likely owner: openperf — Their recent merged work protects session identity and delayed follow-up state across agent lifecycle boundaries, making them the strongest available routing candidate for this persistence decision.
Options:
- Adopt accepted-turn persistence (recommended): Rebase the branch while preserving exactly-once pre-run persistence and internal transcript isolation, then rerun focused and runtime proof on the resolved head.
- Retain success-only persistence: Pause this PR and design a different recovery mechanism that prevents dropped turns without making failed accepted turns durable transcript entries.

Security
Cleared: The diff adds no dependency, permission, credential, network, or supply-chain surface, and its internal-prompt isolation has focused tests and runtime-file proof.

Review details

Best possible solution:

Preserve one canonical shared recorder that writes an accepted turn exactly once to the correct transcript before runtime invocation, then rebase cleanly and prove failed, successful, empty-reply, fallback, rebound, and internal-session cases on the resolved head.

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

Yes at source level: current success-path persistence occurs after the attempt, so a thrown CLI or ACP attempt exits before the user row is written; the linked issue and branch tests exercise that boundary. I did not independently run the failing current-main scenario during this read-only review.

Is this the best way to solve the issue?

Yes in principle: a shared exactly-once recorder before runtime invocation is narrower and safer than duplicating failure-specific persistence or writing synthetic assistant errors. Merge should wait for owner acceptance of the durable timing and refreshed proof after conflict resolution.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P1: The bug can silently lose accepted user turns and wedge session replay or compaction after repeated attempt failures.
merge-risk: 🚨 session-state: Merging changes when user turns become durable and which transcript owns them, directly affecting replay, compaction, recovery, and deduplication.
merge-risk: 🚨 security-boundary: The changed internal-session routing must prevent internal prompts from being persisted into the user-visible session transcript.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR provides after-fix live output from production transcript helpers writing real temporary session files and demonstrates exactly-once internal persistence without visible-session leakage; a live external ACP or CLI subprocess was not exercised.
proof: sufficient: Contributor real behavior proof is sufficient. The PR provides after-fix live output from production transcript helpers writing real temporary session files and demonstrates exactly-once internal persistence without visible-session leakage; a live external ACP or CLI subprocess was not exercised.

Evidence reviewed

PR surface:

Source +106, Tests +273. Total +379 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	158	52	+106
Tests	2	273	0	+273
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	431	52	+379

What I checked:

Pre-attempt persistence: The branch prepares the ACP transcript target and shared user-turn recorder before invoking the external runtime, so an accepted turn can survive an attempt failure. (src/agents/agent-command.ts:1174, 4b60514d98f7)
Post-attempt deduplication: The transcript persistence path adds explicit handling for an already-persisted user turn and updates the session entry when a successful ACP turn produces no visible reply. (src/agents/command/attempt-execution.ts:152, 4b60514d98f7)
CLI and ACP regression coverage: The branch tests failed attempts, internal transcript routing, exactly-once user persistence, assistant-only mirroring, and empty-reply session touching. (src/commands/agent.test.ts:1035, 4b60514d98f7)
Focused persistence coverage: The attempt-execution tests verify that ACP persistence skips a duplicate user row when the shared recorder already owns it. (src/agents/command/attempt-execution.cli.test.ts:1576, 4b60514d98f7)
Real behavior proof: The PR body reports a production-helper runtime smoke writing real temporary transcript files: the internal transcript contains one user row and the assistant reply, while the visible transcript does not contain the internal prompt. (src/agents/agent-command.ts:1203, 4b60514d98f7)
Current merge state: GitHub reports the refreshed head as dirty against its base, so the reviewed patch cannot currently land without resolving the three-way merge and revalidating the resulting code. (4b60514d98f7)

Likely related people:

openperf: Authored the merged session-rebind protection that preserves session-instance integrity across delayed agent follow-ups, closely adjacent to this transcript ownership decision. (role: recent session-state contributor; confidence: medium; commits: 2ffbea20d2f6; files: src/agents/bash-tools.exec-approval-followup.ts, src/gateway/server-methods/agent.ts)
samzong: Authored the merged restart-recovery work that carries persisted session and delivery state across interrupted agent runs. (role: recent recovery-path contributor; confidence: medium; commits: 4decdf6245a1; files: src/agents/main-session-restart-recovery.ts, src/agents/agent-command.live-model-switch.test.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.

Review history (1 earlier review cycle)

reviewed 2026-06-24T18:25:46.617Z sha 3f7c036 :: needs maintainer review before merge. :: none

TurboTheTurtle · 2026-05-26T05:17:44Z

@clawsweeper re-review

clawsweeper · 2026-05-26T05:17:46Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26433802370
Updated: 2026-05-26T05:26:08.403Z

clawsweeper · 2026-05-26T05:26:01Z

ClawSweeper PR egg

✨ Hatched: ✨ glimmer Brave Proofling

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: ✨ glimmer.
Trait: sniffs out flaky tests.
Image traits: location green-check meadow; accessory green check lantern; palette moonlit blue and soft silver; mood sparkly; pose curling around a status light; shell glossy opal shell; lighting warm desk-lamp glow; background subtle branch markers.
Share on X: post this hatch
Copy: My PR egg hatched a ✨ glimmer Brave Proofling in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

TurboTheTurtle · 2026-05-26T05:52:04Z

@clawsweeper re-review

clawsweeper · 2026-05-26T05:52:07Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26434935263
Updated: 2026-05-26T06:00:21.285Z

TurboTheTurtle · 2026-06-05T01:32:57Z

@clawsweeper re-review

clawsweeper · 2026-06-05T01:33:00Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26989977662
Updated: 2026-06-05T01:41:56.093Z

openclaw-barnacle · 2026-07-14T04:23:08Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

TurboTheTurtle · 2026-07-14T23:24:21Z

Rebased onto current upstream/main (1b117408a2) at head 4b60514d98.

The focused transcript and runner suites passed 612 tests before the final replay. Exact-head CI then exposed two current-main contract updates: one accumulator mock now returns null, result metadata includes aborted: false, and a redundant boolean initializer was removed. The amendment passed independent review.

Exact-head GitHub CI is green: 68 checks passed with no failures.

@clawsweeper re-review

clawsweeper · 2026-07-14T23:24:23Z

🦞🧹
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 · 2026-07-14T23:34:18Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29473910503
Updated: 2026-07-16T05:29:43.808Z

openclaw-barnacle Bot added commands Command implementations agents Agent runtime and tooling size: M triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 26, 2026

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 26, 2026

openclaw-barnacle Bot added size: L and removed size: M proof: sufficient ClawSweeper judged the real behavior proof convincing. labels May 26, 2026

This comment was marked as spam.

Sign in to view

RomneyDa added the dependency-guard-backfill label May 29, 2026

clawsweeper Bot removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels May 29, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 5, 2026

TurboTheTurtle force-pushed the fix/persist-user-turn-on-attempt-failure-86592 branch from b98f590 to 762e7ad Compare June 8, 2026 02:52

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 8, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 8, 2026

TurboTheTurtle force-pushed the fix/persist-user-turn-on-attempt-failure-86592 branch from 762e7ad to b4229c0 Compare June 14, 2026 20:02

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026

clawsweeper Bot mentioned this pull request Jun 15, 2026

Inbound user messages are not persisted to session JSONL when the agent attempt throws #86592

Open

TurboTheTurtle force-pushed the fix/persist-user-turn-on-attempt-failure-86592 branch from b4229c0 to 3f7c036 Compare June 18, 2026 07:00

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 18, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 18, 2026

clawsweeper Bot mentioned this pull request Jul 11, 2026

Session transcript integrity: .jsonl silently drops early user messages; .trajectory.jsonl FIFO window makes neither file independently authoritative #104323

Closed

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 14, 2026

TurboTheTurtle force-pushed the fix/persist-user-turn-on-attempt-failure-86592 branch from 3f7c036 to 25c0d62 Compare July 14, 2026 23:24

openclaw-barnacle Bot added size: M and removed size: L labels Jul 14, 2026

fix(agents): persist user turn before attempt failures

4b60514

TurboTheTurtle force-pushed the fix/persist-user-turn-on-attempt-failure-86592 branch from 25c0d62 to 4b60514 Compare July 14, 2026 23:31

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jul 15, 2026

Uh oh!

Uh oh!

Conversation

TurboTheTurtle commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Real behavior proof

Summary

Tests

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TurboTheTurtle commented May 26, 2026

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hatch command

Uh oh!

TurboTheTurtle commented May 26, 2026

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as spam.

TurboTheTurtle commented Jun 5, 2026

Uh oh!

clawsweeper Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jul 14, 2026

Uh oh!

TurboTheTurtle commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TurboTheTurtle commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 5, 2026 •

edited

Loading

TurboTheTurtle commented Jul 14, 2026 •

edited

Loading

clawsweeper Bot commented Jul 14, 2026 •

edited

Loading

clawsweeper Bot commented Jul 14, 2026 •

edited

Loading