Skip to content

fix: excluded categories still capture nested threads - #216

Merged
hannesrudolph merged 1 commit into
openclaw:mainfrom
hannesrudolph:fix/category-exclusion-descendants
Sep 11, 2026
Merged

hannesrudolph merged 1 commit into
openclaw:mainfrom
hannesrudolph:fix/category-exclusion-descendants

Conversation

@hannesrudolph

Copy link
Copy Markdown
Member
Additional instructions

MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.

What Problem This Solves

Fixes an issue where users excluding a Discord category would still collect messages from its nested threads during sync, repair, or live capture after startup. For example, excluding a category containing a forum could leave that forum's threads in collection.

Moving a parent channel between included and excluded categories could also leave existing threads using outdated collection rules.

Why This Change Was Made

Exclusion checks now follow the complete channel ancestry, including when filtering stored channels and targeted threads. Live capture retains channel relationships and evaluates them when checking events, so parent updates also apply to existing descendants. Ancestry traversal handles missing category metadata and cycles.

User Impact

Excluding a category or channel through sync.exclude_channel_ids now excludes its descendant channels and threads consistently. Existing category allowlists remain supported, and explicit exclusions still take precedence.

Users can choose opt-out collection by leaving sync.include_category_ids empty and excluding unwanted categories. This PR documents that option; configuration defaults and database schemas are unchanged.

Evidence

  • Added regressions that failed against unchanged upstream and pass with this fix. They cover public/private threads, exclusions overriding category inclusion, stored repair and targeted sync, live capture, parent moves, and incomplete/cyclic ancestry.
  • Passed go test ./internal/syncer -count=1, go test -race ./internal/syncer -count=1, and go vet ./internal/syncer; changed Go files pass gofumpt.
  • Ran go test ./... -count=1. All packages passed except the pre-existing TestPublishProducerValidationBeforeMutation failure (tracked remote branch origin/main is missing), also reproduced against the unchanged application source.
  • Local deployment smoke check confirmed fresh Discord events after restart and no newly collected messages in known excluded channels during the verification window.

@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 11, 2026
@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 11, 2026, 2:20 PM ET / 18:20 UTC.

ClawSweeper review

What this changes

Makes Discord collection exclusions follow category and channel ancestry during sync, repair, and live capture, with regression tests and configuration guidance.

Merge readiness

Ready for maintainer review

This remains a useful, focused fix: current main and v0.14.1 retain the nested-thread exclusion gap. No introduced correctness defect was found, and the member-authored PR is protected from automatic closure.

Priority: P2
Reviewed head: 5b9f8fb80488aa082a3c58d67bcd8de7033cbab6

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with useful regression coverage and no blocking defect found during source review.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body reports fresh Discord events after restart and no new records in known excluded channels; detailed nested-thread and parent-move runtime traces are not supplied.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body reports fresh Discord events after restart and no new records in known excluded channels; detailed nested-thread and parent-move runtime traces are not supplied.
Evidence reviewed 8 items Current-main defect remains: Both exclusion evaluators check the channel and immediate parent but do not traverse to an excluded grandparent category; category inclusion already uses ancestry traversal.
Latest release retains the gap: The v0.14.1 source contains the same immediate-parent exclusion checks; the requested fix is not already present in that release.
Introduced implementation: The verified base-to-head diff adds cycle-bounded ancestor traversal and replaces cached tail exclusion decisions with a mutex-protected relationship catalog evaluated for each event.
Findings None None.
Security None None.

How this fits together

Discrawl collects Discord messages into a local archive. Its syncer combines configured collection rules with channel relationships to decide which historical messages and live events reach storage.

flowchart TD
  A[Collection configuration] --> D[Ancestry exclusion checks]
  B[Discord channel metadata] --> C[Channel relationship catalog]
  C --> D
  E[Historical sync and live events] --> D
  D -->|Allowed| F[Local message archive]
  D -->|Excluded| G[Skip collection]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +51/-82 (net -31); tests +106/-5 (net +101) The repair reduces production code while adding focused coverage for descendant exclusion behavior.

Technical review

Best possible solution:

Use the shared ancestry checks across collection paths while preserving existing inclusion precedence, configuration values, and archive format.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: a stored thread beneath a forum in an excluded category passes current main's immediate-parent checks when no category allowlist is set. This review did not execute that path.

Is this the best way to solve the issue?

Yes. Extending the existing shared evaluator and resolving live ancestry at event time addresses the defect without introducing a competing configuration mechanism.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 88a05763f1f0.

Labels

Label changes:

  • add P2: Repairs a concrete collection-filtering defect affecting users who exclude Discord categories or move parent channels.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body reports fresh Discord events after restart and no new records in known excluded channels; detailed nested-thread and parent-move runtime traces are not supplied.

Label justifications:

  • P2: Repairs a concrete collection-filtering defect affecting users who exclude Discord categories or move parent channels.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body reports fresh Discord events after restart and no new records in known excluded channels; detailed nested-thread and parent-move runtime traces are not supplied.

Evidence

What I checked:

  • Current-main defect remains: Both exclusion evaluators check the channel and immediate parent but do not traverse to an excluded grandparent category; category inclusion already uses ancestry traversal. (internal/syncer/channel_exclusions.go:123, 88a05763f1f0)
  • Latest release retains the gap: The v0.14.1 source contains the same immediate-parent exclusion checks; the requested fix is not already present in that release. (internal/syncer/channel_exclusions.go:123, 2aef26b2df8a)
  • Introduced implementation: The verified base-to-head diff adds cycle-bounded ancestor traversal and replaces cached tail exclusion decisions with a mutex-protected relationship catalog evaluated for each event. (internal/syncer/tail.go:559, 5b9f8fb80488)
  • Production call paths: Targeted sync supplies the complete stored catalog; live channel and thread updates refresh the tail catalog before filtering, and message handlers check exclusions before archive writes. (internal/syncer/channel_catalog.go:62, 5b9f8fb80488)
  • Regression coverage and reported validation: Four added tests cover nested public/private threads, exclusion precedence, stored repair, targeted sync, tail writes, parent moves, and incomplete/cyclic ancestry. The captured PR body reports test/race/vet success and a local deployment smoke check after restart; no tests were executed during this read-only review. (internal/syncer/category_exclusion_descendants_test.go:28, 5b9f8fb80488)
  • Related merged feature: Scope Discord collection by category #149 merged category-scoped collection on 2026-07-25. Its broader feature does not eliminate the exclusion defect demonstrated in current main. (e38d9b67282a)

Likely related people:

  • hannesrudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@hannesrudolph
hannesrudolph merged commit 34bb67e into openclaw:main Sep 11, 2026
17 checks passed
@hannesrudolph
hannesrudolph deleted the fix/category-exclusion-descendants branch September 11, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant