feat(slack): backfill thread history on mid-thread mention - #576
Conversation
Add OpenSpec change directory for backfilling Slack thread history when the bot is @-mentioned mid-thread. Channel-agnostic design targeting Slack first with Teams/Discord in mind.
…575) Proposal establishes the need for backfilling thread history when the bot is @-mentioned mid-thread, with channel-agnostic design targeting Slack first. Design documents key decisions: reuse existing multimodal inbound pipeline, IThreadHistoryFetcher interface, backfill-as-context injection, and no artificial caps (compaction handles overflow).
Specs: new thread-history-backfill capability plus delta specs for netclaw-input-adapters (IsBackfill flag), netclaw-slack-socket (conversations.replies fetch), and netclaw-session (context block injection). Tasks: 22 items across 6 groups covering abstraction layer, Slack implementation, adapter wiring, session injection, testing, and documentation sync.
…#575) Add thread history backfill so the bot sees prior messages (text + images) when @-mentioned in an existing Slack thread. Backfilled content flows through the existing multimodal inbound pipeline and is injected as a read-only context block before the first LLM turn. Key changes: - IsBackfill flag on ChannelInput/SendUserMessage for backfill detection - IThreadHistoryFetcher interface for channel-agnostic history fetch - SlackThreadHistoryFetcher using conversations.replies with pagination, bot filtering, image download, and content scanning - ThreadHistoryContextBuilder assembles backfill into delimited context - LlmSessionActor accumulates backfill messages and injects as context - SlackThreadBindingActor calls fetcher during first initialization
Two integration tests using full actor hierarchy: - Backfill text + images appear in LLM context before the mention message - Second message in same thread does not re-trigger backfill All 12 backfill-related tests passing, 29/29 tasks complete.
) - Remove unused sessionsBasePath parameter from ThreadHistoryContextBuilder - Replace DateTimeOffset.UtcNow fallback with default in ParseSlackTs (violates TimeProvider rule and silently assigns wrong timestamp) - Replace _backfillComplete flag with generation == 1 check (eliminates redundant mutable state) - Give backfill its own 30s timeout instead of sharing pipeline init budget - Download images concurrently via Task.WhenAll instead of sequentially - Cap _pendingBackfill at 500 messages for defense in depth
…ion comments (#575) Consolidate duplicate file download logic from SlackThreadBindingActor and SlackThreadHistoryFetcher into a shared SlackFileDownloader helper. Both callers now delegate HTTP GET + Bearer token auth to the same method. Remove narration comment that restated the method name.
Aaronontheweb
left a comment
There was a problem hiding this comment.
Needs work
| /// When true, this message is backfilled thread history — not a live message. | ||
| /// Backfilled messages are injected as read-only context before the first LLM turn. | ||
| /// </summary> | ||
| public bool IsBackfill { get; init; } |
There was a problem hiding this comment.
Doesn't seem necessary any more - when we do a channel sync prior to the first turn, we're just supposed to lump the user messages together into a batch (like how we normally do with user messages that accumulate between LLM turns)
| MediaReferences = mediaRefs, | ||
| Source = MessageSourceFactory.Create(input, options, turnId) | ||
| Source = MessageSourceFactory.Create(input, options, turnId), | ||
| IsBackfill = input.IsBackfill |
There was a problem hiding this comment.
Probably also not necessary
| private static readonly TimeSpan FileDownloadTimeout = TimeSpan.FromSeconds(10); | ||
| private static readonly TimeSpan ContentScanTimeout = TimeSpan.FromSeconds(5); | ||
| private static readonly TimeSpan PipelineInitTimeout = TimeSpan.FromSeconds(15); | ||
| private static readonly TimeSpan BackfillTimeout = TimeSpan.FromSeconds(30); |
There was a problem hiding this comment.
why do we need a separate timeout for this?
| } | ||
| catch (Exception ex) | ||
| { | ||
| _log.Warning(ex, "Prompt injection detector failed for backfill message; allowing message through"); |
There was a problem hiding this comment.
shouldn't this be fail-closed instead of fail-open? we just need to make sure the user gets visual feedback that there was a problem.
| _log.Debug("Slack thread cursor did not advance stream={StreamKey} ts={Ts}", _sessionId.Value, ts); | ||
| } | ||
|
|
||
| private static List<AIContent> MergeGapWithLiveContents(IReadOnlyList<ChannelInput> gap, IReadOnlyList<AIContent> liveContents) |
There was a problem hiding this comment.
this all gets batched with the live event that triggered this session to be re-activated right?
| return merged; | ||
| } | ||
|
|
||
| private static string? TryExtractTsFromEventId(string eventId) |
There was a problem hiding this comment.
what information gets exposed by the strongly typed SlackNet APIs? is there a way to get message datetime from that? or are we operating based on monotonic event ids instead?
|
|
||
| namespace Netclaw.Channels.Slack; | ||
|
|
||
| public interface ISlackThreadCursorStore |
There was a problem hiding this comment.
imho, this entire concept should not exist. this should be Akka.Persistence inside the SlackThreadBindingActor. each time we advance the cursor we save that asynchronously using PersistAsync - and we can check LastSeqNr % 10 and delete all prior messages (LastSeqNr - 1) in order to keep the Akka.Persistence state small. All we'd be persisting is the cursor position.
| private static readonly TimeSpan FileDownloadTimeout = TimeSpan.FromSeconds(10); | ||
| private static readonly TimeSpan ContentScanTimeout = TimeSpan.FromSeconds(5); | ||
| private static readonly TimeSpan PipelineInitTimeout = TimeSpan.FromSeconds(15); | ||
| private static readonly TimeSpan BackfillTimeout = TimeSpan.FromSeconds(30); |
There was a problem hiding this comment.
Also, just generally - there are way too many different timeout types - that's ridiculous. Consolidate to having two:
- Inbound processing timeout (covers backfill too) - 30s
- Operation timeout - 10s, covers everything else.
…nd archive Rewrite proposal, design, delta specs, and tasks to match the cursor-based hydration implementation (persistent binding actor, merge-into-trigger semantics, injection gate, stale-event drop). Sync deltas to main specs and move the change to archive.
- Live prompt-injection detector path now fails closed: detector exceptions drop the message and post a visible warning instead of silently allowing it through (CLAUDE.md no-silent-fallback). - IThreadHistoryFetcher is now a required dependency on SlackChannel and SlackGatewayDependencies. Tests opt in with an explicit EmptyThreadHistoryFetcher fake. - Unify the live and backfill detector invocations behind one ClassifyAsync helper returning Allow/Block/DetectorUnavailable. - Move Slack ts parsing onto SlackEventTs (IComparable, TryToDecimal, ToDateTimeOffset) and SlackEventId (TryGetEventTs), replacing four copies of decimal/double parsing in the actor and fetcher. - Parse the inbound event ts once per HandleInboundAsync and pass through stale-check, gap computation, and cursor advance. - Hydrate once per runtime by setting the fetch-attempted flag up front so a downstream throw can't re-fetch the whole thread. - Parallelize gap classification with Task.WhenAll — the detector call was the dominant first-turn latency on long gaps. - Gate DeleteMessages on !IsRecovering so journal truncation only runs on genuine persists, not replay. - Drop gap image bytes from the merged ChannelInput — the `[image attachments: N]` summary line already represents them and copying the raw payloads duplicated multi-MB arrays per turn. - Revert whitespace-only edits on ChannelInput and Commands.
Summary
SlackThreadBindingActor(Akka.Persistence) to support restart/offline gap recovery without full replay.Closes #575
Key Implementation Details
IsBackfillfrom channel/session contracts (ChannelInput,SendUserMessage, and pipeline mapping).LlmSessionActorremains transport-agnostic.SlackThreadBindingActornow:conversations.replies,(cursor, currentEventTs)gap,Tests and Validation
IsBackfillpropagation test.dotnet test src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csprojdotnet slopwatch analyze