feat: Slack workspace archiving (user-token sync + search-based reply sweep)

[m-j-r]

Archive your own view of a Slack workspace — public/private channels you're a member of, group DMs, and 1:1 DMs — via the Web API, searchable alongside mail and other chat sources through the existing TUI / FTS / Parquet analytics.

Supersedes #489. Since that PR opened, live probing established that Slack's search API reliably returns thread replies by creation time (threads:replies, day-granular bounds, stable ascending pagination) — enabling a simpler and more correct reply-capture architecture than the lookback-window thread polling reviewed there. Design docs: docs/internal/slack-ingestion-design.md (base) and docs/internal/slack-reply-sweep-design.md (reply sweep, with the full probe ledger).

Usage

msgvault add-slack     # user token (xoxp-…) from an internal app you create — 2-minute setup, scope list in docs/usage/slack.md
msgvault sync-slack    # first run backfills full history; later runs are incremental

Each workspace membership is its own source (slack, <team-id>:<user-id>); multiple workspaces coexist and stay separately filterable in the TUI. A [slack] config block covers daemon scheduling, channel include/exclude filters, and a media size cap.

How replies are captured

• Backfill walks each conversation's history and fetches every thread's replies inline, before the containing page's cursor advances — interrupt/resume-safe by construction.
• Incremental fetches new top-level messages by cursor, then runs a reply sweep: a search.messages threads:replies query over the days since a per-source UTC watermark discovers replies by creation time — a reply to a thread of any age is found (no tracking window, no blind spot). Hits are pointers only; archiving always goes through canonical conversations.replies fetches into the standard upsert path.
• Sweep mechanics follow live-probed behaviors documented in the design doc: date modifiers evaluate in the user's current profile timezone (offset read fresh each run; watermark stored as a UTC instant so tz changes and DST are gap-free); every query string carries an inert nonce (search results are cached by query string); the pager stops on a mismatched echoed page number (past-the-cap pages are silently clamped to page 1) and logs a tripwire if a single day exceeds the reachable ceiling.
• Edits and reaction changes after capture are ignored by default (archive semantics); sync-slack --maintenance repairs the recent window, --full repairs everything. A backfill run under --no-threads leaves a recorded debt that the next threaded run pays automatically.

What's stored

Message text with FTS (mrkdwn rendered, mentions resolved to names, bot attachments/blocks payloads extracted), threads (root+reply via reply_to_message_id), reactions, @mention recipient rows, conversation membership, files in the content-addressed attachment store, and the verbatim API JSON per message (raw_format = slack_json). Members' profile emails unify their Slack identity with their archived mail. Reuses the existing chat schema — no new core tables.

Safety and correctness properties

• The client is read-only by construction (strict method allowlist) and internal-app rate limits apply (999-message pages; the 2025 non-Marketplace clampdown targets distributed apps only — probed live).
• File bytes are fetched only from files.slack.com (no redirects); external/off-host files are metadata + permalink; failed/deferred downloads (incl. --no-media) stay discoverable by backfill-slack-media.
• Every cursor — backfill page chain, incremental window, sweep watermark — advances only behind persisted work; checkpoints merge atomically per cursor unit; all writes are upserts, so at-least-once fetching yields exactly-once storage.
• Conversations that enumerate but 404 on read are skipped, not fatal (observed live).

Store/query changes

Slack attachment-row methods share prefix-parameterized helpers with the Beeper ones; "slack" registered in KnownMessageTypes / TextMessageTypes.

Validated against live workspaces: full backfill (1,100+ message channel), interrupt/resume, threads incl. cross-run late replies, reactions (incl. custom emoji), media into CAS packs, a real 429 absorbed via Retry-After, history depth past 8 years, and the sweep's search behaviors (replies-only modifier, unjoined-channel leakage → filtered, tz re-filing, query caching, page clamping — each with an env-gated live pin-test or fake-server regression test).

Planned follow-ups (separate PRs): opt-in archiving of unjoined public channels via an explicit config allowlist; concurrent fetch/persist pipeline.

[roborev-ci]

roborev: Combined Review (91ebc80)

High-risk Slack reply-watermark flaws can permanently skip messages; additional sync, limit, media-retention, and token-scope issues require fixes.

High

• Missing required Slack token scope — cmd/msgvault/cmd/add_slack.go:34
  The documented scopes omit search:read, although every unlimited sync calls search.messages. Tokens created from these instructions will fail the reply sweep with missing_scope. Add search:read to CLI and user-documentation scope lists, and preferably validate it during setup.

• Watermark can advance beyond the safe search horizon — internal/slack/sweep.go:70
  An unbounded conversations.replies result can move the watermark past the now - 10m search horizon. Unindexed replies in other threads below that watermark may then be permanently skipped. Derive progress only from fully searched intervals, cap it at the lag horizon, and lower provisional progress to a safe boundary on failures.

Medium

• Truncated searches still advance progress — internal/slack/sweep.go:107
  When a daily search exceeds Slack’s 10,000-result ceiling, the sweep records a skipped item but advances the watermark past unreachable results, permanently losing them. Split searches into narrower batches or withhold watermark advancement until the interval is completely searchable.

• Workspace-wide watermark ignores temporarily excluded channels — internal/slack/sweep.go:36
  Progress advances using only the current target set. Replies can be lost when a previously synced channel is absent and later returns. Track progress per conversation or force a catch-up scan when a conversation re-enters the target set.

• --limit does not constrain thread replies — internal/slack/importer.go:378
  Only top-level history messages count toward the limit; one discovered thread can still fetch and persist thousands of replies. Pass the remaining budget into thread pagination, count persisted replies, and retain resumable thread state when exhausted.

• Tombstones can delete archived attachment references — internal/slack/media.go:133
  A refreshed tombstoned file is omitted from refs, causing replacement to remove an existing downloaded attachment row. Preserve existing downloaded references for tombstoned or subsequently omitted remote files.

---

Reviewers: 2 done | Synthesis: codex, 15s | Total: 11m6s

[roborev-ci]

roborev: Combined Review (7bf7cc5)

Verdict: Changes need fixes for two high-severity Slack sync gaps and three medium-severity data-loss/interruption issues.

High

• cmd/msgvault/cmd/add_slack.go:33 — Documented token scopes omit search:read, although default threaded sync calls search.messages. Users following the instructions will get missing_scope, causing sync failure. Add search:read to all user-facing scope lists and preferably validate required scopes during registration.

• internal/slack/sweep.go:70 — A recently indexed reply can advance newWatermark beyond the ten-minute lag horizon. Earlier replies still absent from Slack’s search index may then fall behind the watermark and be skipped permanently. Do not advance search-derived state beyond the lag horizon; preserve any independently trusted backfill floor.

Medium

• internal/slack/sweep.go:102 — Hits from unfinished or currently filtered conversations are discarded while the source-wide watermark advances over them. Replies may be permanently missed when those conversations later become eligible. Track watermarks per conversation, process eligible hits for incomplete conversations, or prevent advancement past skipped targets.

• internal/slack/sweep.go:107 — When a day exceeds Slack’s 10,000-result ceiling, the sweep records a skipped item and advances the watermark, permanently abandoning unreachable results. Treat truncation as incomplete and hold the watermark, or partition searches into channel batches before advancing.

• cmd/msgvault/cmd/sync_slack.go:106 — An interrupted sync returns only cacheErr; if cache rebuilding succeeds, Ctrl-C exits successfully despite an incomplete sync. Return a non-nil interruption error, such as errors.Join(ctx.Err(), cacheErr).

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 11m7s

[roborev-ci]

roborev: Combined Review (f327527)

The Slack ingestion changes have three medium-severity correctness issues; no Critical or High findings were reported.

Medium

• Thread pagination can stall limited imports — internal/slack/importer.go:394
  When a history page consumes the remaining --limit budget and contains a thread root, fetchThread requests at least one item, exhausts the budget before completing the thread, and leaves the history cursor unchanged. Repeated limited runs can reprocess the same page indefinitely.
  Fix: Persist thread pagination progress or advance durable thread debt separately from the history cursor. Test repeated runs with the same limit and a thread longer than the remaining budget.

• Gap recovery can permanently miss thread replies — internal/slack/importer.go:497
  For non-C conversations, gap recovery sets ThreadsPending, but threadCatchUp scans only through the original BackfillLatest. Returning DMs, group DMs, or legacy G private channels can have newer thread roots excluded, leaving older replies permanently below the sweep watermark.
  Fix: Track gap-recovery debt separately and scan through current history before advancing SweptThrough.

• Duplicate Slack attachment content loses file aliases — internal/store/messages.go:2873
  The generic (message_id, content_hash) conflict handling silently drops additional Slack file IDs whose contents are identical. This loses attachment metadata/counts and can trigger repeated downloads during repairs.
  Fix: Normalize duplicate hashes as the Discord path does, retaining alias rows for every Slack file ID and treating those aliases as downloaded. Add coverage for two file IDs with identical content.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m8s

[roborev-ci]

roborev: Combined Review (229d991)

Slack ingestion has two medium-severity correctness issues involving link attachment preservation and DST-aware sweep boundaries.

Medium

• internal/slack/media.go:227 — Tombstoned or omitted metadata-only link attachments are dropped because preservation only covers rows with a content hash, contrary to the archive semantics.

  • Fix: Preserve existing rows whose MediaType is "link" alongside downloaded rows; discard only genuinely pending markers.

• internal/slack/sweep.go:157 — time.FixedZone applies the user’s current tz_offset to historical dates. In DST-observing time zones, this can certify up to an hour not covered by the corresponding on:<date> query, potentially losing replies when a per-day sweep checkpoints before the next day is searched.

  • Fix: Capture Slack’s IANA tz value and use time.LoadLocation for day boundaries, falling back to the fixed offset only when unavailable. Add tests for interrupted sweeps across DST boundaries.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 14m44s

[roborev-ci]

roborev: Combined Review (5468b09)

The Slack ingestion is broadly sound, but three medium-severity issues could prevent reply discovery, block workspace syncs, or make attachments inaccessible.

Medium

• internal/slack/importer.go:211 — Limited syncs permanently skip reply discovery

  Every run using --limit skips the reply sweep. After backfill completes, repeated limited syncs cannot discover non-broadcast replies added to existing threads because those replies are absent from conversations.history.

  Fix: Make the sweep resumable and budget-aware for limited runs, and add coverage for a late reply discovered across repeated limited syncs.

• internal/slack/importer.go:632 — Deleted conversations cause persistent sync failures

  threadCatchUp treats ErrNotFound as a retryable fetch failure. An enumerated-but-gone conversation with ThreadsPending therefore causes the workspace sync to fail on every run.

  Fix: Treat missing conversations and threads as skipped work, clearing the corresponding catch-up debt as the history and thread-drain paths do.

• internal/store/api.go:292 — Slack CAS aliases lose attachment access

  Hashless CAS alias rows have their content hash restored only for discord: attachments. Slack creates equivalent alias rows for duplicate-content files, so the store-backed message-detail API returns these attachments without either a hash or URL.

  Fix: Restore hashes for Slack aliases, or derive hashes from every validated non-URL CAS path regardless of provider.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 11m1s

[roborev-ci]

roborev: Combined Review (6602554)

Changes requested: three medium-severity issues can prevent limited or incremental Slack sweeps from converging reliably.

Medium

• Limited sweeps can stall indefinitely — internal/slack/sweep.go:180
  A limited sweep consumes one budget unit searching the day before fetching results. With --limit=1, the budget is exhausted at the first reply, so repeated runs remain at the same boundary without archiving it, violating the documented convergence guarantee.
  Fix: Separate search and fetch budgets, reserve capacity for a canonical fetch, or otherwise guarantee progress when a searched day contains hits.

• Certification boundary can permanently skip late replies — internal/slack/importer.go:373
BackfillLatest is rounded forward to the end of the current second. A reply created later in that second may arrive after its thread was drained while still falling below the certification pin, causing subsequent sweeps to consider it archived and skip it permanently.
  Fix: Pin to the actual current microsecond and align the history bound’s inclusivity with the certified boundary.

• Full imports discard resumable progress — internal/slack/importer.go:146
  Every --full invocation discards the saved checkpoint. Interrupted full repairs and repeated --full --limit N runs therefore restart at the newest page and may never reach older history.
  Fix: Persist and resume a full-repair session until completion, or reject incompatible scoped full runs and provide another resumable repair mechanism.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 8m49s

[roborev-ci]

roborev: Combined Review (68f93c9)

The review found two medium-severity issues affecting Slack sweep limits and repair completion; no material security regressions were identified.

Medium

• internal/slack/sweep.go:351 — Reply sweeps can exceed --limit arbitrarily.
  Limited sweeps call fetchThread, which fetches full 999-message pages and drains the entire thread before charging the budget.
  Fix: Fetch threads in budget-sized, resumable pages and persist progress before parking the sweep boundary.

• internal/slack/syncstate.go:156 — RepairPending can remain permanently set.
  Repair completion checks all conversations retained in state, including partially repaired conversations that were later left or excluded and will never be visited again. This prevents subsequent --full runs from starting a new repair session.
  Fix: Check completion against the currently eligible conversation set, or retire state for conversations no longer accessible or in scope.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 11m47s

[roborev-ci]

roborev: Combined Review (df6adff)

Code changes need revision due to a medium-severity data-loss risk.

Medium

• Persistence failures can be silently skipped — internal/slack/media.go:121, internal/slack/importer.go:791

  Failures while persisting attachments, FTS data, edits, mentions, reactions, and reply links are reduced to counters, allowing message processing to succeed and the history cursor to advance. This can permanently omit data—particularly attachment rows, which lack a pending marker and therefore cannot be recovered by backfill.

  Fix: Propagate store errors and prevent cursor advancement. Treat media download failures as nonfatal only after successfully writing a durable pending marker.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 13m35s

[roborev-ci]

roborev: Combined Review (9e1c049)

Overall verdict: Two medium-severity reliability issues could cause stalled Slack sync progress and silently unreported pending attachments.

Medium

• Same-generation merging resurrects completed phase state — internal/slack/syncstate.go:217
  A newer failed checkpoint that clears BackfillCursor/BackfillLatest can be merged with an older completed run that still contains those fields. This produces an advanced Cursor paired with a stale opaque page cursor and bounds, potentially causing rejected pagination, replay, or stalled progress. Catch-up cursors and pending debt are similarly affected.
  Fix: Treat phase tuples and debt fields from the newer checkpoint as authoritative, including empty values. Apply monotonic merging only to timestamp watermarks.

• Media backfill silently ignores invalid archived data — internal/slack/importer.go:969
  Missing, unreadable, or malformed archived JSON is skipped while the run is marked complete and returns success. Although the attachment marker remains pending, AttachmentsPending is not incremented, so the CLI may incorrectly report zero pending files.
  Fix: Propagate store-read failures, report missing or malformed raw data as actionable run errors, include affected attachments in the pending count, and return a partial-failure error.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 14m28s

[roborev-ci]

roborev: Combined Review (1a7f3dd)

The Slack ingestion is security-clean, but two medium-severity reliability issues should be addressed.

Medium

• internal/slack/importer.go:471 — Thread replies can be silently omitted after an initial --no-threads backfill. Catch-up debt is recorded only for messages already reporting replies. If an older unthreaded message receives its first reply later, the next threaded run may advance Cursor, causing sweepReplies to use a boundary that excludes replies older than the overlap window. Mark every initial threadless walk as requiring catch-up, or persist the original backfill boundary separately. Add a regression test covering an old message receiving its first reply between runs.

• internal/slack/importer.go:925 — checkpointNow silently discards serialization and database errors. A failed checkpoint can lose repair-session resets or resumable thread debt when a run is interrupted, despite other store failures being treated as fatal. Return and propagate checkpoint errors, using FailSyncWithCheckpoint when finalizing a failed run where appropriate.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 11m46s

[roborev-ci]

roborev: Combined Review (959c2c7)

Slack ingestion is generally sound, but three medium-severity gaps could cause missed or indefinitely delayed thread data.

Medium

• Stamp-less sweep can permanently skip replies — internal/slack/sweep.go:97
SweptThrough is initialized from Cursor after the current incremental walk advances it. If an earlier run completed initial backfill but failed before sweeping, replies created between the original backfill pin and the next run’s pin-minus-overlap are permanently skipped.
  Fix: Persist the initial backfill pin as SweptThrough when the walk completes, before subsequent walks advance Cursor; conservatively use the pre-walk cursor for resumed stamp-less states.

• Thread catch-up can starve indefinitely — internal/slack/importer.go:332
walkWindow consumes the per-conversation limit before threadCatchUp runs. Continuous top-level traffic at or above --limit can exhaust every run’s budget, preventing ThreadsPending debt from --no-threads or gap recovery from progressing.
  Fix: Process existing catch-up work before the next incremental window, or reserve budget that guarantees catch-up progress.

• Maintenance does not repair thread replies — internal/slack/importer.go:772
  The --maintenance rescan only reads conversations.history, which excludes ordinary thread replies. Edits and reaction changes on recent replies therefore remain stale.
  Fix: Fetch and reprocess replies for thread roots in the maintenance window using bounded, resumable pagination.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 12m10s

[roborev-ci]

roborev: Combined Review (52a6021)

The Slack ingestion changes are generally sound, but two medium-severity data-integrity issues should be fixed before merging.

Medium

• Deleted thread roots can overwrite archived content — internal/slack/importer.go:908
  Slack tombstone messages are blindly upserted, so incremental overlap, --maintenance, or --full may replace an archived message’s body, sender, reactions, and raw JSON. Skip tombstone updates when the source message already exists, while retaining placeholders for unseen roots so replies can be linked. Add full and maintenance regression coverage.

• Late-indexed replies can be permanently skipped — internal/slack/syncstate.go:137
RecordPendingThreadTail retains the existing DrainedTo when an overlapping sweep discovers an earlier reply. Subsequent fetches can begin after that reply while certification advances. Move DrainedTo backward when an earlier value is discovered; duplicate fetches are safe. Add coverage for late indexing with a limited or failed drain.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 15m55s

[roborev-ci]

roborev: Combined Review (abe8834)

The Slack ingestion changes have one Medium-severity convergence issue; no Critical or High findings were reported.

Medium

• Permanent retry loop after truncated Slack sweep — internal/slack/sweep.go:219, internal/slack/importer.go:164

  A day containing more than 10,000 results remains permanently unpageable after --full. Although the full walk archives the replies, the sweep records a fetch error without advancing beyond that day. RepairPending then prevents subsequent --full runs from resetting state, so every future sync retries and fails on the same historical day.

  Suggested fix: Allow a completed full repair to certify or bypass its covered range, make truncation restartable, or implement per-channel narrowing. Add a regression test demonstrating that --full converges after a truncated sweep.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 11m56s

[roborev-ci]

roborev: Combined Review (a2b8fc3)

Changes requested: two medium-severity Slack ingestion issues could cause missed replies or ineffective media backfills.

Medium

• internal/slack/sweep.go:261 — Reply sweep can skip unseen replies while advancing the watermark.
  The sweep stops after 100 pages but marks a day truncated only when Total > 10,000. If Slack returns smaller pages and reports Pages > 100, replies beyond the cap are skipped without recording catch-up debt. Treat Pages > maxSearchPages or an unexpected page clamp as truncation before advancing certification.

• cmd/msgvault/cmd/backfill_slack_media.go:65 — Explicit media backfill may download nothing while reporting success.
  backfill-slack-media reuses slackImportOptions, including NoMedia=true when [slack].media=false. This merely recreates pending markers instead of downloading media. Explicitly override NoMedia to false for this command.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 14m25s

[roborev-ci]

roborev: Combined Review (2ae31db)

The review found two medium-severity correctness issues; no critical or high-severity findings were reported.

Medium

• Unstable pagination snapshot — internal/slack/sweep.go:263, internal/slack/sweep.go:306
  Including the page number in the nonce changes the search query on every page. Each page may observe a different search-index snapshot, so index churn can shift offsets, skip a reply, and still allow the day to be certified as complete.
  Fix: Use one stable nonce for the entire day walk that remains unique across separate sweep attempts, and add coverage for index changes between pages.

• Interrupted sync can exit successfully — cmd/msgvault/cmd/sync_slack.go:106
  An interrupted sync returns only cacheErr, causing a successful exit when the cache rebuild succeeds even though synchronization is incomplete.
  Fix: Return errors.Join(ctx.Err(), cacheErr) and test the interrupted command’s exit status.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 13m5s

[roborev-ci]

roborev: Combined Review (6deea57)

Changes need fixes for two medium-severity reliability gaps in Slack maintenance and media downloading.

Medium

• internal/slack/importer.go:800 — Maintenance misses recent replies on old threads. Threads are selected by the root message timestamp, so a recent reply under a root older than 30 days is absent from conversations.history. Later edits or reaction changes to that reply cannot be repaired by --maintenance.

  • Fix: Identify archived replies within the maintenance window, group them by thread root, and rescan those threads regardless of root age. Add coverage for an old root with a recent edited reply.

• internal/slack/media.go:79 — API timeout is too short for large media downloads. Downloads inherit the API client’s 60-second whole-request timeout. A file near the default 100 MiB cap therefore needs roughly 14 Mbps to complete, leaving valid files permanently pending on slower connections; larger configured caps amplify the issue.

  • Fix: Use a dedicated, substantially longer media-download timeout, or rely on caller cancellation with appropriate transport-level timeouts.

---

Reviewers: 2 done | Synthesis: codex, 16s | Total: 15m38s

[roborev-ci]

roborev: Combined Review (5d1e4a7)

High-severity issue: Slack search results may be silently discarded, permanently missing late thread replies.

High

• internal/slack/client.go:456 — search.messages returns the conversation ID as top-level channel_id, but the decoder only reads channel.id. Real search hits therefore get an empty ChannelID, fail the target filter, and are discarded even though the sweep advances its watermark.
  • Fix: Decode channel_id, optionally retaining channel.id as a compatibility fallback. Reject hits missing required IDs before certification, and add a regression test using the real response shape.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 15m56s

[roborev-ci]

roborev: Combined Review (5d1e4a7)

Slack ingestion is generally sound, but a retry-safety issue can permanently leave partially persisted tombstones unrepaired.

Medium

• Partial tombstones are incorrectly treated as complete — internal/slack/importer.go:932

  Tombstones are skipped whenever the message row exists, although message persistence spans several non-atomic writes. If processing stops after UpsertMessage but before saving the body, raw JSON, attachments, mentions, or reactions, retries treat the partial row as an archived original and never repair it.

  Fix: Persist the message and auxiliary data atomically, or skip an existing tombstone only when a completeness marker—such as its archived raw record—confirms the prior write completed. Add a retry regression test.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 6m39s

[wesm]

looking

[roborev-ci]

roborev: Combined Review (737961b)

Slack ingestion is generally sound, but three medium-severity data-integrity issues need attention.

Medium

• Missing recipients on late DM/MPIM replies — internal/slack/sweep.go:436
  Reply-sweep drains create a convScope without toRecipients, so late replies archived in DMs and group DMs lack expected "to" recipient rows. Carry resolved recipients in sweepTarget, pass them into the drain scope, and add coverage for late DM and MPIM replies.

• MPIM history advances after membership-fetch failure — internal/slack/importer.go:441
  A failed membership fetch returns no recipients while still allowing the history cursor to advance. Archived messages then lack "to" recipients and generally will not be revisited after recovery. Hold history progress until membership succeeds, or persist recipient-repair debt and backfill affected messages.

• Default Slack identity uses a non-namespaced identifier — cmd/msgvault/cmd/add_slack.go:98
  The default identity stores bare auth.UserID, while Slack participants use <team-id>:<user-id>. This prevents owner-participant resolution and breaks owner/counterpart relationship analytics. Confirm the namespaced identifier instead and test that it resolves to the Slack user participant.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 15m10s

[roborev-ci]

roborev: Combined Review (1d2d905)

Medium — The Slack media backfill updates attachment rows and message attachment statistics but does not refresh the Parquet analytics cache, leaving cached file counts and sizes stale.

• Location: cmd/msgvault/cmd/backfill_slack_media.go:65
• Fix: Rebuild the cache after any backfill writes, including partial or interrupted runs, and join cache-refresh errors with the operation error.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 11m27s

[roborev-ci]

roborev: Combined Review (726143e)

Slack ingestion is generally sound, but one medium-severity configuration validation gap should be fixed.

Medium

• Invalid Slack schedules can be persisted — internal/config/config.go:917, internal/config/edit.go:655
slack.schedule is missing from validateEditableCandidate, allowing configuration APIs to save invalid cron expressions. The daemon later rejects the job and continues without scheduled Slack synchronization. Add "slack.schedule": cfg.Slack.Schedule to the schedule validation map and test invalid expressions.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 14m44s

[roborev-ci]

roborev: Combined Review (79d53e0)

Medium

• internal/slack/sweep.go:263 — A truncated day is queried again because the next sweep overlaps its boundary by ten minutes. With --limit 1, the repeated day exhausts the sweep budget, prevents advancement beyond the next-day boundary, and resets CatchUpCursor/CatchUpLatest on every run, permanently stalling documented standing-limit convergence.
  • Fix: Persist that the truncated scope/day became catch-up debt, preserve in-flight catch-up progress during overlapping retries, and allow certification to proceed. Add a Limit: 1 regression test spanning the day boundary.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 22m14s

[roborev-ci]

roborev: Combined Review (9be27f4)

Slack ingestion is generally sound, but one medium-severity flag interaction violates --no-threads semantics.

Medium

• internal/slack/importer.go:400 — Combining --no-threads with --maintenance still invokes rescanHead, causing unbounded conversations.replies traversal even though --no-threads promises to skip thread fetching. Suppress maintenance thread rescans when NoThreads is set, or reject this conflicting flag combination.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 17m55s

[roborev-ci]

roborev: Combined Review (cb1c3b9)

Medium-severity issue found: Slack search results are decoded with the wrong channel field, causing valid matches to be discarded.

Medium

• internal/slack/client.go:462 — Slack search.messages returns the channel in the top-level channel_id field, but the decoder expects channel.id. Consequently, SearchMatch.ChannelID remains empty, sweepDay discards real matches, and the sweep watermark still advances—potentially leaving late thread replies unarchived until a later canonical audit recovers them.
  • Fix: Decode channel_id directly, update the fake server to use the real response shape, and assert the channel ID in the live search test.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 12m30s

[roborev-ci]

roborev: Combined Review (795fe9a)

Medium-severity issues remain in Slack pagination and media backfill.

Medium

• Persisted invalid cursors can permanently wedge conversation syncs
  Location: internal/slack/client.go:209, internal/slack/importer.go:503
  Slack may reject a persisted pagination cursor (for example, with invalid_cursor), but the importer preserves and retries it indefinitely. Map invalid-cursor responses to a sentinel error, then restart the affected pinned window with its page cursor cleared while preserving timestamp bounds.

• Deleted files remain pending and are retried forever
  Location: internal/slack/media.go:109
  Permanent file-not-found responses are handled as transient download failures. Since backfill retries archived JSON without refreshing file metadata, deleted files never reach a terminal state. Treat HTTP 404/410 as terminal and retain a metadata-only link, or refresh metadata before keeping the pending marker.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 15m49s

[roborev-ci]

roborev: Combined Review (b830fa6)

The Slack ingestion changes are generally sound, but --full cannot recover from malformed resume state as documented.

Medium

• internal/slack/importer.go:181 — Malformed checkpoints block full repair syncs. Resume state is decoded before --full can reset it, so a malformed checkpoint prevents the documented repair path and all future Slack syncs. When Full is requested, treat state-decoding failures as a fresh repair state while continuing to surface database read failures.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 21m15s

[roborev-ci]

roborev: Combined Review (60aa530)

Code is not clean: one medium-severity checkpoint-loading issue can cause a nil-pointer panic.

Medium

• internal/slack/syncstate.go:141 — A syntactically valid checkpoint containing a null conversation ({"conversations":{"C01":null}}) causes a nil-pointer panic during state loading. The panic occurs before invalid-state handling, so even sync-slack --full cannot recover as documented.
  • Fix: Validate conversation entries before dereferencing them, return an error that the full-repair path can classify and reset, and add a regression test for null entries.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 18m20s

[roborev-ci]

roborev: Combined Review (b4aa66e)

Unsanitized Slack-controlled names can inject terminal control sequences; changes are required before merge.

Medium

• Locations: internal/slack/importer.go:267, cmd/msgvault/cmd/sync_slack.go:89, cmd/msgvault/cmd/add_slack.go:105
• Finding: User-controlled Slack profile display names and workspace-controlled team names are printed without terminal sanitization. Malicious ANSI/OSC sequences could spoof output or affect terminal features such as the clipboard.
• Fix: Apply textutil.SanitizeTerminal to Slack-derived strings at the CLI output boundary while preserving original values in storage.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 13m54s

[roborev-ci]

roborev: Combined Review (c395118)

No Medium, High, or Critical findings were identified.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 13m22s

[wesm]

This appears to be good now, running CI checks and will merge

[roborev-ci]

roborev: Combined Review (7f9de8d)

The reviewers found no Medium, High, or Critical issues.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 13m3s

[wesm]

thank you! Will release soon. Let me know if you encounter any problems syncing from main