feat(cli): add EML import, sender repair, conversation search, and GC

[salmonumbrella]

What changed

• Lock in source-type display in msgvault list-accounts (table and JSON) with CLI tests; the column itself already existed.
• Add resumable msgvault import-eml support for MailMate-style mailbox trees, with mailbox labels and raw-byte deduplication.
• Add conversation_id: filtering across lexical search, SQLite and DuckDB analytics, drill-down queries, and vector/hybrid search.
• Add a dry-run-by-default msgvault repair-senders command that can restore missing sender data from bounded archived MIME headers.
• Add SQLite-only msgvault gc for source-deleted rows, with confirmation, a backup by default, optimistic plan validation, reply-pointer cleanup, and compaction. Dedup-hidden rows remain intact, and PostgreSQL is refused before mutation.
• Admit the new commands through the daemon CLI runner.

Why

Several older archive workflows still required manual database work or could not be done at all: standalone EML trees were not importable, missing senders had no safe repair path, conversation IDs were not searchable, account listings omitted source type, and retained source-deleted rows could not be purged and compacted from the CLI. This closes those gaps while keeping destructive maintenance explicit and recoverable by default.

Usage

msgvault list-accounts
msgvault import-eml /path/to/Mail --identifier me@example.com
msgvault search 'conversation_id:123'
msgvault repair-senders
msgvault repair-senders --apply
msgvault gc

Closes #158
Closes #175
Closes #192
Closes #200
Closes #214

[roborev-ci]

roborev: Combined Review (c2eb844)

Verdict: Three medium-severity issues require attention.

Medium

• internal/store/gc.go:81-84 — GC deletes messages without removing standalone messages_fts rows, leaving orphaned content that may be associated with reused SQLite message IDs. Delete corresponding FTS rows in the same transaction.

• cmd/msgvault/cmd/import_eml.go:70-79 — Hard-error imports skip confirmDefaultIdentity but still run legacy identity migrations, potentially breaking “from me” attribution. Confirm the source identifier before migrations unless --no-default-identity is set.

• internal/api/handlers.go:3570-3575 and 3665-3671 — Daemon HTTP search rejects structured conversation_id filters even though local and vector engines support them. Accept and apply ConversationID in both handlers and update admission tests.

---

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

[roborev-ci]

roborev: Combined Review (768fc23)

Verdict: Two medium-severity issues require attention.

Medium

• internal/query/sqlite.go:1997-1999 — A ConversationID filter replaces existing ConversationIDs instead of intersecting them, causing conflicting filters to return incorrect results. Intersect the ID sets and preserve no-match semantics.

• internal/store/sender_repair.go:221-234 — Sender repair updates relational data but leaves the standalone SQLite messages_fts row stale, so full-text searches may miss repaired messages. Refresh or rebuild the affected FTS row within the repair transaction.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 9m52s

[roborev-ci]

roborev: Combined Review (5188f61)

Verdict: High- and medium-severity issues remain in GC correctness and sender repair.

High

• internal/store/gc.go:62-96 — GC validates only aggregate counts. If one source-deleted row is restored and another is marked deleted between planning and execution, counts may remain unchanged while GC permanently deletes the newly marked row instead of the planned population. Snapshot and compare exact source-deleted IDs, or use a writer lock/generation token.

Medium

• internal/store/gc.go:93-96 — Deleting messages does not recompute denormalized conversation metadata, leaving fields such as message_count, last_message_at, and last_message_preview referring to purged messages. Recompute affected conversation statistics within the same maintenance transaction.

• internal/store/sender_repair.go:49,198 — Candidate discovery and guarded updates exclude rows whose legacy message_type is SQL NULL, although the project treats NULL as an email message. Include message_type IS NULL in both predicates.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 8m33s

[roborev-ci]

roborev: Combined Review (4cd15e8)

Verdict: Two medium-severity issues require attention; no high or critical findings were identified.

Medium

• internal/importer/eml_import.go:176-177 — Resume skips every file lexically at or before LastFile, so newly added or modified earlier .eml files may be silently omitted. Persist and validate a file manifest/content fingerprint, or safely rescan with idempotent deduplication.

• internal/store/sender_repair.go:155-209 — The optimistic repair guard does not verify the MIME content used during planning. A concurrent raw MIME update could result in a stale sender address being written. Include an expected raw-content hash/version in the conditional update.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 12m12s

[roborev-ci]

roborev: Combined Review (b3d9c52)

Verdict: One medium-severity concurrency issue requires attention.

Medium

• internal/store/sender_repair.go:238-270 — The repair checks for existing From recipients, then separately deletes/replaces From rows while only locking message_raw. A concurrent writer can add sender evidence between these operations, allowing the repair to delete newer evidence. Serialize sender-recipient updates with the repair transaction, or insert recovered evidence without destructively replacing rows.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 17m9s

[roborev-ci]

roborev: Combined Review (63fd9d4)

Verdict: One medium-severity issue requires attention.

• Medium — cmd/msgvault/cmd/import_eml.go:110-114
  The new default source type eml is not recognized by SourceTypeUsesEmailIdentity, so legacy [identity] entries are never migrated to EML accounts. If another email source is present, migration may be marked complete while EML remains skipped.
  Fix: Add eml to the email source classifier and gate confirmDefaultIdentity on that classifier.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 19m6s

[roborev-ci]

roborev: Combined Review (d48ff46)

Verdict: One medium-severity issue requires attention; no critical or high-severity findings were identified.

Medium

• SQLite URI handling is incomplete — cmd/msgvault/cmd/import_eml.go:59-90, cmd/msgvault/cmd/repair_senders.go:95-98
  SQLite file: URI handling is applied when opening the store, but cache refresh/build paths still treat the raw DSN as a filesystem path. Imports or repairs using a URI may fail during mandatory cache refreshes, especially when query parameters are present. Normalize SQLite DSNs to filesystem paths before passing them to cache and OS-level helpers.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 20m43s

[roborev-ci]

roborev: Combined Review (7aa801b)

Verdict: 2 medium-severity issues require attention; no critical or high-severity findings.

Medium

• internal/sqliteutil/dsn.go:48-50 — All file-URI hosts are converted to UNC-style paths. file://localhost/path.db should resolve locally as /path.db, but currently becomes //localhost/path.db, potentially affecting directory creation and backup paths. Treat localhost as a local authority.

• internal/api/handlers.go:925-936, internal/api/routes.go:922-934 — conversation_id is supported for vector/hybrid searches but missing from the semantic-filter allowlist and OpenAPI parameters. In FTS mode it is silently ignored, and clients cannot discover the filter. Add it to both lists and either reject it in FTS mode or apply it there.

Review 2 found no concrete security issues.

---

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

[roborev-ci]

roborev: Combined Review (6bce506)

Verdict: Two medium-severity issues require attention; no critical or high-severity findings were reported.

Medium

• internal/search/parser.go:571-584, internal/store/api.go:649-659 — Conflicting conversation scopes become a non-nil empty slice, which is treated as no filter. FTS searches may return messages from unrelated conversations. Preserve explicit-empty scopes as match-nothing and add a 1=0 predicate to the store query.

• internal/sqliteutil/dsn.go:12-17, cmd/msgvault/cmd/build_cache.go:377-390 — SQLite file-URI normalization is applied to store.Open, but cache building still uses the raw URI for filesystem and SQLite operations. Configurations using file:///... may import successfully but fail during cache refresh. Resolve file DSNs before cache and read-only SQLite operations.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 17m22s

[roborev-ci]

roborev: Combined Review (a3b260a)

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 16m36s

[roborev-ci]

roborev: Combined Review (6afd3f4)

Verdict: Two medium-severity correctness issues were identified.

Medium

• internal/search/formatter.go:30-32 — Conflicting conversation scope becomes a non-nil empty ConversationIDs slice, but Format emits no token. Remote deep/body searches and TUI aggregate statistics can therefore include unrelated conversations. Propagate explicit empty scopes as match-nothing or add a serialized match-none representation.

• internal/store/sender_repair.go:94-100 — A corrupt or undecodable compressed MIME row aborts the entire sender-repair scan, preventing valid higher-ID candidates from being processed. Treat decode failures as unresolved candidates, record the failure, and continue advancing the keyset cursor.

---

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

[roborev-ci]

roborev: Combined Review (d79b942)

Verdict: Two medium-severity issues remain in SQLite URI handling.

Medium

• internal/store/store.go:285-301 — OpenReadOnly bypasses sqliteutil.ResolveDSN, causing valid file: database URIs to be misreported as missing by os.Stat; direct query-string concatenation also breaks URIs with existing parameters. Resolve the DSN first and use the query-aware SQLite parameter helper.

• internal/vector/sqlitevec/fused.go:497-511 — Fused search opens b.mainPath without DSN normalization. Supported Windows file://C:%5C... URIs may therefore resolve differently from the primary store. Normalize mainPath before opening the fused connection and add hybrid-search URI coverage.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 17m43s

[roborev-ci]

roborev: Combined Review (9c032f2)

Verdict: One medium-severity issue found; no critical or high-severity findings.

Medium

• Semantic search rejects conversation filters — internal/query/engine.go:114-120, internal/tui/keys.go:110-113
  Semantic search still rejects any ConversationID filter, disabling TUI semantic search within conversation drill-downs despite new conversation filtering support. Remove ConversationID from the unsupported-filter predicate and error path.

---

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

[roborev-ci]

roborev: Combined Review (962ad28)

Verdict: One medium-severity issue found; no security issues identified.

Medium

• internal/tui/model.go:917-926 — hasDrillFilter does not recognize MessageFilter.ConversationID, causing conversation-only drill scopes to be discarded during message loading and deletion setup. Operations may therefore escape the intended conversation scope.
  • Fix: Include m.drillFilter.ConversationID != nil and add coverage for reload and deletion with a conversation-only scope.

---

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

[roborev-ci]

roborev: Combined Review (82d75e5)

Verdict: 2 medium-severity issues require attention.

Medium

• internal/vector/hybrid/filter.go:102-104 — An explicitly empty ConversationIDs scope is omitted, causing hybrid/vector searches to treat a match-nothing scope as unrestricted. Preserve non-nil empty scopes using the vector no-match sentinel.

• internal/importer/eml_import.go:115-147, 252-256 — Per-file errors mark the sync as failed, but subsequent runs check only GetActiveSync, so the failed checkpoint is ignored and the entire tree is reprocessed. Resume from the latest running or failed checkpoint, or retain failed imports as resumable.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 20m41s

[roborev-ci]

roborev: Combined Review (f7025a1)

Verdict: Medium-severity cache invalidation issue found; no security issues identified.

Medium

• internal/importer/eml_import.go:198-202 — When processing an exact duplicate, the importer adds a mailbox label but only increments MessagesSkipped. Because AddMessageLabels does not invalidate the analytics cache, rebuildCacheAfterWrite may skip rebuilding when no messages are added or updated. Newly imported mailbox labels can therefore be missing from Parquet-backed searches.
  • Suggested fix: Make label-link changes update a cache-visible revision, or force an analytics rebuild whenever duplicate processing adds a label.

---

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

[roborev-ci]

roborev: Combined Review (cf8b262)

Verdict: Two medium-severity integration and safety issues were identified.

Medium

• cmd/msgvault/cmd/attachment_maintenance.go:316-341 — import-eml writes attachment files but is not classified as an attachment-producing command. Daemon-proxied imports therefore skip the attachment mutation lease and automatic packing. Add it to the producer classification and cover the daemon path.

• cmd/msgvault/cmd/repair_senders.go:51-57 — The read-only repair dry run opens the database through a writable initializer, which may execute startup migrations and modify rows before scanning. Use a read-only, no-migration initialization path when --apply is not specified.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 15m33s

[roborev-ci]

roborev: Combined Review (da1d294)

Verdict: Two medium-severity correctness issues require attention; no security vulnerabilities were identified.

Medium

• cmd/msgvault/cmd/import_eml.go:72-75 — If message import succeeds but runEMLPostImportMigrations fails, rebuildCacheAfterWrite is skipped, leaving enabled analytics caches stale. Rebuild the cache on this error path or defer rebuilding until all post-import steps complete.

• internal/importer/eml_import.go:115-123 — EML resume selects the latest checkpoint by source only. A failed/running checkpoint from another importer may be decoded as an EML checkpoint and abort the import. Persist and filter checkpoints by import type, or ignore incompatible checkpoints and start a new EML run.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 20m31s

[roborev-ci]

roborev: Combined Review (09f47eb)

Verdict: 2 medium-severity issues require attention; no high or critical issues found.

Medium

• internal/store/messages.go:2276-2280 — AddMessageLabels increments DerivedDataRevision even when INSERT OR IGNORE adds no labels, causing unnecessary cache invalidation and potentially triggering a full rebuild. Bump the revision only when rows are actually inserted.

• cmd/msgvault/cmd/serve.go:1665-1685 — The new gc command removes message and attachment database rows but is not recognized by attachmentRemovalCommand, so physical attachment cleanup/repacking may be skipped. Include gc in the attachment-removal path or invoke cleanup explicitly after successful GC.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 26m32s

[roborev-ci]

roborev: Combined Review (0e77fa3)

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 17m14s

[wesm]

looking

[wesm]

Reviewed the full diff against the five linked issues (#158, #175, #192, #200, #214). Findings: 1 P2, 3 P3, 3 P4 — 4 fixed in 5d79572, 3 dismissed/informational.

Fixed

• P2 — repair-senders planned repairs its own apply step rejects. Planning only checked that the recovered From was non-empty, while ApplySenderRepairContext also requires a single valid bare address. A salvaged address like x..y@example.test (reachable through the fallback header parser) showed up as Repairable in the dry run, then failed during --apply and made the command exit non-zero. Planning now applies the same rule (store.ValidateRepairSender) and counts these as unresolved; covered by a new test case.
• P3 — decode failures were invisible. Candidates whose raw MIME failed to decode were silently folded into the Unresolved count. They are now logged with the underlying error.
• P3 — real company identifier in test fixtures. noreply@electrolux.com violated the synthetic-test-data rule; replaced with fridgeco.example.
• P4 — import-eml overcounted mailboxes on cancellation. An interrupted mailbox was counted as imported in the Mailboxes: N/M summary. Now only fully processed mailboxes count; asserted in the cancellation test.

Dismissed / informational

• P3 — gc confirms before showing plan counts. The client-side confirmation happens before the daemon prints "Source-deleted messages to purge: N", inverted relative to delete-deduped's plan → confirm → execute flow. Making gc plan-first would require dedicated daemon plan/execute endpoints; the current design is explicitly tested, creates a backup by default, and re-validates the exact planned population inside the delete transaction. Left as a possible follow-up.
• P4 — PR description overstated the list-accounts change. The TYPE column and JSON type field already existed on main; this PR adds test coverage. I edited the PR body to say so.
• P4 — import-eml passes a zero fallback date, so messages with unparsable Date headers land undated. repair-dates covers this, and file mtimes are unreliable after copies; acceptable.

Verification

• go build, go vet, and make lint-ci clean (one dogsled finding in my own first fix iteration, resolved before commit).
• Full go test ./... (fts5 + sqlite_vec tags) green. Two suites fail only under the sandboxed runner (TestOpenAPIClientArtifactUpToDate needs network for codegen; the peoplesweep Codex isolation tests spawn fixture processes) and pass when run normally — the regenerated OpenAPI client artifacts are confirmed in sync with api/openapi.yaml.

Notes for the record: DELETE FROM messages in gc relies on ON DELETE CASCADE child cleanup, which is safe here because the store opens SQLite with _foreign_keys=ON; the FTS cleanup-by-rowid matches the existing messages_fts convention; and gc is classified as an attachment-removal command so the daemon runs blob maintenance after it.

[roborev-ci]

roborev: Combined Review (5d79572)

Verdict: One medium-severity issue requires attention; no critical or high-severity findings.

Medium

• SQLite DSN normalization is bypassed during cache rebuilds — cmd/msgvault/cmd/build_cache.go:850, called from remove_account.go:251, repair_dates.go:174, and repair_encoding.go:101
  These callers pass cfg.DatabaseDSN() directly to buildCacheLocked, bypassing SQLite DSN normalization. File URIs—especially Windows forms such as file://C:%5C... or URIs with query parameters—may target the wrong path or fail to open during cache rebuilds. Normalize the DSN inside buildCacheLocked or at each caller.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 40m52s

[wesm]

Verified and fixed the roborev medium finding in 1bab95e.

Confirmed reachable: repair-dates, repair-encoding, and remove-account pass the raw configured DSN into buildCacheLocked, which appends ?mode=ro to a DSN that may already carry query parameters, hands the literal URI to the DuckDB sqlite attach, and runs filepath.Dir on the URI string for its staging directory. With a file: URI database_url (a configuration this PR's other changes explicitly support), those rebuilds fail — reproduced with a new regression test that fails without the fix (DuckDB cannot attach the URI) and passes with it.

Fix: normalization now lives inside buildCacheLocked itself so every current and future caller gets a plain filesystem path; the now-redundant copy in buildCacheImpl is removed. make lint-ci clean, full cmd/msgvault/cmd package green.

[roborev-ci]

roborev: Combined Review (1bab95e)

Verdict: Two medium-severity issues require attention; no high or critical findings were identified.

Medium

• cmd/msgvault/cmd/repair_senders.go:139 — Sender repair plans only parsed.From[0], silently omitting additional recovered From addresses from the envelope snapshot. Treat multiple recovered addresses as unresolved, or persist the complete set while using the first for sender_id.

• internal/importer/eml_import.go:162 — Fatal errors after sync-run creation can return without marking the run failed, leaving it stuck as running. Centralize cleanup to call FailSync for fatal errors while preserving running for intentional cancellation or resumption.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 22m30s

[wesm]

Verified and fixed both roborev medium findings in e98496a.

Multi-From envelope loss (repair_senders.go) — confirmed. Planning kept only parsed.From[0], so a repaired multi-From message lost the remaining addresses from its envelope snapshot, unlike a normally ingested message (buildRecipientSet persists every From row). The repair now installs the complete recovered set: the first address (still strictly validated as sender) becomes sender_id, and the remaining rows follow the ingest path's lenient rules so a repaired message matches an ingested one. The new TestApplySenderRepairPersistsEveryRecoveredFromAddress fails against the single-address behavior and passes with the fix (verified both ways).

Stuck running sync run (eml_import.go) — confirmed. Fatal store errors after StartSync (label ensure, dedup lookups, checkpoint saves) returned without marking the run failed. This matters beyond bookkeeping: GetLatestCheckpointedSyncByType treats a running row with a cursor as resumable state, so a run abandoned by a fatal error looked identical to an interrupted one. Fatal paths now call FailSync via the same failSync closure pattern the mbox and pst importers use; intentional cancellation still leaves the run running so the checkpoint stays resumable.

make lint-ci clean; internal/store, internal/importer, and cmd/msgvault/cmd packages green (the store package needs ~13 min on this machine, so it was run with an extended timeout).

[roborev-ci]

roborev: Combined Review (e98496a)

Verdict: One medium-severity issue requires attention; no high or critical issues were found.

Medium

• Attachment cleanup leak — internal/store/gc.go:151-154; cmd/msgvault/cmd/attachment_maintenance.go:91-98
  GC deletes attachment database rows but does not remove corresponding loose attachment blobs. Loose-attachment repacking is disabled, and its error is ignored, leaving unreachable files consuming disk space. Run a loose-blob sweep after successful GC under the appropriate attachment lock and surface cleanup failures.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 24m3s

[wesm]

Verified and fixed the roborev medium finding in 4d003fa.

Attachment blob leak after gc — confirmed. The message delete cascades away attachments rows, but the loose blob files those rows were the last reference to stayed on disk: the daemon's post-command runAutomaticRepack reclaims packed dead bytes only, and is a no-op entirely when [data].loose_attachments disables packing. That contradicts gc's purpose (#214 is about reclaiming disk). One correction to the finding's framing: the ignored repack error is deliberate best-effort maintenance (it logs and streams a stderr warning with a retry command), consistent with runAfterSuccessfulAttachmentIngest — the real gap was the missing loose sweep, not the error handling.

Fix: after successful delete + VACUUM, gc runs the same sweepUnreferencedLooseMedia pass purge-excluded-media uses, reports the removed count, and surfaces sweep failures in the command error (while still reporting that deletion and compaction succeeded). No new locking needed — the daemon already holds the attachment mutation lease around the whole gc subprocess, since gc is classified as an attachment-removal command; that classification also keeps the automatic repack for packed dead bytes. The extended test seeds one blob referenced only by the purged message and one shared with an active message, and asserts the orphan is swept while the shared blob survives; verified it fails without the sweep.

make lint-ci clean; gc, store GC, and purge-excluded-media tests green.

[roborev-ci]

roborev: Combined Review (4d003fa)

Verdict: One medium-severity issue requires attention; no security vulnerabilities were identified.

Medium

• cmd/msgvault/cmd/gc.go:85-89 — GC returns early when no source-deleted messages remain, preventing the loose-media sweep and retries of orphan cleanup after database rows were deleted but blob removal failed. Allow the loose-media sweep to run independently, with appropriate confirmation and reporting.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 48m0s

[roborev-ci]

roborev: Combined Review (845f796)

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 41m0s