fix(gateway): preserve legacy reseed attachments#99839
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 2:18 AM ET / 06:18 UTC. Summary PR surface: Source +13, Tests +54. Total +67 across 2 files. Reproducibility: yes. at source level: current main returns null for an empty legacy reseed before preserving array siblings, and the PR head adds the mixed native-content fixture. I did not run tests because this review is read-only. Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused importer and regression-test fix after normal maintainer review and required checks, keeping broader Claude placeholder filtering separate. Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main returns null for an empty legacy reseed before preserving array siblings, and the PR head adds the mixed native-content fixture. I did not run tests because this review is read-only. Is this the best way to solve the issue? Yes. The branch keeps the repair inside the Claude CLI importer, shares the remove-block invariant with receipt-backed rows, and preserves existing suppression behavior for scalar and single-text empty reseeds. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d71a24f3a4b7. Label changesLabel justifications:
Evidence reviewedPR surface: Source +13, Tests +54. Total +67 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
Land-ready proof for head
The repair removes only the empty synthetic legacy reseed text block, preserves sibling native content in order, and retains existing suppression for scalar or single-block empty rows. No known proof gaps remain for this two-file surface. |
|
Merged via squash.
|
1 similar comment
|
Merged via squash.
|
Follow-up to #99653.
What Problem This Solves
Legacy Claude CLI reseed rows can contain an empty
<next_user_message>text block alongside an image, document, or other native user content. The importer returnednullfor the whole row, hiding those sibling blocks from visible OpenClaw history.Why This Change Was Made
The importer now applies one invariant to receipt-backed and legacy reseed rows: remove only the proven synthetic text block, and drop the row only when no content remains.
Scalar and single-text-block empty reseeds keep their existing suppression behavior. Mixed native content preserves sibling order and bytes.
User Impact
Image-only and attachment-bearing legacy Claude turns remain visible after reseed import instead of disappearing with the empty synthetic wrapper.
Evidence
node scripts/run-vitest.mjs src/gateway/cli-session-history.test.tsoxfmt --checkandoxlint --deny-warningspassed for both changed filesgit diff --checkpassedtbx_01kwnvp27dr9wa2xhv78wkqgzcpnpm check:changedpassed for core and core-tests