feat: add external archive provider sync - #86
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 31, 2026, 8:26 PM ET / August 1, 2026, 00:26 UTC. ClawSweeper reviewWhat this changesThe branch adds Merge readinessThis draft collaborator PR is still necessary: current main has no external archive-provider source, while the branch adds a new permanent configuration and JSONL subprocess contract. The implementation appears deliberate and extensively covered in source tests, but accepting trusted local executables and a core provider protocol requires maintainer product and security-boundary approval before merge. Priority: P2 Review scores
Verification
How this fits togetherSlacrawl’s sync command collects Slack data from API, desktop, and MCP sources into a local SQLite archive with full-text search. This PR adds an explicit provider source that sends sync scope to a configured local executable, validates its JSONL records, and writes catalog data, messages, and checkpoints into that same archive. flowchart LR
A[Slacrawl configuration] --> B[Sync command]
B --> C[Selected provider source]
C --> D[Trusted local executable]
D --> E[Validated JSONL records]
E --> F[Transactional SQLite writes]
F --> G[Checkpoints and FTS index]
G --> H[Search and archive tools]
Decision needed
Why: The patch adds new configuration, a public protocol, and an intentional local-code execution boundary; source review can verify safeguards, but cannot determine whether that ongoing product and security contract belongs in core. Before merge
Findings
Agent review detailsSecurityNeeds attention: The diff deliberately adds a trusted local-executable boundary with documented environment filtering, but the permanent security model needs maintainer approval. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep provider imports explicit and opt-in, land the protocol only after maintainers accept the trusted-executable model and validate one representative existing archive upgrade plus an end-to-end provider run. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new archive-import capability rather than reporting broken existing behavior. Source inspection confirms current main has no provider source to reproduce. Is this the best way to solve the issue? Unclear pending maintainer direction: the implementation is a coherent explicit-provider design, but only maintainers can decide whether its permanent subprocess and compatibility contract should live in core. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 53dd8d09b584. LabelsLabel changes:
Label justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (18 earlier review cycles; latest 8 shown)
|
8dcd80d to
f384036
Compare
Summary
sync --source provider:<name>Why
Existing Slack archives should be importable without coupling Slacrawl to another tool's database schema. The provider protocol keeps that boundary explicit while retaining Slacrawl's canonical storage and checkpoint behavior.
Safety
Verification
go test -count=1 ./...go vet ./...deadcode -test ./...gofmt -l .go mod tidywith no dependency changes