feat(slack): sync Saved for Later items via void slack saved

[Telvary]

Summary

Sync Slack's "Saved for Later" (Later view) items into void's local DB during background sync, exposed via a new void slack saved CLI command.

How it works:

• Calls search.messages with is:saved query using the user's OAuth token (search:read scope)
• Stores an is_saved flag on messages (DB migration v13 adds column + index)
• Reconciles on each sync: new saves are marked, removed saves are cleared
• Messages not yet in the local DB are fetched on demand (conversations.info + get_single_message)
• Command is Slack-specific, nested under void slack saved (not top-level)

Includes:

• Slack API: search_messages_saved() with cursor pagination
• Sync: sync_saved() + ingest_saved_match() fetch-on-miss logic
• DB: reconcile_saved, list_saved_messages, count_saved_messages
• CLI: void slack saved with --connection, -n, --page
• Setup wizard: documents search:read scope, reinstall note
• Tests: DB saved tests, wiremock API/sync tests, CLI parse tests

This branch is based directly on upstream main (cherry-picked from Telvary/void#7) to avoid duplicate commits from prior fork-only history already landed upstream in #20.

Test plan

• cargo test -p void-slack
• cargo test -p void-core saved
• void slack saved against a connected Slack workspace

[MaximeGaudin]

Thanks for this — it's a well-structured, well-tested PR and the DB/upsert/reconcile design is correct (the ON CONFLICT … DO UPDATE rightly leaves is_saved untouched, and external_id == msg.ts keeps reconcile matching sound). Security and supply-chain are clean. I'd like one change before merge, because Slack is a critical channel for us and the saved view needs to be reliable on every sync cycle.

Required: make per-item ingestion in sync_saved resilient (don't let one bad item abort the whole run)

Where: crates/void-slack/src/connector/sync.rs, the matches loop inside sync_saved.

Problem: the per-item fetch propagates with ?:

} else if self
    .ingest_saved_match(db, &m.channel.id, &m.ts, &user_cache)
    .await?      // <-- aborts the entire sync_saved run
{

ingest_saved_match calls conversations.info + get_single_message. If a single saved item lives in a channel the user can no longer access (left/archived private channel, deleted message) or a transient network/permission error hits, that ? aborts the whole run before reconcile_saved is reached. The call sites in connector_trait.rs and socket_mode.rs wrap sync_saved as non-fatal (warn!), so the failure is swallowed and the saved view silently stops updating — and it fails the same way every cycle, since the offending item is always returned by search. One un-fetchable saved message permanently breaks void slack saved.

Fix: warn-and-continue per item, mirroring the existing resilient pattern in backfill_avatars (users.info lookups already warn! … skipping and continue). Replace the if/else if/else with:

for m in &resp.messages.matches {
    slack_matches += 1;
    match db.find_message_by_external_id(&self.connection_id, &m.ts) {
        Ok(Some(_)) => {
            saved_external_ids.insert(m.ts.clone());
        }
        Ok(None) => match self
            .ingest_saved_match(db, &m.channel.id, &m.ts, &user_cache)
            .await
        {
            Ok(true) => {
                ingested += 1;
                saved_external_ids.insert(m.ts.clone());
            }
            Ok(false) => {
                warn!(
                    connection_id = %self.connection_id,
                    channel_id = %m.channel.id,
                    ts = %m.ts,
                    "saved message could not be ingested"
                );
            }
            Err(e) => {
                warn!(
                    connection_id = %self.connection_id,
                    channel_id = %m.channel.id,
                    ts = %m.ts,
                    error = %e,
                    "failed to fetch saved message; skipping"
                );
            }
        },
        Err(e) => {
            warn!(
                connection_id = %self.connection_id,
                ts = %m.ts,
                error = %e,
                "saved lookup failed; skipping"
            );
        }
    }
}

Important nuance — keep the page-level ? fatal

Please do not soften the search call itself:

let resp = self
    .api
    .search_messages_saved(cursor.as_deref(), page_size)
    .await?;   // keep this fatal

reconcile_saved clears is_saved = 0 for every message of the connection not in saved_external_ids. So the saved set must be a complete, trustworthy enumeration before reconcile runs. If a search.messages page fails we cannot enumerate reliably, and continuing would wrongly un-save items — so aborting there (before reconcile) is the correct, safe behavior. Keep it as-is.

The per-item skip above is safe w.r.t. reconcile precisely because the fetch path only runs for items not yet in the local DB (the Ok(None) branch): an item we fail to fetch was never saved locally, so omitting it from the set can't clear an existing flag. Already-present saved items go through Ok(Some(_)) and never touch the network.

Please add a regression test

In crates/void-slack/src/connector/tests.rs, alongside sync_saved_fetches_missing_message_and_marks_saved, add a test where search returns two saved matches and one channel's conversations.info returns an error/non-200: assert sync_saved still Ok, the healthy item is ingested + marked saved, and list_saved_messages returns exactly that one. That locks in the "one bad item doesn't break the batch" guarantee.

Optional (non-blocking) nit

sync_saved runs on every Socket Mode reconnect (socket_mode.rs) in addition to start. search.messages is rate-limited (tier 2); get_with_retry handles 429s so it's fine, but if reconnects get chatty you may want to debounce saved-sync (e.g. skip if it ran in the last N minutes). Happy to leave for a follow-up.

Everything else looks good to merge once the per-item resilience is in. Thanks again!

[Telvary]

Pushed review fix to Telvary/void@upstream/slack-saved (6bc870d → a185c9a).

Changes:

• Per-item ingestion in sync_saved now warn-and-continues on fetch/lookup errors (mirrors backfill_avatars); search pagination stays fatal so reconcile_saved only runs on a complete enumeration.
• Added regression test sync_saved_skips_inaccessible_item_and_continues (two saved matches, one channel returns 403 — sync still completes, healthy item ingested and marked saved).

Tests:

• cargo test -p void-slack sync_saved — 2 passed
• cargo test -p void-core saved — 5 passed