fix(integration-events): suppress self-bot-user echo of own Slack writebacks#177
Conversation
… trees A Slack thread PARENT materializes under BOTH the flat messages/<ts> record AND the threads/<ts> root (thread_ts == parent ts) — two distinct files with distinct revisions, one logical message. slackLogicalChangeFingerprint keyed the threads-root copy as `...:thread:<ts>:...` and the messages copy as `...:message:<ts>:...`, so the two copies produced different injection-dedupe keys and the same message was injected twice (and re-injected as each tree's record was re-committed). Operator observed the same message echoing into the channel repeatedly (one msg logged 59x received / 3x injected). Fix: collapse the thread ROOT (no reply segment) to the message identity so both materializations share one dedupe key and inject once. Thread REPLIES keep their distinct `:reply:<ts>` identity. Regression test added; 74/74 bridge tests pass. Note: an agent still receives ONE copy of its own outbound post (mirror reads it back down); fully suppressing self-echo needs a self-bot-user signal — follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR extends the integration event bridge to suppress Slack "self-echo" events from outbound writebacks by tracking correlation keys and bot identity, refines thread-root deduplication fingerprinting, and adds comprehensive integration tests with telemetry coverage. ChangesSlack Self-Echo Suppression and Deduplication
Test Harness and Coverage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
a8b58d8 to
e62cd0d
Compare
|
Implemented one fix: updated the renderer IPC mock telemetry snapshot to include Local verification run:
Addressed comments
I did not print |
|
Implemented and locally verified the PR fixes. I fixed the stale renderer mock breakage from the new telemetry counter and tightened Slack self-echo learning so a same-text post from another bot is not mislearned as Pear’s relay bot. The regression now covers that collision case before confirming the real Validation run:
Addressed comments
I did not print |
|
Implemented the validated fixes for PR #177. Changed:
Verified:
Addressed comments
I did not print |
|
✅ pr-reviewer applied fixes — committed and pushed Implemented one additional validated fix: Slack self-echo suppression now treats a parsed Verified:
Addressed comments
I did not print |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #177 against the checked-out diff and current PR state. I did not make code changes; the current checkout already contains the prior fixes. Working tree is clean. Validation run:
I had to repair local dependencies with Addressed comments
Not printing |
|
I found no additional current breakage in the PR checkout, so I made no new edits. Local validation passed:
Addressed comments
I am not printing |
|
✅ pr-reviewer applied fixes — committed and pushed Reviewed the PR diff and fixed one demonstrated gap: Slack outbound writeback capture now works for bounded Slack command roots even when historical download is off, which is the narrow writeback mode. I also made missing local draft files a no-op during capture so cleanup/delete races do not emit noisy warnings. Changed:
Verified:
Addressed comments
|
|
Reviewed PR #177 against Validation:
Addressed comments
Not printing |
7dde964 to
e62cd0d
Compare
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
… telemetry (#182) #177 added eventsSelfEchoSuppressed to IntegrationEventCounterName but the ipc-mock telemetry totals object was not updated, breaking `tsc -p tsconfig.web.json` on main (TS2741). Add the field (default 0) to the mock. Co-authored-by: kjgbot <kjgbot@agentrelay.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Design
Pear does not receive a returned Slack message
tsfrom the writeback command file path; the local command file contains only the outbound{ "text": ... }. Because discovery lists Pear's bot user indistinguishably from other Slack bots,user_is_botalone is not a valid suppression signal.This uses a scoped bootstrap correlation instead:
scope:textHash, where scope is the canonical channel id or DM/user id and text is trimmed with collapsed whitespace before hashing;user_is_bot === true, an author id exists, and the same scope/text key was recently recorded;Bootstrap caveat: the very first echo before Pear has captured any outbound writeback text cannot be suppressed because there is nothing safe to correlate yet. After the first correlated echo learns the bot id, later messages from that id are suppressed directly.
Test Plan
node --experimental-strip-types --no-warnings --test src/main/__tests__/integration-event-bridge.test.ts(75/75 pass)npx tsc --noEmitRegression Coverage
The new bridge regression verifies:
U0B2596R7EZ/file_by_agent_relayecho is suppressed after outbound writeback capture (text-correlation branch);U0B2596R7EZmessage with non-matching text is still suppressed (authoritative learned-id branch);coderabbit) with nonmatching text is still injected.Reviewer-noted caveats (non-blocking; shadow review by skills-fixer + lead review)
is_botauthor posts our exact writeback text in the same scope within the 15-min outbound window before we ever observe our own first echo (learned-id cache still empty), it could mis-learn that bot's id and suppress it for the 1h self-id TTL. Probability is very low — our own echo returns within seconds and is almost always the first match — and this is inherent to text being the only correlation signal pear has. The 1h TTL self-heals a bad correlation. Documented, not fixed.recordSlackOutboundWriteback()directly, so thewatchLocalMounts→onSlackOutboundWritebackfile-watch wiring and theisSlackLocalWritebackCommandPathpredicate are not exercised end-to-end. Worth an integration-level test as a follow-up.🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com