fix(sessions): persist sender metadata in user turn transcript JSONL#90552
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 12:23 AM ET / 04:23 UTC. Summary PR surface: Source +226, Tests +380. Total +606 across 18 files. Reproducibility: yes. source-level: current main prepares and writes user-turn transcript rows without any sender identity field, while the linked issue shows the group transcript attribution gap. I did not rerun a live Telegram group message in this read-only review. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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 this focused transcript fix after exact-head checks finish green, preserving the non-phone sender metadata boundary, direct-chat negative behavior, and hook redaction coverage. Do we have a high-confidence way to reproduce the issue? Yes, source-level: current main prepares and writes user-turn transcript rows without any sender identity field, while the linked issue shows the group transcript attribution gap. I did not rerun a live Telegram group message in this read-only review. Is this the best way to solve the issue? Yes, with accepted merge-risk boundaries: the shared user-turn transcript writer is the right owner, the PR scopes persistence to group/channel routes, excludes phone identity, and covers hook redaction plus queued writes. The remaining non-code requirement is green exact-head validation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 785f197de600. Label changesLabel justifications:
Evidence reviewedPR surface: Source +226, Tests +380. Total +606 across 18 files. View PR surface stats
Security concerns:
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
|
3a0b07d to
7f539c6
Compare
7f539c6 to
5a66108
Compare
|
Removed senderE164 from the persisted __openclaw envelope per ClawSweeper feedback. Only senderId/senderName/senderUsername are persisted now — phone metadata excluded for privacy. Tests pass (27/27). @clawsweeper re-review |
cef3699 to
8ba0295
Compare
|
@clawsweeper re-review Refreshed PR body with security-boundary risk checklist:
Proof: production appendUserTurnTranscriptMessage + 27/27 tests pass. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
97cd10e to
ae72cca
Compare
Thread senderId/senderName/senderUsername/senderE164 from the channel inbound context into the persisted user-turn transcript message so that group chat session JSONL records include __openclaw sender identity. Ref openclaw#90531
Remove phone-number field from the persisted __openclaw sender envelope, keeping only senderId, senderName, and senderUsername. Privacy-sensitive E.164 metadata can be added back by maintainers if needed. Ref openclaw#90531
The `as TBase & ExecPolicyLayer` casts failed because the returned objects did not spread `...base`, losing generic TBase fields. Co-Authored-By: Claude Opus 4.7 <[email protected]>
The baseWithoutMode destructuring produces Omit<TBase, "mode"> which is not assignable to TBase & ExecPolicyLayer when the as-cast is removed. Co-Authored-By: Claude Opus 4.7 <[email protected]>
…ranch The second branch (security/ask override without a mode change) must exclude base.mode from the spread so that mode is not leaked into results when only security or ask fields are being overridden. Without this fix, the spread ...base carries mode through to the returned object, breaking callers that expect applyExecPolicyLayer to clear stale mode when applying explicit security/ask policy fields. Co-Authored-By: Claude Opus 4.7 <[email protected]>
ae72cca to
4d8648e
Compare
|
Merged via squash.
|
…enclaw#98336) Use __openclaw.senderId as the per-participant discriminator in the compaction dedup key, so two different group participants who happen to send the same text within the dedup window are both preserved. Requires openclaw#90552 which persists senderId/senderName/senderUsername in the __openclaw envelope on user-turn transcript messages. Key change: - Read __openclaw.senderId (string) instead of senderIsOwner (boolean) - Same sender, same text, inside window → still deduped - Different sender, same text, inside window → both kept (bug fixed) - Fall back to empty prefix when no senderId is available Ref openclaw#98310
…penclaw#90552) * fix(sessions): persist sender metadata in user turn transcript JSONL Thread senderId/senderName/senderUsername/senderE164 from the channel inbound context into the persisted user-turn transcript message so that group chat session JSONL records include __openclaw sender identity. Ref openclaw#90531 * fix(sessions): exclude senderE164 from persisted transcript for privacy Remove phone-number field from the persisted __openclaw sender envelope, keeping only senderId, senderName, and senderUsername. Privacy-sensitive E.164 metadata can be added back by maintainers if needed. Ref openclaw#90531 * fix(infra): spread base fields in applyExecPolicyLayer return values The `as TBase & ExecPolicyLayer` casts failed because the returned objects did not spread `...base`, losing generic TBase fields. Co-Authored-By: Claude Opus 4.7 <[email protected]> * fix(infra): use ...base spread instead of ...baseWithoutMode The baseWithoutMode destructuring produces Omit<TBase, "mode"> which is not assignable to TBase & ExecPolicyLayer when the as-cast is removed. Co-Authored-By: Claude Opus 4.7 <[email protected]> * fix(infra): preserve baseWithoutMode in second applyExecPolicyLayer branch The second branch (security/ask override without a mode change) must exclude base.mode from the spread so that mode is not leaked into results when only security or ask fields are being overridden. Without this fix, the spread ...base carries mode through to the returned object, breaking callers that expect applyExecPolicyLayer to clear stale mode when applying explicit security/ask policy fields. Co-Authored-By: Claude Opus 4.7 <[email protected]> * fix(sessions): scope persisted sender attribution * fix(sessions): preserve sender metadata through hooks * refactor(sessions): keep sender metadata path lean * fix(sessions): preserve sender metadata in runtime writes * fix(sessions): preserve queued sender attribution * test(sessions): use complete message fixtures * refactor(sessions): rely on narrowed user message type * test(sessions): use shared temp directory helper * test(sessions): align sender metadata assertions * fix(sessions): honor sender metadata redaction hooks * test(agents): use automatic temp cleanup * test(sessions): cover queued turn provenance * test(auto-reply): expect room sender metadata * refactor(sessions): isolate queued transcript context --------- Co-authored-by: Claude Opus 4.7 <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
…penclaw#90552) * fix(sessions): persist sender metadata in user turn transcript JSONL Thread senderId/senderName/senderUsername/senderE164 from the channel inbound context into the persisted user-turn transcript message so that group chat session JSONL records include __openclaw sender identity. Ref openclaw#90531 * fix(sessions): exclude senderE164 from persisted transcript for privacy Remove phone-number field from the persisted __openclaw sender envelope, keeping only senderId, senderName, and senderUsername. Privacy-sensitive E.164 metadata can be added back by maintainers if needed. Ref openclaw#90531 * fix(infra): spread base fields in applyExecPolicyLayer return values The `as TBase & ExecPolicyLayer` casts failed because the returned objects did not spread `...base`, losing generic TBase fields. Co-Authored-By: Claude Opus 4.7 <[email protected]> * fix(infra): use ...base spread instead of ...baseWithoutMode The baseWithoutMode destructuring produces Omit<TBase, "mode"> which is not assignable to TBase & ExecPolicyLayer when the as-cast is removed. Co-Authored-By: Claude Opus 4.7 <[email protected]> * fix(infra): preserve baseWithoutMode in second applyExecPolicyLayer branch The second branch (security/ask override without a mode change) must exclude base.mode from the spread so that mode is not leaked into results when only security or ask fields are being overridden. Without this fix, the spread ...base carries mode through to the returned object, breaking callers that expect applyExecPolicyLayer to clear stale mode when applying explicit security/ask policy fields. Co-Authored-By: Claude Opus 4.7 <[email protected]> * fix(sessions): scope persisted sender attribution * fix(sessions): preserve sender metadata through hooks * refactor(sessions): keep sender metadata path lean * fix(sessions): preserve sender metadata in runtime writes * fix(sessions): preserve queued sender attribution * test(sessions): use complete message fixtures * refactor(sessions): rely on narrowed user message type * test(sessions): use shared temp directory helper * test(sessions): align sender metadata assertions * fix(sessions): honor sender metadata redaction hooks * test(agents): use automatic temp cleanup * test(sessions): cover queued turn provenance * test(auto-reply): expect room sender metadata * refactor(sessions): isolate queued transcript context --------- Co-authored-by: Claude Opus 4.7 <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
…re test comments Codex review feedback: - Remove top-level sender field precedence — only canonical __openclaw envelope is the trusted source for durable sender metadata (openclaw#90552) - Restore unintentionally dropped security/lifecycle test comments from the rebase merge - Consolidate sender-label tests to keep file under max-lines limit - Remove 'prefers top-level over envelope' test (no longer applicable)
ClawSweeper review: openclaw#90552 only establishes senderName, senderUsername, senderId in the __openclaw envelope. senderLabel has no producer contract as a persisted identity field. Remove it from HistoryMessage type and the renderer fallback chain. Update tests to use contracted senderName.
What Problem This Solves
Telegram group and channel conversations can share one OpenClaw session, but their durable user-turn transcript rows did not retain which participant sent each message. That made later transcript reads lose participant attribution even though the inbound channel context had it.
Fixes #90531.
Why This Change Was Made
senderId,senderName, andsenderUsernamein the existingmessage.__openclawenvelope; intentionally exclude phone-number identity.senderIsOwnerauthorization bit.User Impact
Shared group/channel transcripts now retain participant attribution for every committed user turn, including turns queued behind an active run. Direct chats do not gain new persisted sender identity, and hook/redaction behavior remains intact.
Evidence
97cd10e0eef686cd09b2158cbe539e3d74c8ad13passed a full build on isolated AWS Crabbox leasecbx_4f878654a015, runrun_0503a3f8d022.pnpm check:changedforcoreandcoreTestswith zero warnings and zero runtime import cycles.4d8648ea9131dd6d86ead4e7d4da6eaf1e6ff94apassed all 69 applicable hosted checks, including build, both whole-suite shards, and QA smoke (run 28767468051).ae72ccaee550a301baa8756f7763f9aad368802b.