Pending import retry survives an app restart

[wingleeio]

Follow-up to #79, addressing the remaining blocker from the final review: the pending-import retry path disappeared on app restart, so "Later" promised a recovery route the next launch couldn't deliver.

What changed

Persisted intent. The local-import.json marker entry now records pendingRetry alongside the outcome: a run whose summary carries errors sets it, a clean retry clears it. It rides the same atomic publish (temp file + fsync + rename) added in #79, so it's crash-safe, and LocalImportStatus surfaces it.

Restored on boot. The shell runs a one-shot LocalImportStatus probe per attached synced runtime (only from Ready + Idle flow, re-armed across runtime replacement). If the engine reports a recorded pending retry, the shell restores the postponed ImportFailed state — the account menu regains its "Finish sync setup" entry and reopening shows the retry dialog, exactly what the pre-restart "Later" promised. Accounts with no recorded failure — including "Start fresh" users, whose marker never gains the flag — see nothing, so there is no boot-time nag for people who declined the import by choice.

Testing

Per the review's ask, the regression test is at the bootstrap level, not a reducer over one in-memory Shell — pending_import_retry_survives_an_app_restart runs real runtimes end to end:

1. seed a local profile with sessions (one journaled),
2. boot the synced profile from a saved session (EngineHandle::bootstrap),
3. fail the import via a planted journal obstruction (summary carries the error, marker records the intent),
4. stop the runtime and bootstrap a fresh one — the app restart,
5. assert LocalImportStatus.pendingRetry is still true on the restarted runtime and that the restored state's menu action is the reopen entry,
6. retry cleanly on the restarted runtime and assert the persisted intent clears.

The engine failure-injection test from #79 additionally asserts the marker flag both ways at the importer level.

Validation: full workspace suite 848 passed / 0 failed; clippy and rustfmt --check clean on touched files.

🤖 Generated with Claude Code

[wingleeio]

Blocking review

The normal partial-item failure path is covered well, but the restart-durable guarantee does not yet hold for several real failure paths:

1. Not every error summary records pendingRetry. LocalImporter::run can return through ? before reaching record_import (open_source, registry reads, etc.); the RPC wrapper then synthesizes an error summary, but no pending marker is written. Also, pending_retry is computed before record_import, so a marker-persistence failure is appended to the summary only after the attempted entry was built with pendingRetry: false—and in practice no new marker was published at all. Thus the existing marker_persistence_failure_is_an_import_error case still loses its retry path after restart, directly contradicting “a run whose summary carries errors sets it.” Please persist pending intent before the fallible import starts and clear it only after a clean terminal outcome, while retaining a discoverable manual recovery route for cases where the marker itself cannot be written. Add coverage for a top-level importer error and the existing marker-write obstruction, asserting the restarted status/UI rather than only the summary.

2. A clean no-source retry never clears a prior pending flag. The open_source() == None branch emits a clean summary and returns before record_import. If a failed import recorded pendingRetry: true and the local profile or registry is subsequently removed, Retry reports success, but every later boot restores ImportFailed forever. Clear the pending outcome on this clean no-op path and add a regression test.

3. The boot probe permanently suppresses recovery after one failed or mistimed status call. import_status_checked is set before the RPC, and any RPC/status error is collapsed into pending = false without re-arming. A pending marker paired with an unreadable local store makes LocalImportStatus fail while scanning availability, so the account-menu recovery entry remains hidden for the entire runtime. The same happens if a true result arrives while the flow is briefly non-Idle: it is discarded, but the checked flag stays set. Please return/recover the marker state independently from availability scanning and retry or re-arm failed/deferred probes.

Validation on 01cd61ebccdd6f9ea4a4efb93f1d93136e6779a7:

• cargo test -p zeron-engine --test local_import: 8 passed.
• cargo test -p zeron-ui --lib shell::tests::: 31 passed.
• cargo fmt --all -- --check: passed.
• git diff --check: passed.
• Prospective merge with current origin/main: conflict-free.
• No malicious, backdoor-like, dependency, or dangerous execution changes found.

This is a blocking, non-approving review.

[wingleeio]

All three addressed in 34277c8.

1. Arm first, clear only on clean outcomes. run now persists pendingRetry: true before any fallible work (creating a zero-count entry if none exists) and it is cleared only by a clean terminal outcome — the final record_import on a zero-error run, or the clean no-source path. Every failure class you listed now leaves the armed flag on disk: ? exits (open_source, registry reads), per-item failures, and a failed final marker write (the flag armed at the top survives it — the previous circularity is gone). The one unpersistable case is the arming write itself failing; that error still reaches the summary (in-session ImportFailed), and the new manual route below is the discoverable recovery for it. New test top_level_import_error_arms_pending_retry_across_restart corrupts the source store, asserts run errors, and asserts a restarted runtime still reports the pending retry; the marker-obstruction test now also asserts the restarted status.

2. No-source retry clears the flag. The open_source() == None branch is treated as the clean terminal outcome it is: it clears a previously armed flag (and reports an error if even that write fails). New test clean_no_source_retry_clears_a_stale_pending_flag: arm via a real failure, delete profiles/local, retry → clean summary and the flag is gone, so boots stop restoring ImportFailed.

3. The probe can no longer swallow recovery.

• The outcome logic is a pure, tested function (import_probe_outcome): a failed RPC defers (guard stays unset, bounded at 4 attempts per runtime to prevent hot loops), and a pendingRetry: true that lands while the flow is briefly non-Idle also defers instead of being dropped with the guard set. Only a definitive answer settles the probe.
• Engine-side, LocalImportStatus now reports marker state unconditionally — the availability scan is best-effort (zeros + a warn on error), so an unreadable local store can't make the status call fail and hide a recorded pending retry. Covered by status_reports_pending_retry_despite_unreadable_source.
• The account menu gains a standing "Import local work" entry (synced scope, idle flow, importable rows reported by the probe — show_manual_import_row, tested). This is the marker-independent recovery route: it consults LocalImportStatus availability, so it works even in the arming-write-failed case, and it never appears for accounts with nothing to import.

Validation on 34277c8: full workspace suite 853 passed / 0 failed; cargo test -p zeron-engine --test local_import 11 passed; cargo test -p zeron-ui --lib shell::tests:: 33 passed; clippy and rustfmt --check clean on touched files.

[wingleeio]

Follow-up blocking review

The arm-first/clean-clear importer changes address the normal and top-level failure paths well. Three recovery/UI gaps remain:

1. The claimed markerless recovery route is hidden in the marker-write failure case. marker_persistence_failure_is_an_import_error imports both chats successfully (imported == 2) and then fails only while publishing the marker. After restart, pendingRetry is necessarily false and target deduplication reports availableChats == 0 / availableSpaces == 0, because the rows already landed. show_manual_import_row requires availability greater than zero, so Import local work is not shown. The user therefore still cannot retry the marker publication, and imported transcript attachments lose their read-root grant after restart. The updated test never asserts availability or the menu predicate; its comment even notes the rows are already present. Please base markerless recovery on the missing grant/source state rather than only unimported rows, and add the exact restart-to-visible-menu assertion.

2. Defer does not actually schedule another probe. maybe_restore_pending_import only runs from on_state_changed. When the RPC fails, or a pending result arrives while the flow is temporarily busy, the callback leaves import_status_checked unset but neither schedules a bounded retry nor invokes the probe when the flow returns to Idle. If the app state is otherwise stable, there is no later callback and recovery remains hidden indefinitely; the four-attempt limit is never exercised. Please add an actual backoff/retry task or explicitly trigger probing on the relevant Shell flow transitions, with an async lifecycle test rather than only testing the pure enum mapping.

3. Successful manual imports leave stale availability cached. A settled probe sets import_status_checked = true and caches local_import_available, but spawn_local_import and a clean summary never invalidate or refresh either field. After using Import local work successfully, returning to Idle continues to show that row forever even though the engine now reports zero available items. Clear/re-probe the cache after terminal import outcomes and cover the menu disappearing after success.

Validation on 34277c8ef24ed25e630cd60a4ad5ecef6aef6937:

• cargo test -p zeron-engine --test local_import: 11 passed.
• cargo test -p zeron-ui --lib shell::tests::: 33 passed.
• cargo fmt --all -- --check: passed.
• git diff --check: passed.
• Prospective merge with current origin/main: conflict-free.
• No malicious, backdoor-like, dependency, or dangerous execution changes found.

This remains a blocking, non-approving review.