fix(desktop): ignore nested attachments during message discovery

[ViaxCo]

Why

Slack Desktop can keep a message unfurl inside an attachment. When that attachment has a timestamp, text, and another channel ID, slacrawl treats it as a separate message and stops the whole desktop sync with the error in #254.

Change

• Keep attachments on their parent message. Do not scan them as separate messages.
• Add a synthetic regression test. Real message identity conflicts still stop the sync.

This addresses the desktop sync failure in #254. It does not change schema migration behavior.

Checks

• Go 1.27 race-enabled test suite, vet, vulnerability check, and CLI smoke test passed.
• GoReleaser snapshot build passed.
• A full sync of the affected local Slack cache completed.

Real-cache proof (redacted)

The affected cache has a timestamped attachment under a parent message. The attachment includes channel_id and text fields. Message text, Slack IDs, and timestamps are withheld.

I ran the committed local build (7af4853) against that cache:

$ slacrawl --no-color sync --source desktop
● Completed  local state refreshed

State
  workspaces=1  |  channels=63  |  users=99  |  messages=2597

A read-only archive query for the affected parent message returned:

[{"parent_rows":1,"rows_with_attachment":1}]

Maintainer validation

Expanded the synthetic regression across message/thread containers, foreign/same/missing attachment channel IDs, all three DM policies, and repeat ingestion. The test verifies one parent row and an intact raw attachment payload; the existing real-identity-conflict regressions remain unchanged. Added desktop upgrade recovery guidance and changelog credit.

The baseline decoder reproduces two messages and one identity conflict from a single parent plus attachment. Independent Codex P0–P2 review is clean. Full make check passed on AWS Crabbox at ff2902ce0c9b08ce96547c19b16607c2aa762d3d, including the race suite, CLI smoke, and snapshot packaging. A compiled CLI synced the synthetic desktop cache twice and retained exactly one parent row with one attachment. Exact-head hosted CI is required before merge.

Fixes #254.

[clawsweeper]

🦞👀
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]

Codex review: needs maintainer review before merge. Reviewed September 23, 2026, 3:37 PM ET / 19:37 UTC (Revision 2).

ClawSweeper review

What this changes

The PR changes Slack Desktop cache decoding to keep attachments with their parent message, adds an ingestion regression test, and documents the behavior.

Regression provenance

Possible regression — suspected (failure trace). No predecessor PR is attributed.

Merge readiness

✅ Ready for maintainer review

Keep this PR open. Current main and v0.10.0 still traverse nested attachments as messages, while the updated PR body now provides redacted output from a successful sync of the affected cache and an archive query showing one parent row with its attachment. The linked report’s schema rollback concern remains separate.

Priority: P1
Reviewed head: 7af4853616e0c82705ad42701e0e43115bb6d858

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A small, focused fix has regression coverage and relevant redacted real-cache proof, with no concrete patch defect found.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The changed production Node decoder feeds desktop sync. For the affected local cache, the PR body shows the committed build completing sync --source desktop and a read-only archive query finding one parent row with its attachment, satisfying the previous review’s requested after-fix observation. No stored-data schema or serialization contract changes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production Node decoder feeds desktop sync. For the affected local cache, the PR body shows the committed build completing sync --source desktop and a read-only archive query finding one parent row with its attachment, satisfying the previous review’s requested after-fix observation. No stored-data schema or serialization contract changes.
Evidence reviewed	8 items	Current-main failure path: The main-branch walker recurses through every child, including attachments. A timestamped attachment with text and a different channel ID can therefore be treated as a message and reach the retained-container identity check. Introduced guard: The pinned PR delta skips the attachments child during message discovery; it does not remove the parent message. Regression coverage: The added ingestion fixture gives a parent message a timestamped attachment with another channel ID and asserts that only the parent is archived.
Findings	None	None.
Security	None	None.

How this fits together

Desktop sync reads Slack’s local cache, decodes cached messages, checks their identities, and writes admitted records to the SQLite archive. This change affects how the decoder treats attachments before those checks.

flowchart LR
  A[Slack Desktop cache] --> B[Message decoder]
  B --> C{Nested attachment?}
  C -->|Yes| D[Keep with parent]
  C -->|No| E[Check message identity]
  D --> F[SQLite archive]
  E --> F

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Changed lines	production +3, tests +21, spec +1	The production change is a narrow guard backed by a focused ingestion fixture.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #254
Summary: This PR is a candidate fix for the desktop sync failure in the linked issue; that issue also reports a separate schema rollback concern.

Members:

• canonical: v0.10.0: desktop sync fails with "desktop message identity conflicts with retained cache container" (also on empty DB) #254 - It reports the desktop identity-conflict failure that this PR addresses and remains open.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Land the focused attachment-discovery guard while retaining identity checks for genuine messages; handle the linked report’s downgrade concern on its own merits.

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

Yes. The pre-patch walker and added fixture establish a concrete path from a timestamped, cross-channel attachment to the identity-conflict error; this review did not run the private cache.

Is this the best way to solve the issue?

Yes. Skipping attachment traversal at message discovery is narrow, preserves the parent payload, and leaves checks for genuine message identity conflicts in place.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The changed production Node decoder feeds desktop sync. For the affected local cache, the PR body shows the committed build completing sync --source desktop and a read-only archive query finding one parent row with its attachment, satisfying the previous review’s requested after-fix observation. No stored-data schema or serialization contract changes.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production Node decoder feeds desktop sync. For the affected local cache, the PR body shows the committed build completing sync --source desktop and a read-only archive query finding one parent row with its attachment, satisfying the previous review’s requested after-fix observation. No stored-data schema or serialization contract changes.
• remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

• P1: The linked v0.10.0 report describes a desktop sync failure affecting a working user setup.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production Node decoder feeds desktop sync. For the affected local cache, the PR body shows the committed build completing sync --source desktop and a read-only archive query finding one parent row with its attachment, satisfying the previous review’s requested after-fix observation. No stored-data schema or serialization contract changes.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production Node decoder feeds desktop sync. For the affected local cache, the PR body shows the committed build completing sync --source desktop and a read-only archive query finding one parent row with its attachment, satisfying the previous review’s requested after-fix observation. No stored-data schema or serialization contract changes.

Evidence

What I checked:

• Current-main failure path: The main-branch walker recurses through every child, including attachments. A timestamped attachment with text and a different channel ID can therefore be treated as a message and reach the retained-container identity check. (internal/slackdesktop/redux_decoder.js:157, 4eeb7828ca42)
• Introduced guard: The pinned PR delta skips the attachments child during message discovery; it does not remove the parent message. (internal/slackdesktop/redux_decoder.js:158, 7af4853616e0)
• Regression coverage: The added ingestion fixture gives a parent message a timestamped attachment with another channel ID and asserts that only the parent is archived. (internal/slackdesktop/admission_test.go:214, 7af4853616e0)
• Parent payload retained: The decoded message still carries attachments, and the ingestion path stores the message as raw JSON. (internal/slackdesktop/redux.go:103, 4eeb7828ca42)
• Real-cache proof: The updated PR body reports running committed build 7af4853 against the affected local Slack cache. Its redacted terminal transcript shows desktop sync completing with 2,597 messages, followed by a read-only archive result of one parent row containing an attachment. This addresses the previous review’s specific proof request; the private cache was not available for independent inspection.
• Release boundary: The latest identified release tag, v0.10.0, points to e2f5738. The PR commit is absent from the fetched main branch and has no containing release tag. (7af4853616e0)

Likely related people:

• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Peter Steinberger: 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.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-23T19:09:19.786Z sha 7af4853 :: needs real behavior proof before merge. :: none

[clawsweeper]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(desktop): ignore nested attachments during message discovery This is item 1/1 in the current shard. Shard 0/1.

This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

[steipete]

The synthetic baseline reproduces the retained-container identity failure. The fix correctly stops discovery at attachments while retaining their parent payload; the expanded regression covers message/thread containers, all DM policies, attachment identity variants, and replay. Existing genuine identity-conflict rejection stays in place. Independent P0–P2 review is clean; merge remains gated on the full remote and exact-head CI checks.