fix(session): prevent amnesia on crash by deferring cursor advance by Aaronontheweb · Pull Request #733 · netclaw-dev/netclaw

Aaronontheweb · 2026-04-23T22:34:19Z

Summary

Fixes two independent amnesia vectors that cause sessions to lose all state after daemon crashes and restarts:

Cursor advancement race (Slack): SlackThreadBindingActor persisted its cursor (marking messages as "seen") at enqueue time via PersistAsync, before the downstream LlmSessionActor persisted the completed turn via Persist. On crash between those two writes, the message was permanently lost — the cursor said "seen" but no turn was recorded, and thread history hydration skipped it on restart. Now the cursor advances only when TurnCompleted(Completed) confirms the turn is durably recorded. _pendingCursorTs provides transient stale-event dedup within a single runtime and is cleared on failed/skipped turns and pipeline reinitialization.
System prompt eviction: When identity files (SOUL.md, AGENTS.md, TOOLING.md) were missing on recovery, SetSystemPrompt() actively deleted the last-known system prompt from state. Now it retains the recovered prompt as a fallback and logs a warning. The proper long-term fix is CWD-aware prompt resolution (Track session CWD in WorkingContext and emit as [working-context] + [project-instructions] #595).

Discord adapter note

The Discord adapter (DiscordConversationActor on PR #713) is a plain ReceiveActor with no cursor persistence — it won't replicate the Slack cursor race bug, but also has zero crash recovery for thread state. If Discord ever adds cursor-based dedup, it should follow the deferred-advance pattern from this fix.

Test plan

All 11 SlackThread* tests pass
All 5 Backfill/SessionBinding tests pass
dotnet slopwatch analyze — 0 violations
Manual verification: crash daemon mid-turn, verify session resumes with full context on restart
Verify hydration gap-fill includes the un-persisted message after crash recovery

… TurnCompleted Two fixes for session state loss after daemon crashes: 1. Slack cursor race: the thread binding actor persisted its cursor (marking messages as "seen") at enqueue time, before the session actor persisted the completed turn. On crash between those two writes, the message was permanently lost. Now the cursor advances only when TurnCompleted confirms the turn is durably recorded. 2. System prompt eviction: when identity files were missing on recovery, SetSystemPrompt() actively deleted the last-known prompt from state. Now it retains the recovered prompt as a fallback and logs a warning.

- Clear _pendingCursorTs unconditionally on TurnCompleted (not just on Completed outcome) so failed/skipped turns don't leave stale state that blocks message retries - Clear _pendingCursorTs on pipeline reinitialization - Condense enqueue and system prompt comments

…nt restart-duplication The hydration gap filter included messages with ts == cursor (not just ts > cursor), under the comment "keep the cursor event itself during fresh-runtime hydration in case the daemon restarts during an in-flight turn." That defensive rule pre-dated PR netclaw-dev#733, which guarantees the cursor only advances on TurnCompleted with outcome Completed. With netclaw-dev#733 in place, ts == cursor strictly means "the session has already persisted this message" — re-including it on a restart hydration just duplicates the message into the session a second time. Symptom (observed in session D0AC6CKBK5K/1778728886.944599): a single Slack message with 12 image attachments (~12 MB raw bytes) was ingested at session start. After the daemon was restarted to swap the failover model, the new binding-actor lifecycle ran hydration, the cursor-keep rule re-emitted the message as adopted-context, and the session's persistent history ended up with two copies of the same 12 images. Prompt size doubled from ~6.9M chars to ~13.8M chars; the primary 256K-token vLLM correctly 400'd; failover hit a text-only fallback that 400'd for a different reason ("not a multimodal model"). The user-visible failover error masked the actual duplication. Fix: change the predicate from `itemTs < cursor → skip` to `itemTs <= cursor → skip` in both SlackThreadBindingActor.PerformOneShotHydrationAsync and the Discord equivalent. The in-flight-crash case still works: a turn that didn't complete leaves the cursor un-advanced, so the message has ts > cursor and is correctly included in the gap. Test: added Hydration_after_restart_does_not_re_emit_message_at_cursor_position to SessionBindingContractTests (runs against both Slack and Discord concrete suites). Test was first added against the pre-fix code and confirmed to fail ("gap=1, backfill_enqueued" log on the second lifecycle); fix applied; test passes; full 1576-test Actors suite green.

…t for media in compaction (#990) * fix(channels): hydrate thread history once per actor lifetime; account for media in compaction Slack/Discord binding actors were running thread-history hydration on every inbound event. PR #733 (cursor defers to TurnCompleted for crash safety) leaves the cursor intentionally lagging during in-flight turns, so concurrent inbounds re-included in-flight messages in the gap window. Their image DataContent was re-downloaded, re-wrapped, and re-written to disk under a fresh GUID each time. One 554KB PNG ended up persisted 8 times in a single session, blowing the post-vLLM-fix 256K context window via base64-inflated duplicates in the LLM payload. Layer 1 — hydrate once per actor lifetime: - Both binding actors gain a Hydrating behavior between Initializing and Active. - Recover<RecoveryCompleted> self-tells PerformHydration. After Initializing completes pipeline init, the actor becomes Hydrating; the queued PerformHydration message runs FetchThreadHistoryAsync once, synthesizes one backfill ChannelInput using the latest authorized gap message as trigger (older gap messages wrapped as adopted context), enqueues it, then Become(Active); Stash.UnstashAll(). - Inbound handler in Active is fetch-free; the live event stream is the authoritative source after startup. - Inbounds arriving during hydration are stashed and unstashed on Become(Active). - Cursor (_cursorTs / _cursorSnowflake) still advances only on TurnCompleted — PR #733's amnesia fix is preserved. Layer 2 — compaction estimator accounts for media (defense-in-depth): - SerializableMediaReference gains long FileSizeBytes (proto3 additive field, zero-default = legacy-safe). - Populated at both media-write sites (ChatMessageConverter.WriteMediaToSession, ChannelPipeline.MapToCommand). - SessionCompactionPipeline.EstimateTokens sums fileSize * 4 / 3 per media ref (base64 inflation) using a long accumulator so multi-megabyte images don't overflow int. Tests — new channel-conformance invariants in SessionBindingContractTests (inherited by both Slack and Discord): startup-backfill emission, slash-text safety in adopted projection, self-only authorship flag, fetch-at-most-once per actor lifetime, plain post-hydration live inbounds, image-flows-once regression test, stashing during hydration, re-runs after supervised restart, cursor-only-on-TurnCompleted regression pin. Plus seven EstimateTokens unit tests covering legacy compatibility, media size accounting, multiple refs, text+media combined, system-prompt media, and long-accumulator overflow safety. Three pre-existing integration tests in SlackThreadBackfillIntegrationTests updated to reflect the new fetch-at-most-once semantics. No persistence schema changes; existing journals replay unchanged. * fix(channels): drop cursor-keep rule in hydration gap filter to prevent restart-duplication The hydration gap filter included messages with ts == cursor (not just ts > cursor), under the comment "keep the cursor event itself during fresh-runtime hydration in case the daemon restarts during an in-flight turn." That defensive rule pre-dated PR #733, which guarantees the cursor only advances on TurnCompleted with outcome Completed. With #733 in place, ts == cursor strictly means "the session has already persisted this message" — re-including it on a restart hydration just duplicates the message into the session a second time. Symptom (observed in session D0AC6CKBK5K/1778728886.944599): a single Slack message with 12 image attachments (~12 MB raw bytes) was ingested at session start. After the daemon was restarted to swap the failover model, the new binding-actor lifecycle ran hydration, the cursor-keep rule re-emitted the message as adopted-context, and the session's persistent history ended up with two copies of the same 12 images. Prompt size doubled from ~6.9M chars to ~13.8M chars; the primary 256K-token vLLM correctly 400'd; failover hit a text-only fallback that 400'd for a different reason ("not a multimodal model"). The user-visible failover error masked the actual duplication. Fix: change the predicate from `itemTs < cursor → skip` to `itemTs <= cursor → skip` in both SlackThreadBindingActor.PerformOneShotHydrationAsync and the Discord equivalent. The in-flight-crash case still works: a turn that didn't complete leaves the cursor un-advanced, so the message has ts > cursor and is correctly included in the gap. Test: added Hydration_after_restart_does_not_re_emit_message_at_cursor_position to SessionBindingContractTests (runs against both Slack and Discord concrete suites). Test was first added against the pre-fix code and confirmed to fail ("gap=1, backfill_enqueued" log on the second lifecycle); fix applied; test passes; full 1576-test Actors suite green.

Replace the _pendingCursor* null check with a dedicated _turnInFlight flag in the Slack, Discord, and Mattermost binding actors. The cursor is set only for inbounds with a parseable ordering key, so a null-key in-flight turn left the cursor unset and let a following mention re-arm hydration mid-turn -- the concurrency the PR #733 invariant prevents. _turnInFlight is set on every enqueue (live inbound and hydration backfill), independent of the ordering key, and cleared at TurnCompleted and on pipeline reinit so an abandoned turn cannot leave it stuck. Document two accepted trade-offs of the reuse-based backfill in the re-arm comment: one thread-history fetch per mention on an active thread, and a mention in the in-flight window adopts chatter on the next idle mention.

…gered backfill (#1798) * docs(openspec): add per-channel MentionRequiredInThread change plan Adds the OpenSpec change per-channel-mention-required-in-thread with proposal, design, six spec deltas, and tasks. Planning only; no runtime code. The change makes MentionRequiredInThread a per-channel value, reuses the existing thread-history backfill on a mention, seeds the value at channel-add time, and deletes the connector-wide bool from #1783 (never deployed, so no migration). * feat(channels): make MentionRequiredInThread per-channel; delete connector bool Replaces the connector-wide MentionRequiredInThread bool (never deployed) with a per-channel MentionRequiredInThreadByChannel map on the Slack, Discord, and Mattermost channel options, plus a MentionRequiredInThreadFor(channelId) resolver that defaults to false for unmapped channels. The three conversation actors resolve the per-channel value and pass the resolved bool into the unchanged, pure routing policies. Updates the config schema (adds the per-channel map, removes the connector bool) and adds per-channel binding/resolution tests. Implements OpenSpec tasks group 2 (config storage) and group 3 (routing gate). * chore(openspec): mark tasks groups 1-3 done (config storage + routing gate) * feat(channels): re-hydrate the thread gap on a tap-gated mention When MentionRequiredInThread is on for a channel, the conversation actor forwards only mentions to the thread binding actor, so each inbound is a deliberate re-entry. The binding actor now re-arms the existing deferred thread-history hydration on such an inbound, guarded on no in-flight turn (cursor not lagging) to preserve the PR #733 no-duplicate invariant, so the mention catches up on the gap the tap held. Applies to the Slack, Discord, and Mattermost binding actors, reusing the existing fetch / gap-computation / prompt-injection-gate / merge path (no new fetch path, no new watermark). Adds a Slack integration test proving a second mention on the same live actor re-fetches (fetchCount 1 -> 2 with no restart); Discord and Mattermost use the identical re-arm. Implements OpenSpec tasks group 4 (backfill re-trigger). * chore(openspec): mark tasks group 4 (backfill re-trigger) done * docs(channels): document per-channel MentionRequiredInThreadByChannel Updates the config reference (docs/spec/configuration.md), the three channel integration pages, slack-acl-policy.md, and the adding-a-channel runbook to describe the per-channel MentionRequiredInThreadByChannel map and the removal of the connector-wide bool. The netclaw-operations skill covers operational tasks (scheduling, doctor, approvals, MCP), not channel config, so no skill change is warranted. Implements OpenSpec tasks group 6. * feat(cli): add per-channel MentionRequiredInThread toggle to netclaw config Adds a per-channel MentionRequiredInThread control to the netclaw config Channels editor, mirroring the ChannelAudiences per-channel machinery for a bool map (MentionRequiredInThreadByChannel): - The EditAudience leaf shows 'Mention required in thread: On/Off' and toggles it with Space; Enter persists it alongside the audience. - The add-channel step seeds the rule from the assigned audience (Team/Public -> on; Personal/DM -> off). - Name->id canonicalization remaps the mention map like audiences, so a rule set on a channel name is not silently dropped when the name resolves to its id. - Adds a config round-trip test and a native smoke tape (config-mention-thread) registered in the light suite. Implements OpenSpec tasks group 5. * test(smoke): add config-mention-thread post-tape assertion The config-mention-thread tape is a config-writing tape, so it requires a paired semantic assertion. Adds tests/smoke/assertions/config-mention-thread.sh, which verifies Slack.MentionRequiredInThreadByChannel.C01 = true after the leaf toggle + apply. Marks OpenSpec tasks groups 5-6 and quality gates 7.1-7.4 done. * chore(openspec): sync per-channel MentionRequiredInThread deltas to specs and archive openspec archive synced the six delta specs into openspec/specs/ (7 added requirements, 3 modified across netclaw-input-adapters, thread-history-backfill, the three socket specs, and channel-audience-tui) and moved the completed change to openspec/changes/archive/2026-08-07-per-channel-mention-required-in-thread. Closes out OpenSpec task group 7 (quality gates + close-out): full solution build clean, routing/backfill/TUI tests green, slopwatch 0 issues, copyright headers verified, and the full native smoke light suite (23 tapes + 9 scenarios) green including the new config-mention-thread tape. * fix(channels): harden mention re-arm guard against null-key turns Replace the _pendingCursor* null check with a dedicated _turnInFlight flag in the Slack, Discord, and Mattermost binding actors. The cursor is set only for inbounds with a parseable ordering key, so a null-key in-flight turn left the cursor unset and let a following mention re-arm hydration mid-turn -- the concurrency the PR #733 invariant prevents. _turnInFlight is set on every enqueue (live inbound and hydration backfill), independent of the ordering key, and cleared at TurnCompleted and on pipeline reinit so an abandoned turn cannot leave it stuck. Document two accepted trade-offs of the reuse-based backfill in the re-arm comment: one thread-history fetch per mention on an active thread, and a mention in the in-flight window adopts chatter on the next idle mention. * feat(cli): fold the channel detail leaf into the Channels & Permissions list Remove the per-channel EditAudience leaf. The list edited a channel's audience inline already, so the leaf only re-presented one field; its sole unique job was the mention toggle. Make the list the single per-channel editor. On the Channels & Permissions list a real channel row now shows the audience cycler plus an arrow-free 'Require @mention: On/Off' field. Space toggles the rule on the focused row and autosaves, like the left/right audience toggle. A description line under the list follows the cursor and explains the selected row's audience and mention state. A DM row shows audience only; the rule does not apply to one-to-one messages. Delete BuildEditAudience, HandleEditAudienceKey, OpenSelectedChannelAudience, the leaf-only view-model state, and the EditAudience screen. Enter on the list now only activates the Add/Done rows. Rewrite the config-mention-thread smoke tape to toggle on the list row instead of the leaf; the paired assertion is unchanged (C01 persists true). Update the view-model tests to the list flow. * fix(cli): restore [Enter] Done in the Channels & Permissions footer The list footer legend lost its [Enter] binding when the mention toggle was added, but Enter still activates the Add/Done rows. Restore [Enter] Done so the footer and the secondary hint line agree. Drop the [a] accelerator from the footer to keep it within terminal width; the hint line still documents it. Addresses a code-review finding. The paired finding (raw channel ID no longer shown after removing the detail leaf) is intentionally not fixed: the list deliberately hides raw IDs for resolved channels (see the Channels_ChannelPermissions_RendersResolvedDiscordLabelWithoutRawId test).

Move the fetch, cursor-filter, injection-classify, adopted-context merge, and turn-enqueue algorithm from the three binding actors into one ThreadGapHydrationEngine in Netclaw.Channels. All constructor dependencies are required. The engine holds no actor state: cursor reads, queue access, warnings, and enqueue bookkeeping are callbacks the actor supplies. Channel differences that remain are genuine and stay per channel: - cursor ordering (Slack decimal event ts, Discord snowflake comparator, Mattermost ordinal) via an injected comparer - authorization basis (Slack ACL policy vs allowed-user options) via a required callback - fetcher availability: Discord and Mattermost construct the engine only when the gateway supplies a history fetcher Slack log lines gain the session and allowed-count fields the other channels already log. No behavior change otherwise; the PR #733 cursor-advance invariants move into the engine verbatim. Net: -859 lines across the three actors, +442 shared.

…2005) * Add OpenSpec change for binding-actor engine consolidation Plan the extraction of the four duplicated orchestration regions from the Slack, Discord, and Mattermost binding actors into shared engines. The design records the key decision set: plain engine classes over a base actor class, an injected cursor comparator with a length-then- ordinal Discord comparator, hook-limited channel differences, and a stop rule for any real semantic difference found during transplant. * Convert the Discord cursor to a string with a snowflake comparator The persisted CursorAdvanced event stores a string cursor for every channel. Discord alone converted that string to ulong for in-memory comparisons, which kept its hydration code textually different from Mattermost's and blocked a shared gap-hydration engine. Hold the cursor as a canonical string and compare with SnowflakeCursorComparer (length first, then ordinal). Plain ordinal is wrong across a digit-length boundary; a unit test proves the comparator matches ulong ordering, with an explicit case that shows the plain ordinal failure. The ulong parse remains as a normalization step, so corrupt IDs are rejected exactly as before and persisted values are byte-identical. * Extract the shared thread gap-hydration engine Move the fetch, cursor-filter, injection-classify, adopted-context merge, and turn-enqueue algorithm from the three binding actors into one ThreadGapHydrationEngine in Netclaw.Channels. All constructor dependencies are required. The engine holds no actor state: cursor reads, queue access, warnings, and enqueue bookkeeping are callbacks the actor supplies. Channel differences that remain are genuine and stay per channel: - cursor ordering (Slack decimal event ts, Discord snowflake comparator, Mattermost ordinal) via an injected comparer - authorization basis (Slack ACL policy vs allowed-user options) via a required callback - fetcher availability: Discord and Mattermost construct the engine only when the gateway supplies a history fetcher Slack log lines gain the session and allowed-count fields the other channels already log. No behavior change otherwise; the PR #733 cursor-advance invariants move into the engine verbatim. Net: -859 lines across the three actors, +442 shared. * Extract the shared approval-response flow Move text-approval parsing, the cold-spawn approval path, and prompt resolution from the three binding actors into ApprovalResponseFlow in Netclaw.Channels. All dependencies are required. Persistence stays in the actor via a persist callback. The requester identity check keeps one home in PendingApprovalLookup. The transplant surfaced one real semantic difference: Slack resolved the earliest matching pending approval, Discord and Mattermost the most recent. The shared lookup takes a required ApprovalMatchOrder, so each channel keeps its exact selection. The parity spec documents the difference; which order is correct is a separate product question. Mattermost keeps its synchronous webhook reply through an optional per-call hook; Discord and Slack pass none. One deliberate Slack alignment: its old try block also swallowed journal persist and redraw failures on the text path. The shared flow wraps only the feedback send, so a persist failure now faults the Slack actor exactly as it does the other two. Net: -737 lines across the three actors, +437 shared. * Extract the shared output engine and safe transport-call skeleton ChannelOutputEngine owns the per-turn delivery state machine: pending cursor, turn-in-flight, reminder observer settlement, empty-turn fallback suppression, and prompt clearing. The engine returns the PendingApprovalPromptCleared events; the actor persists them. A required channel hook handles outputs only some channels support. SafeTransportCall owns the timing, telemetry, failure-notify skeleton for Discord and Mattermost posts and uploads. Slack keeps its own transport wrappers: its three-way exception classification and RecordReplyRejected category do not fit the shared skeleton, per the stop rule. Genuine differences stay per-channel behind hooks: delivery-failure report timing (Slack defers to turn completion), reinitialize cursor discard (Slack only), reminder observed-at source, text trimming, and error formatting. One behavior-preserving unification: prompt-post failure now removes the pending entry before the auto-deny on all three channels, which also removes a Mattermost double-deny risk. Net: -401 lines across the actors, +489 shared. * Record the reinitialize cursor divergence in the parity spec * Sync the channel-binding-parity spec to main specs

Aaronontheweb added 2 commits April 23, 2026 22:28

Aaronontheweb mentioned this pull request Apr 23, 2026

docs(openspec): draft Discord channel interactions planning spec #713

Merged

5 tasks

Aaronontheweb marked this pull request as ready for review April 24, 2026 00:43

Aaronontheweb added bug Something isn't working context-pipeline LLM context assembly: prompt layers, dynamic injection, memory recall, temporal grounding reliability Retries, resilience, graceful degradation sessions LLM session actor, turn lifecycle, pipelines labels Apr 24, 2026

Aaronontheweb merged commit 5f4e5df into dev Apr 24, 2026
5 of 6 checks passed

Aaronontheweb deleted the fix/session-amnesia-crash-recovery branch April 24, 2026 00:46

Aaronontheweb mentioned this pull request Apr 24, 2026

release: prepare v0.15.0 release notes and version bump #739

Merged

3 tasks

Aaronontheweb mentioned this pull request May 14, 2026

fix(channels): hydrate thread history once per actor lifetime; account for media in compaction #990

Merged

7 tasks

Aaronontheweb mentioned this pull request Aug 7, 2026

feat(channels): per-channel MentionRequiredInThread with mention-triggered backfill #1798

Merged

Navigation Menu

fix(session): prevent amnesia on crash by deferring cursor advance - #733

fix(session): prevent amnesia on crash by deferring cursor advance#733
Aaronontheweb merged 2 commits into
devfrom
fix/session-amnesia-crash-recovery

Aaronontheweb commented Apr 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Aaronontheweb commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Discord adapter note

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Aaronontheweb commented Apr 23, 2026 •

edited

Loading