fix(dreaming): drop heartbeat assistant responses from dream corpus#104085
fix(dreaming): drop heartbeat assistant responses from dream corpus#104085ekinnee wants to merge 14 commits into
Conversation
openclaw-barnacle
Bot
added
size: XS
triage: needs-pr-context
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
62806d4 to
8de7aaf
Compare
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 8:45 PM ET / July 14, 2026, 00:45 UTC. Summary PR surface: Source +566, Tests +375, Other +1. Total +942 across 13 files. Reproducibility: no. executed current-main reproduction was established in this review, but the source path is high-confidence: current main drops only exact HEARTBEAT_OK assistant text and retains natural-language heartbeat acknowledgments. Review metrics: none identified. 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:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the provenance-backed forward filter and targeted memory-core doctor repair after a redacted exact-head workspace run demonstrates selective removal, preservation of normal corpus and diary content, idempotence, and clean subsequent ingestion for the applicable storage backends. Do we have a high-confidence way to reproduce the issue? No executed current-main reproduction was established in this review, but the source path is high-confidence: current main drops only exact HEARTBEAT_OK assistant text and retains natural-language heartbeat acknowledgments. Is this the best way to solve the issue? Yes for the implementation direction: runtime provenance is the narrowest non-spoofable forward filter and memory-core doctor is the correct repair owner; merge readiness remains blocked on real upgrade proof. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cae29a079203. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +566, Tests +375, Other +1. Total +942 across 13 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
Review history (21 earlier review cycles; latest 8 shown)
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
openclaw-barnacle
Bot
added
size: S
and removed
triage: needs-pr-context
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
the
merge-risk: 🚨 other
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
The forward behavior is sound: authenticated provenance.kind === "heartbeat" avoids the user-spoofable cross-message coupling rejected in #70737, and the pending flag correctly survives tool/thinking records until the paired assistant text. I cannot land this as the complete bug fix yet because upgraded installations do not converge.
The production evidence in this PR found 21 already-derived ghost entries. This patch changes future transcript ingestion only; it does not invalidate the existing session corpus / ingestion checkpoints or remove already-written DREAMS.md fallback entries. Current auditDreamingArtifacts() detects self-ingested dreaming prompts, not heartbeat-derived contamination or the generic fallback diary phrase, so openclaw doctor will not offer the existing archive/rebuild repair for this case. After upgrade, users can therefore keep seeing the polluted derived state even though new heartbeat turns are filtered.
Please add a bounded convergence contract before merge. The safe shape is an explicit doctor/audit repair (or a versioned derived-corpus rebuild) that can prove heartbeat contamination from source/corpus state, archives the affected derived artifacts, clears the matching ingestion checkpoints, and lets them rebuild under the new filter. Do not delete the generic fallback sentence by text alone: it is also emitted for legitimate request-scoped narrative failures and is not sufficient provenance. Add an upgrade test that starts with pre-fix corpus/checkpoint/diary state, runs the repair or migration, and proves the contaminated artifacts no longer remain eligible while unrelated dream entries survive or are safely archived according to the chosen contract.
I did not find a blocker in the new heartbeat provenance value or the forward session parser tests themselves.
|
Forward-only convergence is not bounded by the 2/7-day lookback in the current implementation. Those windows limit which transcript material is selected for future dreaming runs; they do not delete already-written I am not accepting manual deletion of all dreams as the upgrade contract because it discards unrelated derived state and is not discoverable or automated. The pre-merge requirement remains a targeted audit/doctor repair or versioned rebuild that archives the affected derived artifacts, clears the matching ingestion checkpoints, and rebuilds under the provenance filter. The repair must use source/corpus provenance rather than deleting the generic fallback sentence by text. |
openclaw-barnacle
Bot
added
extensions: memory-core
openclaw-barnacle
Bot
removed
the
triage: needs-pr-context
c59f102 to
5a70da0
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
clawsweeper
Bot
added
rating: 🦪 silver shellfish
The dreaming ingestion pipeline filters heartbeat user messages (containing [OpenClaw heartbeat poll]) via sanitizeSessionText, but the paired assistant response is only dropped when it is the exact string HEARTBEAT_OK. Local models frequently respond with natural-language acknowledgments (e.g. "Heartbeat received. Main is active.") which pass through the sanitizer unchanged and enter the dream corpus as low-confidence (0.58) memory snippets. Fix: track heartbeat user message drops in buildSessionEntry and skip the immediately following assistant response. This cross-message coupling is safe because the heartbeat prompt pattern is injected by the runtime and cannot be spoofed by user input, unlike [cron:...] or System (untrusted): ... patterns (see PR openclaw#70737). Refs: openclaw#103720
…xt matching
Replace text-based heartbeat detection (isGeneratedHeartbeatPromptMessage)
with provenance-based authentication. The heartbeat turn now carries
provenance: { kind: "heartbeat" } when persisted to the transcript,
and buildSessionEntry checks for this provenance instead of matching
user-spoofable text content.
Changes:
- src/sessions/input-provenance.ts: Add "heartbeat" to
INPUT_PROVENANCE_KIND_VALUES
- src/auto-reply/reply/get-reply-run.ts: Attach heartbeat provenance
to user turn input when isHeartbeat is true
- packages/memory-host-sdk/src/host/session-files.ts: Check
message.provenance.kind === "heartbeat" instead of text matching
- packages/memory-host-sdk/src/host/session-files.test.ts: Update
heartbeat test to include provenance; add lookalike regression test
This addresses the ClawSweeper review finding [P1] about user-spoofable
text matching. The cross-message coupling is now authenticated by
runtime provenance, not user-typed content.
Refs: openclaw#103720
The findHeartbeatContaminatedCorpusLines function could not read SQLite-backed session transcripts. When a corpus ref had no .jsonl extension (SQLite logical path format), it appended .jsonl, read an empty file, and silently skipped — leaving pre-fix heartbeat contamination intact for SQLite users. Fix: - Add loadTranscriptLinesFromSqlite helper that reads events via the canonical loadTranscriptEventsSync API - Serialize all events to preserve original event positions for corpus #L<n> line-number references - Fall back to SQLite reader when filesystem read fails on a non-.jsonl corpus ref path Test: add SQLite-backed session regression test that seeds a session, writes corpus refs without .jsonl extension, and verifies both audit detection and targeted repair.
…nd early return P1: Remove clearScopedSessionIngestionState call after heartbeat line removal. Clearing the cursor causes the next ingestion to re-process the transcript from the beginning, duplicating normal corpus lines that were deliberately retained. P2: Remove early return after heartbeat cleanup so the self-ingestion narrative content check and archiveDiary request still run.
…t ID The session path in corpus refs is already in the format 'sessions/main/abc.jsonl' (includes agent subdirectory). The previous code prepended source.agentId again, creating a path like 'agents/main/sessions/main/abc.jsonl' — which silently returned empty in production workspaces. Fix: use path.basename to extract just the filename from the session path, then construct the transcript path correctly.
…LOC ratchet The dreaming-repair.ts file grew from 337 to 831 lines, exceeding the 500-line LOC ratchet limit. Split helper utilities into a new dreaming-repair-utils.ts file: - dreaming-repair.ts (365 lines): imports, types, public API functions (auditDreamingArtifacts, repairDreamingArtifacts) - dreaming-repair-utils.ts (498 lines): all helper functions, constants, and internal types
5d65f1d to
62a51e8
Compare
|
Closing this PR in favor of a fresh one with the updated repair approach. The forward fix (provenance-based heartbeat detection) is unchanged. The upgrade repair now archives the entire session-corpus directory and clears all SQLite checkpoints instead of targeted line-by-line rewrite, per steipete's review requirements. |
2 participants
What Problem This Solves
The dreaming ingestion pipeline filters heartbeat user messages (containing
[OpenClaw heartbeat poll]) viasanitizeSessionText, but the paired assistant response was only dropped when it was the exact stringHEARTBEAT_OK. Local models frequently respond with natural-language acknowledgments (e.g. "Heartbeat received. Main is active.") which pass through the sanitizer unchanged and enter the dream corpus as low-confidence memory snippets.See issue #103720.
Why This Change Was Made
The original text-based heartbeat detection was user-spoofable (anyone could type the heartbeat marker and trigger the drop). The fix uses runtime provenance (
provenance: { kind: "heartbeat" }) attached at the source inget-reply-run.ts, which cannot be spoofed by user input.For existing workspaces with heartbeat contamination, the
doctoraudit/repair system:loadTranscriptEventsSyncUser Impact
openclaw doctorfor existing workspacesEvidence
Forward Prevention (Provenance-based)
provenance: { kind: "heartbeat" }attached at the source inget-reply-run.tsbuildSessionEntrychecksmessage.provenance.kind === "heartbeat"instead of text-matching user contentpendingHeartbeatUserDropflagBackward Repair (Targeted Doctor Audit)
auditDreamingArtifacts+repairDreamingArtifactsfunctions available viaopenclaw doctorReal Behavior Proof (Rebased on upstream/main @ 5a0fcc2)
Test: Contaminated workspace with 4 corpus lines (2 heartbeat-derived, 2 normal), session ingestion state, and file-backed session transcript.
Audit detects contamination:
Repair removes only heartbeat lines, preserves normal content:
Idempotent (second repair is no-op):
Validation
oxfmt --checkpassesoxlint --deny curlypassestsgopassesFiles Changed