fix: skip cross-workspace channel collisions during bot sync

[steipete]

Problem

slacrawl sync (bot source) aborts the entire run as soon as it meets a channel that already belongs to a different workspace in the archive:

error: channel "C0HKT7BFH" already belongs to workspace "T0ASS4K1HT5", not "T0BQTNSUF"

Slack Connect shared channels legitimately surface in multiple workspaces. Whichever workspace records the channel first "owns" it in the single-workspace channels table, and every later sync of the other workspace then hits WorkspaceCollisionError and dies — on a real 11-workspace archive this killed a 20,304-channel sync at channel 0.

Fix

The store's collision guard stays intact — it's the right integrity check for message/batch writes, and the desktop import path (redux.go) already skips the same collision class. This change demotes the collision only at the API sync boundary: syncChannelsWithSource now skips a channel owned by another workspace with a warning and continues, in both the serial and concurrent worker paths. Any other upsert error stays fatal.

Test

• New TestSyncSkipsChannelOwnedByAnotherWorkspace: seeds the colliding channel under a different workspace, runs a full bot sync against the mock server, and asserts the sync succeeds, unaffected channels still sync, and the original workspace keeps ownership.
• Full make check green (tidy/fmt/lint/test/smoke/snapshot).
• Codex autoreview clean, no accepted/actionable findings.

[clawsweeper]

Codex review: found issues before merge. Reviewed August 6, 2026, 11:39 AM ET / 15:39 UTC.

ClawSweeper review

What this changes

The PR makes Slack bot sync warn and skip a channel already owned by another archived workspace, continues syncing other channels, adds regression coverage, and adds an unreleased changelog entry.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep this collaborator-authored PR open for normal review. Current main still makes a cross-workspace channel collision fatal; the proposed API-boundary skip preserves the store invariant and matches the existing desktop-import precedent, but the release-owned changelog edit should be removed and the applicable policy requires current real-behavior proof before merge.

Priority: P1
Reviewed head: a4eeb61b42676e1ca29ff4a7783931082c626259

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	PR readiness rating was derived from proof quality, review findings, security review, and reviewer confidence.
Proof confidence	🌊 off-meta tidepool	Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Evidence reviewed	5 items	Current fatal path: Current main calls the channel upsert directly and returns any error, so a typed cross-workspace collision aborts the sync. Store invariant: The store returns WorkspaceCollisionError when an existing channel belongs to a different workspace; the PR leaves this integrity guard unchanged. Existing collision precedent: Desktop Redux ingestion already recognizes a channel workspace collision and continues rather than aborting the import.
Findings	1 actionable finding	[P3] Remove the release-owned changelog entry
Security	None	None.

How this fits together

Slacrawl’s bot sync retrieves workspace channels, stores their metadata in the local archive, then fetches their messages. The archive store enforces a single workspace owner per channel; the sync layer determines whether that collision aborts the run or skips the affected channel.

flowchart LR
A[Slack workspace channel list] --> B[Bot sync]
B --> C[Archive channel store]
C --> D{Owned by another workspace?}
D -->|No| E[Sync channel messages]
D -->|Yes| F[Warn and skip channel]
E --> G[Archived messages]
F --> G

Loading

Before merge

• Remove the release-owned changelog entry (P3) - CHANGELOG.md is release-owned under the applicable repository policy, so this normal bug-fix PR should keep its release context in the PR body or commit message and leave the release note to the release process.
• Resolve merge risk (P1) - The PR body describes mock-based validation and a full check, but no inspectable after-fix real multi-workspace sync transcript is available under the applicable proof policy.
• Resolve merge risk (P1) - The unreleased CHANGELOG.md entry conflicts with the repository policy that keeps release notes release-owned.
• Complete next step (P2) - A collaborator-authored patch with a minor policy correction and missing real-behavior proof should remain with its owner and a human reviewer rather than enter an automated repair lane.

Findings

• [P3] Remove the release-owned changelog entry — CHANGELOG.md:5-8

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Code and test delta	production +35 net (+37/-2), tests +32, release notes +4	The behavioral change is narrowly scoped and has dedicated regression coverage; the release-note edit is separately release-owned.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Land the narrow API-boundary collision skip after recording a redacted real multi-workspace sync result, while retaining fatal handling for all non-collision store errors and leaving release-note ownership to the release process.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the narrow API-boundary collision skip after recording a redacted real multi-workspace sync result, while retaining fatal handling for all non-collision store errors and leaving release-note ownership to the release process.

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

Yes, source-reproducible with high confidence: seed a channel under one workspace, then sync a second workspace that exposes that channel; current main propagates the typed collision as a fatal error.

Is this the best way to solve the issue?

Yes for the functional change: handling only the typed channel collision at the API sync boundary preserves the store’s ownership protection and follows the existing desktop-import pattern.

Full review comments:

• [P3] Remove the release-owned changelog entry — CHANGELOG.md:5-8
CHANGELOG.md is release-owned under the applicable repository policy, so this normal bug-fix PR should keep its release context in the PR body or commit message and leave the release note to the release process.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4d3eadee7c07.

Labels

Label changes:

• add P1: A valid multi-workspace Slack archive can stop at the first shared-channel collision, preventing later channels from syncing.
• 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: Real behavior proof is not required for maintainer- or bot-authored pull requests.

Label justifications:

• P1: A valid multi-workspace Slack archive can stop at the first shared-channel collision, preventing later channels from syncing.
• 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: Real behavior proof is not required for maintainer- or bot-authored pull requests.

Evidence

What I checked:

• Current fatal path: Current main calls the channel upsert directly and returns any error, so a typed cross-workspace collision aborts the sync. (internal/slackapi/api.go:1181, 4d3eadee7c07)
• Store invariant: The store returns WorkspaceCollisionError when an existing channel belongs to a different workspace; the PR leaves this integrity guard unchanged. (internal/store/store.go:826, 4d3eadee7c07)
• Existing collision precedent: Desktop Redux ingestion already recognizes a channel workspace collision and continues rather than aborting the import. (internal/slackdesktop/redux.go:249, 4d3eadee7c07)
• Current-main provenance: Blame attributes the current bot-sync and store paths to the v0.8.1 release preparation, with Peter Steinberger as the recorded area author. (internal/slackapi/api.go:1181, 2e4da62a58a7)
• Patch scope: The branch changes 3 files: API handling, a focused mock-server regression test, and one release-owned changelog entry. (internal/slackapi/api.go:1154, a4eeb61b4267)

Likely related people:

• steipete: Peter Steinberger is recorded by current-main blame across the bot sync and channel-store paths and authored the proposed focused repair. (role: recent area contributor; confidence: high; commits: 2e4da62a58a7, a4eeb61b4267; files: internal/slackapi/api.go, internal/store/store.go)
• Nguyen Cao Cuong - Peter James: Commit 1d09369 introduced desktop IndexedDB handling that skips duplicate-channel workspace collisions, the closest established behavior pattern. (role: introduced adjacent behavior; confidence: medium; commits: 1d09369ef506; files: internal/slackdesktop/redux.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Tighten the PR description with what changed, how it was validated, and any remaining risk.

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.