Skip to content

fix(sessions): drop dead-end orphan entries when retry forks parentId chain (#48810)#79635

Closed
jeffrey701 wants to merge 1 commit into
openclaw:mainfrom
jeffrey701:fix/compaction-retry-orphan-fork-48810
Closed

fix(sessions): drop dead-end orphan entries when retry forks parentId chain (#48810)#79635
jeffrey701 wants to merge 1 commit into
openclaw:mainfrom
jeffrey701:fix/compaction-retry-orphan-fork-48810

Conversation

@jeffrey701

@jeffrey701 jeffrey701 commented May 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Reported by @taohaowei in #48810: when compaction retries against a session JSONL file, the original (failed) compaction event and the successful retry both get appended under the same parentId. One side becomes a dead-end (no descendants), the other carries the rest of the transcript. Anything downstream that walks the causal chain via parentId (replay, dashboards, exports, audit tooling) breaks at the fork — it either picks the orphan and stops, or crashes on the ambiguity.

Root cause

src/agents/session-file-repair.ts already owns load-time repairs for several JSONL corruption shapes (truncated tail, malformed lines, missing session header, etc.). Compaction retry's same-parentId duplicate slipped through every existing branch because it isn't a parse error — both entries are individually well-formed, the corruption is structural: two compaction siblings sharing one parentId where exactly one of them has a non-empty subtree.

Fix

src/agents/session-file-repair.ts:

  1. New RepairReport field droppedOrphanForkEntries?: number so the existing buildRepairSummaryParts output stays consistent with how other repair counters (droppedMalformedLines, etc.) are reported.
  2. New helper readParentLinkedEntry(entry) parses one raw JSONL entry and returns { id, parentId, type } (or null for entries that don't participate in the parent graph — session headers, legacy entries without an id/parentId, etc.).
  3. New helper detectAndDropOrphanForkEntries(entries) builds a parent->children map across the whole entries array, computes subtree size for every node, and drops a sibling only when ALL of:
    • the entry is type: "compaction" (the canonical discriminator the source-side compaction writers use, per manual-compaction-boundary.ts and pi-embedded-runner-extraparams.ts), AND
    • it shares a parentId with at least one other compaction sibling, AND
    • the entry itself has zero descendants in the parent-linked graph, AND
    • exactly one of its compaction siblings under the same parentId has descendants (the retry winner).
  4. Wired into repairSessionFileIfNeeded between the existing parse step and the "did we change anything" check, so an orphan-only repair triggers the same write-back path as any other repair, and the summary string surfaces the new counter.

Why the compaction-only carve-out (clawsweeper #79635 [P2] response)

Initial implementation used a generic "any zero-descendant sibling next to a sibling-with-descendants" rule. The reviewer correctly flagged that a session JSONL is a tree, so a deliberate side branch can naturally be a leaf next to a continued branch — generic message leaves must not be deleted. Restricting drop candidates to type: "compaction" siblings of other type: "compaction" siblings is the documented seam: those are the entries the compaction retry path produces under the same parentId, and they're the only entries this repair touches. Generic non-compaction entries are now never dropped. A new regression test pins this exact carve-out.

Tests

src/agents/session-file-repair.test.ts adds 9 cases:

Real behavior proof

Behavior or issue addressed: #48810 — compaction retry creates orphan fork in parentId chain, breaking causal-order walkers downstream. After this patch, repairSessionFileIfNeeded detects compaction-vs-compaction same-parentId forks where exactly one compaction sibling has descendants and drops the dead-end loser(s), while leaving generic non-compaction leaves and deliberate two-branch transcripts untouched.

Real environment tested: Linux 6.17.0-23-generic, Node.js v22.22.0, this PR's branch fix/compaction-retry-orphan-fork-48810 at head 1583ec738b checked out into a real openclaw working tree. The proof script imports the actual repairSessionFileIfNeeded from src/agents/session-file-repair.ts (no module shim, no mock) and drives it against real on-disk JSONL files in a mkdtemp directory. The @openclaw/fs-safe workspace dependency was built locally from the pinned commit c7ccb99d3058f2acf2ad2758ad2470c7e113a53c and dropped into node_modules/@openclaw/fs-safe/dist/ so the production module loads end-to-end.

Exact steps or command run after this patch:

cd ~/Project/GittensorMilos/Jeff/openclaw
git checkout fix/compaction-retry-orphan-fork-48810   # head 1583ec738b
node ./node_modules/.bin/tsx .repro-79635.mts

The script writes 6 JSONL fixtures into mkdtemp(...), calls repairSessionFileIfNeeded({ sessionFile }) against each, then re-reads the file from disk and prints the before/after entry list (line by line) plus the captured debug callback output.

Evidence after fix:

Verbatim terminal capture (stdout + stderr combined) from the live run of node ./node_modules/.bin/tsx .repro-79635.mts against the patched repairSessionFileIfNeeded import. This is the actual console output, not unit test or vitest output.

[A_compaction_retry_orphan]
  repaired=true droppedOrphanForkEntries=1 expectedDropped=1 pass=true
  debug: session file repaired: dropped 1 orphan fork entry/entries (A_compaction_retry_orphan.jsonl)
  -- before (5 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-A                       parentId=(null)
    type=compaction  id=compact-orphan-A             parentId=root-A
    type=compaction  id=compact-winner-A             parentId=root-A
    type=message     id=after-compact-A              parentId=compact-winner-A
  -- after  (4 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-A                       parentId=(null)
    type=compaction  id=compact-winner-A             parentId=root-A
    type=message     id=after-compact-A              parentId=compact-winner-A

[B_p2_regression_generic_leaf_branch]
  repaired=false droppedOrphanForkEntries=0 expectedDropped=0 pass=true
  -- before (5 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-B                       parentId=(null)
    type=message     id=continued-B                  parentId=root-B
    type=message     id=continued-child-B            parentId=continued-B
    type=message     id=leaf-side-B                  parentId=root-B
  -- after  (5 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-B                       parentId=(null)
    type=message     id=continued-B                  parentId=root-B
    type=message     id=continued-child-B            parentId=continued-B
    type=message     id=leaf-side-B                  parentId=root-B

[C_compaction_leaf_with_message_continuation]
  repaired=false droppedOrphanForkEntries=0 expectedDropped=0 pass=true
  -- before (5 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-C                       parentId=(null)
    type=message     id=continued-C                  parentId=root-C
    type=message     id=continued-child-C            parentId=continued-C
    type=compaction  id=compaction-leaf-C            parentId=root-C
  -- after  (5 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-C                       parentId=(null)
    type=message     id=continued-C                  parentId=root-C
    type=message     id=continued-child-C            parentId=continued-C
    type=compaction  id=compaction-leaf-C            parentId=root-C

[D_mixed_siblings_only_compaction_loser_dropped]
  repaired=true droppedOrphanForkEntries=1 expectedDropped=1 pass=true
  debug: session file repaired: dropped 1 orphan fork entry/entries (D_mixed_siblings_only_compaction_loser_dropped.jsonl)
  -- before (6 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-D                       parentId=(null)
    type=compaction  id=mixed-compact-loser-D        parentId=root-D
    type=compaction  id=mixed-compact-winner-D       parentId=root-D
    type=message     id=winner-child-D               parentId=mixed-compact-winner-D
    type=message     id=unrelated-leaf-D             parentId=root-D
  -- after  (5 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-D                       parentId=(null)
    type=compaction  id=mixed-compact-winner-D       parentId=root-D
    type=message     id=winner-child-D               parentId=mixed-compact-winner-D
    type=message     id=unrelated-leaf-D             parentId=root-D

[E_nested_compaction_forks_one_pass]
  repaired=true droppedOrphanForkEntries=2 expectedDropped=2 pass=true
  debug: session file repaired: dropped 2 orphan fork entry/entries (E_nested_compaction_forks_one_pass.jsonl)
  -- before (8 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-E                       parentId=(null)
    type=compaction  id=first-orphan-E               parentId=root-E
    type=compaction  id=first-winner-E               parentId=root-E
    type=message     id=midway-E                     parentId=first-winner-E
    type=compaction  id=second-orphan-E              parentId=midway-E
    type=compaction  id=second-winner-E              parentId=midway-E
    type=message     id=tail-E                       parentId=second-winner-E
  -- after  (6 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=root-E                       parentId=(null)
    type=compaction  id=first-winner-E               parentId=root-E
    type=message     id=midway-E                     parentId=first-winner-E
    type=compaction  id=second-winner-E              parentId=midway-E
    type=message     id=tail-E                       parentId=second-winner-E

[F_linear_transcript_untouched]
  repaired=false droppedOrphanForkEntries=0 expectedDropped=0 pass=true
  -- before (4 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=linear-a-F                   parentId=(null)
    type=message     id=linear-b-F                   parentId=linear-a-F
    type=message     id=linear-c-F                   parentId=linear-b-F
  -- after  (4 lines) --
    type=session     id=session-79635                parentId=-
    type=message     id=linear-a-F                   parentId=(null)
    type=message     id=linear-b-F                   parentId=linear-a-F
    type=message     id=linear-c-F                   parentId=linear-b-F

=== Summary ===
  PASS: A_compaction_retry_orphan dropped=1 expected=1
  PASS: B_p2_regression_generic_leaf_branch dropped=0 expected=0
  PASS: C_compaction_leaf_with_message_continuation dropped=0 expected=0
  PASS: D_mixed_siblings_only_compaction_loser_dropped dropped=1 expected=1
  PASS: E_nested_compaction_forks_one_pass dropped=2 expected=2
  PASS: F_linear_transcript_untouched dropped=0 expected=0

ALL SCENARIOS PASSED via real repairSessionFileIfNeeded import

Observed result after fix: every scenario behaves as the design contract requires when run end-to-end against the real production module. Scenario A (the canonical #48810 compaction-retry fork) drops the dead-end loser and keeps the winner plus the post-compact message. Scenario B is the [P2] regression the bot asked for: a deliberate generic message leaf next to a continued sibling stays in the file. Scenario C confirms a type: "compaction" leaf is preserved when its only sibling is a non-compaction continued branch (no compaction winner exists, so nothing to reconcile). Scenario D verifies mixed siblings: the compaction loser is dropped while the non-compaction message leaf under the same parent is preserved untouched. Scenarios E and F cover nested forks and linear transcripts respectively.

What was not tested: a real end-to-end compaction retry against a live model provider that produces the duplicate compaction entry on disk in the first place was not run — that requires a live API key plus a transcript long enough to trigger compaction, which is outside the scope of a load-time-repair fix. The repair operates on the on-disk artifact regardless of which write-side path produced it, and the proof script reproduces every shape of that artifact verbatim, including the exact ordering and parentId linkage from the issue's reproduction notes. Preventing the duplicate at write time (in the compaction retry path itself) is also out of scope and tracked as a follow-up in the PR body.

Verification (supplemental to the proof above)

  • npx vitest run src/agents/session-file-repair.test.ts -> 60/60 passed locally on 1583ec738b.
  • npx vitest run src/gateway/session-utils.test.ts src/gateway/session-utils.fs.test.ts src/agents/session-file-repair.test.ts (the bot's stated acceptance criteria suites) -> 555/555 passed.
  • npx oxfmt --check --threads=1 src/agents/session-file-repair.ts src/agents/session-file-repair.test.ts CHANGELOG.md -> clean.

Out of scope (follow-ups)

  • Preventing the duplicate at write time (in the compaction retry path itself) — would need maintainer judgment on whether to mark the loser as superseded vs. drop pre-append vs. de-dup at flush. This PR fixes the corruption that's already on disk for affected users and leaves the write-side decision to a follow-up.

Fixes #48810.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 9, 2026
@clawsweeper

clawsweeper Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed July 1, 2026, 10:22 AM ET / 14:22 UTC.

Summary
The PR adds on-load session JSONL repair for dead-end same-parent type: "compaction" forks, regression tests, and a direct changelog entry.

PR surface: Source +190, Tests +468, Docs +1. Total +659 across 3 files.

Reproducibility: yes. at source level: the linked issue supplies a concrete JSONL artifact with two compactions sharing one parentId, current main has no same-parent compaction fork detector, and appendCompaction can write that parent shape. I did not run a live provider retry reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Persisted repair deletion path: 1 added. The patch adds a load-time path that removes selected existing JSONL entries, which is session-state upgrade policy rather than routine parser cleanup.

Stored data model
Persistent data-model change detected: migration/backfill/repair: CHANGELOG.md, serialized state: src/agents/session-file-repair.test.ts, serialized state: src/agents/session-file-repair.ts, unknown-data-model-change: src/agents/session-file-repair.test.ts, unknown-data-model-change: src/agents/session-file-repair.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #48810
Summary: This PR is the open candidate fix for the canonical same-parent compaction retry orphan-fork issue; the sessions_yield compaction report overlaps in session-state impact but has a distinct trigger and requested behavior.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • [P2] Refresh the branch over current main and add a regression for cached clean sessions that later receive a trusted compaction retry fork.
  • [P2] Ask maintainers to explicitly accept or reject the compaction-only persisted-entry deletion policy.

Risk before merge

  • [P1] The branch is currently conflicting and predates current main's validated transcript repair cache, so exact merged behavior cannot be accepted without a refresh.
  • [P1] A refreshed version would intentionally delete selected persisted compaction entries during repair, which needs explicit maintainer acceptance as session-state policy.
  • [P1] The supplied proof covers full-file repair fixtures, but not a cached clean session file that later receives the compaction retry fork through the trusted append path.

Maintainer options:

  1. Refresh and integrate the cache path (recommended)
    Rebase or rebuild the PR over current main so orphan-fork detection cannot be bypassed by tryIncrementalSessionRepair after trusted appends.
  2. Accept the deletion policy explicitly
    Maintainers can approve the narrow rule that only dead-end same-parent compaction siblings are removed when exactly one sibling carries descendants.
  3. Pause for write-side prevention instead
    If load-time deletion is not an acceptable repair policy, pause this PR and pursue a compaction retry write-side fix plus a separate migration decision.

Next step before merge

  • [P2] The next action is maintainer/author handling: refresh the conflicting branch, resolve the incremental-cache finding, and make the session-state deletion policy decision visible before merge.

Security
Cleared: The diff changes session repair code, tests, and changelog text only; it does not add dependencies, workflows, scripts, permissions, package metadata, or secret-handling paths.

Review findings

  • [P1] Integrate fork repair with the incremental cache path — src/agents/session-file-repair.ts:448-451
  • [P3] Remove the direct changelog edit — CHANGELOG.md:207
Review details

Best possible solution:

Refresh or rebuild the fix on current main so compaction-fork repair participates in full-file and trusted-append repair paths, remove the direct changelog edit, add cache-path regression coverage, and get maintainer sign-off on the compaction-only deletion rule.

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

Yes at source level: the linked issue supplies a concrete JSONL artifact with two compactions sharing one parentId, current main has no same-parent compaction fork detector, and appendCompaction can write that parent shape. I did not run a live provider retry reproduction in this read-only review.

Is this the best way to solve the issue?

No as submitted: load-time repair is the right owner boundary for already-corrupted files, but the branch must be refreshed over current main and integrated with the incremental repair cache before it is the best fix. Source-side duplicate prevention can remain a follow-up once maintainers accept the repair policy.

Full review comments:

  • [P1] Integrate fork repair with the incremental cache path — src/agents/session-file-repair.ts:448-451
    The new orphan-fork repair is only wired into the full-file path. On current main, a previously validated session can return early from tryIncrementalSessionRepair after parsing only the trusted appended suffix; a retry fork appended as valid JSONL can therefore bypass this detector once the session is warm. Refresh against current main and make this repair either force a full pass for same-parent compaction appends or participate in the incremental path.
    Confidence: 0.88
  • [P3] Remove the direct changelog edit — CHANGELOG.md:207
    Root policy marks CHANGELOG.md as release-owned and says normal PRs should keep release-note context in the PR body, squash message, or commit message instead. Please drop this entry so release generation remains the single changelog owner.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR addresses a session-chain integrity regression that can break replay, export, audit, and context reconstruction for real session files.
  • merge-risk: 🚨 session-state: Merging the repair could remove selected persisted session JSONL entries during load-time cleanup, so green CI alone does not settle the session-state policy risk.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes after-fix terminal output from a real on-disk JSONL proof script that imports the production repair module and shows before/after behavior for the relevant fixture shapes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a real on-disk JSONL proof script that imports the production repair module and shows before/after behavior for the relevant fixture shapes.
Evidence reviewed

PR surface:

Source +190, Tests +468, Docs +1. Total +659 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 192 2 +190
Tests 1 468 0 +468
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 661 2 +659

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/agents/session-file-repair.test.ts.
  • [P1] node scripts/run-vitest.mjs src/agents/sessions/session-manager.test.ts.

What I checked:

  • Repository policy read and applied: Root policy requires full AGENTS review, compatibility caution for session/auth/persisted state, best-fix review, and release-owned changelog handling; scoped agents policy was also read for the touched test area. (AGENTS.md:29, 8abd5d40712d)
  • Live PR state: Live GitHub reports the PR head e70433a is open, maintainer-editable, and currently dirty/conflicting against its base. (e70433a073ab)
  • Current-main incremental repair path: Current main can return from tryIncrementalSessionRepair after validating only the appended suffix of a cached clean session file, before the full-file repair path runs. (src/agents/session-file-repair.ts:714, 8abd5d40712d)
  • Current-main compaction append shape: SessionManager.appendCompaction writes a fresh type: "compaction" entry with parentId set to the current append parent, matching the same-parent duplicate shape when retry appends again from the same parent. (src/agents/sessions/session-manager.ts:2392, 8abd5d40712d)
  • Current-main absence of orphan-fork repair: Focused search found no current-main symbols for dropped orphan fork entries, compaction sibling repair, or same-parent parentId fork repair in the central session repair files. (src/agents/session-file-repair.ts:866, 8abd5d40712d)
  • PR repair insertion point: The PR adds orphan-fork detection in the full-file repairSessionFileIfNeeded path after parsing all entries, which does not by itself address current main's incremental early-return path. (src/agents/session-file-repair.ts:448, e70433a073ab)

Likely related people:

  • Alix-007: Authored recent validated transcript cache and incremental repair work touching session-file-repair.ts, session-file-repair.test.ts, session-manager.ts, and session-manager.test.ts, which the PR now needs to integrate with. (role: recent area contributor; confidence: high; commits: a0b16f37e835, 76038c3a625b; files: src/agents/session-file-repair.ts, src/agents/session-file-repair.test.ts, src/agents/sessions/session-manager.ts)
  • shakkernerd: Authored earlier hardening in the session-file repair module and tests, relevant to repair policy and compatibility review. (role: session repair contributor; confidence: medium; commits: e6fdac7bfb6d; files: src/agents/session-file-repair.ts, src/agents/session-file-repair.test.ts)
  • justinhuangai: Added malformed tool-call and session-file repair behavior in the same load-time repair module family. (role: session repair contributor; confidence: medium; commits: 0da6de6624a9; files: src/agents/session-file-repair.ts, src/agents/session-file-repair.test.ts)
  • mpz4life: Worked on compaction retry timeout handling adjacent to the retry lifecycle that can produce the reported duplicate compaction artifact. (role: adjacent compaction retry contributor; confidence: medium; commits: 6b255b4dec03; files: src/agents/pi-embedded-runner/run/compaction-retry-aggregate-timeout.ts, src/agents/pi-embedded-runner/run/compaction-retry-aggregate-timeout.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.

jeffrey701 added a commit to jeffrey701/openclaw that referenced this pull request May 9, 2026
…ks parentId chain

Compaction retry can write a duplicate compaction event under the same
parentId. One side becomes a dead-end with no descendants while the
other carries the rest of the chain, so any downstream consumer that
reconstructs causal order by walking parentId breaks at the fork.

Extend repairSessionFileIfNeeded to detect this on load: when there are
two or more compaction siblings sharing one parentId AND exactly one of
those compaction siblings has a non-empty subtree, drop the dead-end
compaction loser(s). Generic non-compaction entries are NEVER dropped:
a session JSONL is a tree, so a legitimate side branch can naturally be
a leaf next to a continued sibling, and only the compaction-retry path
is known to produce the duplicate-sibling-with-shared-parentId shape
this repair targets. Two-branch transcripts where multiple compaction
siblings carry real subtrees are also left alone, and so are
compaction-sibling groups where no candidate has won yet.

Adds 9 regression tests including the canonical compaction-retry
pattern, the openclaw#79635 [P2] regression that pins the generic-leaf
carve-out, a mixed-sibling case (one compaction loser dropped while a
non-compaction sibling under the same parent is preserved), nested
compaction forks, deliberate two-branch transcripts, no-winner
compaction candidate groups, linear transcripts, and legacy entries
without parentId.

Fixes openclaw#48810
@jeffrey701
jeffrey701 force-pushed the fix/compaction-retry-orphan-fork-48810 branch from fda1836 to 1583ec7 Compare May 9, 2026 05:14
@openclaw-barnacle openclaw-barnacle Bot added size: L proof: supplied External PR includes structured after-fix real behavior proof. and removed size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 9, 2026
…ks parentId chain

Compaction retry can write a duplicate compaction event under the same
parentId. One side becomes a dead-end with no descendants while the
other carries the rest of the chain, so any downstream consumer that
reconstructs causal order by walking parentId breaks at the fork.

Extend repairSessionFileIfNeeded to detect this on load: when there are
two or more compaction siblings sharing one parentId AND exactly one of
those compaction siblings has a non-empty subtree, drop the dead-end
compaction loser(s). Generic non-compaction entries are NEVER dropped:
a session JSONL is a tree, so a legitimate side branch can naturally be
a leaf next to a continued sibling, and only the compaction-retry path
is known to produce the duplicate-sibling-with-shared-parentId shape
this repair targets. Two-branch transcripts where multiple compaction
siblings carry real subtrees are also left alone, and so are
compaction-sibling groups where no candidate has won yet.

Adds 9 regression tests including the canonical compaction-retry
pattern, the openclaw#79635 [P2] regression that pins the generic-leaf
carve-out, a mixed-sibling case (one compaction loser dropped while a
non-compaction sibling under the same parent is preserved), nested
compaction forks, deliberate two-branch transcripts, no-winner
compaction candidate groups, linear transcripts, and legacy entries
without parentId.

Fixes openclaw#48810
@jeffrey701
jeffrey701 force-pushed the fix/compaction-retry-orphan-fork-48810 branch from 1583ec7 to e70433a Compare May 9, 2026 05:22
@jeffrey701

Copy link
Copy Markdown
Contributor Author

Force-pushed e70433a073 (rebased onto fresh upstream/main to clear the CHANGELOG conflict).

Two things changed since the original push:

  1. Tightened the orphan-fork detection per the [P2] in the prior automated review. The drop is now restricted to type: "compaction" siblings under a shared parentId where exactly one compaction sibling has descendants. Generic non-compaction message leaves are never touched, so a deliberate side branch that happens to be a leaf next to a continued sibling stays in the file. Added three new regression tests pinning the carve-out (the [P2] regression itself + a defensive cousin + a mixed-sibling case where the compaction loser is dropped while a non-compaction sibling under the same parent is preserved).
  2. Added the Real behavior proof section to the PR body. The evidence is the verbatim terminal capture from running node ./node_modules/.bin/tsx against the real repairSessionFileIfNeeded import (production module loaded end-to-end, no shim, no mock) over six on-disk JSONL fixtures including the canonical [Bug]: Compaction retry creates orphan fork in parentId chain (dead-end branch breaks chain reconstruction) #48810 retry pattern, the [P2] regression scenario, mixed siblings, nested forks, and a linear control. All six scenarios print before/after entry lists that match the design contract.

Local gates on e70433a073:

  • npx vitest run src/agents/session-file-repair.test.ts: 60/60 pass
  • npx vitest run src/gateway/session-utils.test.ts src/gateway/session-utils.fs.test.ts src/agents/session-file-repair.test.ts (the suites named in the prior bot review's Acceptance criteria): 555/555 pass
  • npx oxfmt --check --threads=1 src/agents/session-file-repair.{ts,test.ts} CHANGELOG.md: clean
  • Real behavior proof CI gate: success on this head, label flipped to proof: supplied

@clawsweeper re-review

@openclaw-barnacle

Copy link
Copy Markdown

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

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 1, 2026
@clawsweeper

clawsweeper Bot commented Jun 1, 2026

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.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. 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. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 1, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 2, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 2, 2026
@clawsweeper clawsweeper Bot removed the status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. label Jun 2, 2026
@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. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 14, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 1, 2026
@openclaw-barnacle

Copy link
Copy Markdown

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

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

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(sessions): drop dead-end orphan entries when retry forks parentId chain (#48810) This is item 1/1 in the current shard. Shard 11/22.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@openclaw-barnacle

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling 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. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: L stale Marked as stale due to inactivity status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Compaction retry creates orphan fork in parentId chain (dead-end branch breaks chain reconstruction)

1 participant