feat(status): report whether a sync run actually measured its source by eserie · Pull Request #1149 · kenn-io/msgvault

eserie · 2026-10-06T20:49:36Z

Depends on #1142.

Reports whether Apple-source sync runs actually measured their source, records missing or unreadable attempts as unmeasured, and exposes that state and reason through the API and Sources UI.

roborev-ci · 2026-10-06T20:51:58Z

roborev: Combined Review (`4dfdc3e`)

Verdict: Changes require fixes for 1 finding.

Medium

internal/imessage/client.go:250: When all 500 messages in a full page fail to archive, the error path skips advancing lastROWID. The importer repeatedly fetches the same page, inflates skipped counters, and holds the daemon operation gate indefinitely, blocking other scheduled and interactive work until cancellation.

Fix: Advance lastROWID for every examined message, including failures. Add a behavioral regression test with a full page of rejected messages that verifies termination and counts each skip once.

Reviewers: 2x codex, codex (security) | Synthesis: codex, 7s | Total: 3m7s

roborev-ci · 2026-10-07T01:09:39Z

roborev: Combined Review (`8b55c6b`)

Verdict: Changes require fixes for 1 finding.

Medium

web/src/lib/sources/labels.ts:38: A scheduled WhatsApp import can record status='completed', outcome='unmeasured', and reason='writer_not_running'. The Sources page checks only status, displaying a green 'Completed' result, and its details control hides the reason, so users cannot see that the run did not measure a live source.

Fix: Handle outcome='unmeasured' before the completed-status branch, expose its reason in source details, and add a UI test for a completed run with an unmeasured outcome.

Reviewers: 2x codex, codex (security) | Synthesis: codex, 7s | Total: 4m41s

roborev-ci · 2026-10-07T03:33:22Z

roborev: Combined Review (`fe188b7`)

Verdict: Changes require fixes for 2 findings.

Medium

internal/imessage/client.go:506: If an existing message is updated and a later body or raw-data operation fails, the committed changes are counted only as a skip. MessagesUpdated remains zero and the run succeeds, potentially leaving messages.parquet with stale fields because cache invalidation relies on update counters.

Fix: Make each message import atomic, or account for committed changes when a later operation fails. Add coverage for a failure after an existing message row is updated.
cmd/msgvault/cmd/import_imessage.go:74: A manual iMessage import targeting a missing chat.db returns from resolveChatDBPath before openImessageClientRecorded runs. No unmeasured/source_missing run is recorded, so source status continues showing the previous result despite the failed attempt.

Fix: Return the intended path from path resolution and let openImessageClientRecorded handle missing-file checks and recording, or explicitly record the unmeasured run before returning the error. Add coverage through the manual import path.

Reviewers: 2x codex, codex (security) | Synthesis: codex, 8s | Total: 3m47s

roborev-ci · 2026-10-07T05:56:49Z

roborev: Combined Review (`aad1758`)

Verdict: Changes require fixes for 2 findings.

Medium

cmd/msgvault/cmd/import_imessage.go:215: When an existing iMessage changes but a later write fails, failAfterWrite increments MessagesUpdated without incrementing MessagesImported. Subtracting these counters records negative messages_added, which ProjectSourceState rejects, making operation history containing that run unreadable.

Fix: Track newly added messages explicitly, independently of successful imports and partial updates. Add a regression through importImessageRecorded covering an existing message changed before a later write fails.
internal/config/config.go:1053: Both new Apple database paths bypass configuration path normalization. Paths beginning with '~' are treated literally and can report source_missing despite the file existing; relative paths resolve against the daemon's working directory instead of the explicit configuration directory.

Fix: Apply expandPath to both Apple database path fields and resolveRelative when loading an explicit configuration, preserving empty values for defaults.

Reviewers: 2x codex, codex (security) | Synthesis: codex, 9s | Total: 3m56s

roborev-ci · 2026-10-07T22:00:25Z

roborev: Combined Review (`0e3b6f2`)

Verdict: Changes require fixes for 2 findings.

High

internal/imessage/client.go:482: Reimporting an archived message with NULL text and a nonempty attributedBody that cannot be decoded erases its stored body and search entry. The unconditional body write treats decoding failure as confirmed deletion; previously, the empty result preserved archived content.

Fix: Distinguish decoding failure from successfully decoded empty content. Preserve archived content or skip the message on decoding failure, while still clearing bodies when the source actually contains no content. Cover both cases with behavioral tests.

Medium

cmd/msgvault/cmd/scheduled_apple_imports.go:57: If ChatStorage.sqlite disappears after the readability check but before Import opens it, Import returns source_missing before creating a sync run. This branch records nothing, so source status reports the previous completed run instead of the unmeasured attempt.

Fix: Classify importer errors too. Record an unmeasured attempt for pre-run source-access failures, and update the existing run when access fails after it starts. Cover the post-check disappearance path.

Reviewers: 2x codex, codex (security) | Synthesis: codex, 11s | Total: 5m29s

roborev-ci · 2026-10-14T23:53:00Z

roborev: Combined Review (`5b46bcd`)

Verdict: Changes require fixes for 4 findings.

High

internal/imessage/parser.go:177: Truncated or invalid UTF-8 in a streamtyped payload can be trimmed to empty while returning decoded=true. Reimporting it with NULL text then clears the previously archived body and search entry.

Fix: Return decoded=false for truncated payloads or invalid UTF-8, and add a reimport regression test proving the existing body is preserved.
cmd/msgvault/cmd/scheduled_apple_measurement_test.go:195: TestOpenImessageClientRecordedDeniedIsUnmeasured duplicates the permission-error execution and persisted failed/unmeasured/fda_denied assertions in TestScheduledIMessageUnreadableIsUnmeasuredNotZero. It does not invoke the manual command, so it adds no manual-path coverage.

Fix: Delete TestOpenImessageClientRecordedDeniedIsUnmeasured and retain the scheduled-path test.
internal/imessage/client_test.go:345: TestReimportCountsUpdateWhenLaterWriteFails duplicates the edited-message phase of TestImportImessagePartialWriteCounters: both reject message_raw inserts with the same SQLite trigger and assert one update plus two errors. The broader test also verifies persistence and operation-history compatibility.

Fix: Delete TestReimportCountsUpdateWhenLaterWriteFails and retain TestImportImessagePartialWriteCounters.

Medium

internal/api/scheduler_jobs.go:122: Every apple_messages source advertises the singleton sync job, but runScheduledIMessage imports only the source selected by resolveImessageSource. With both local and legacy source rows, Sync now on the unselected row imports another source and never updates the requested row.

Fix: Advertise and accept sync requests only for the resolved source, or reconcile alias rows before exposing the singleton job.

Reviewers: 3x codex, codex (security) | Synthesis: codex, 12s | Total: 10m30s

roborev-ci · 2026-10-15T00:35:34Z

roborev: Combined Review (`f724ced`)

Verdict: Changes require fixes for 4 findings.

High

internal/imessage/parser.go:177: A truncated or invalid UTF-8 streamtyped payload can be trimmed to an empty string while decoding reports success. On reimport, this bypasses the preservation guard and clears the previously archived body and search entry.

Fix: Return decoding failure for truncated or invalid UTF-8 payloads, and add a reimport regression verifying that existing archived content survives.
cmd/msgvault/cmd/scheduled_apple_measurement_test.go:195: TestOpenImessageClientRecordedDeniedIsUnmeasured duplicates the scheduled denial test by injecting the same permission error and asserting the same persisted failure and measurement. Despite its comment, it does not exercise the manual command.

Fix: Delete the redundant test, retaining the scheduled denial test and the separate test that invokes the manual command.

Medium

internal/store/sync_measurement.go:124: Recording a missing or denied source as a failed sync advances the cache's interrupted-sync watermark even though no messages were modified. Repeated unreadable-source schedules therefore force unnecessary full archive rebuilds.

Fix: Identify attempts that made no archive writes and exclude them from the cache interruption watermark, while retaining invalidation for imports that partially wrote data.
internal/api/scheduler_jobs_test.go:95-96: The scheduler test signals that the second callback started before it executes its cancellation select. Calling Stop immediately afterward can make both select branches ready, allowing legitimate scheduler shutdown to increment cancelled and spuriously fail the request-detachment assertion.

Fix: Signal completion after the second callback passes its select, and wait for that signal before stopping the scheduler.

Reviewers: 3x codex, codex (security) | Synthesis: codex, 13s | Total: 9m24s

roborev-ci · 2026-10-15T03:00:10Z

roborev: Combined Review (`61786ce`)

Verdict: Changes require fixes for 4 findings.

High

cmd/msgvault/cmd/scheduled_apple_imports_test.go:177: The unchanged iMessage reimport test counts builder calls only when the staleness predicate reports FullRebuild, while its replacement builder always forces a rebuild. Unnecessary builds can go uncounted and repair the cache before freshness assertions.

Fix: Count every builder invocation unconditionally and assert that an unchanged import starts no additional build. Retain the real cache build and changed-message case.
internal/imessage/client_test.go:339: The malformed attributed-body reimport test rebuilds the search index before checking search results, masking regressions that preserve the body but invalidate its existing search entry.

Fix: Populate and verify the search index before malformed imports, then check search results after each import without rerunning BackfillFTS. Keep the body-preservation assertions.

Medium

cmd/msgvault/cmd/scheduled_apple_imports.go:76: A scheduled WhatsApp run that edits a cached message and imports a new message can leave the edited message’s old snippet and size cached. The importer counts rewrites as messages_added without setting messages_updated, allowing an incremental cache append when no other invalidation signal changes.

Fix: Distinguish additions from updates in the Apple importer and persist messages_updated. Add coverage for an edit plus an addition against an existing cache.
cmd/msgvault/cmd/scheduled_apple_imports.go:66: Successful scheduled WhatsApp imports never advance sources.last_sync_at, so account and source-status responses continue reporting an unknown or outdated last-sync time.

Fix: Call TouchSourceLastSyncAt after a successful import and handle its error while preserving the Apple chat-marker cursor. Add a test verifying that successful scheduled imports advance freshness.

Reviewers: 3x codex, codex (security) | Synthesis: codex, 14s | Total: 11m56s

roborev-ci · 2026-10-15T03:53:56Z

roborev: Combined Review (`98ac43a`)

Verdict: Changes require fixes for 4 findings.

High

cmd/msgvault/cmd/import_imessage_sync_runs_test.go:111: TestImportImessageParallelNeverOverlaps discards worker errors and requires only one success. Its polling assertion can pass even when competing imports incorrectly supersede live runs, because StartSync recovery keeps at most one row running.

Fix: Replace the polling stress test with a deterministic admission test: hold an active source run, invoke importImessageRecorded, require ErrSyncAlreadyActive, and verify the original run remains running with no additional run created.
internal/imessage/client_test.go:354: TestReimportCountsUpdateWhenLaterWriteFails duplicates the SQLite failure scenario covered by TestImportImessagePartialWriteCounters at cmd/msgvault/cmd/import_imessage_sync_runs_test.go:239. The command test exercises the same implementation and also verifies persistence, so the additional test provides no unique failure detection.

Fix: Remove TestReimportCountsUpdateWhenLaterWriteFails and retain the partial-write counter test.

Medium

internal/imessage/client.go:488: For an existing iMessage row with empty plain text and a nonempty attributedBody, extraction ignores attributedBody. The unconditional body write then erases archived text and its search entry even though source content remains available.

Fix: Decode attributedBody when plain text is nil or empty. Clear archived content only after confirming the source body is empty, and preserve it when decoding fails.
cmd/msgvault/cmd/scheduled_apple_imports.go:61: Scheduled WhatsApp imports discard duplicate-stanza warnings through NullProgress. The importer increments summary.Errors without persisting errors_count, so affected messages are skipped while source status reports Completed without errors.

Fix: Persist import errors in the sync-run counters and route importer warnings to the daemon logger.

Reviewers: 3x codex, codex (security) | Synthesis: codex, 14s | Total: 12m7s

roborev-ci · 2026-10-15T04:43:49Z