fix(auto-sync): retry setup on next launch when any step fails

[lavaman131]

Summary

Fixes a bug where a partial setup failure was silently marked as complete, preventing automatic retry on subsequent launches. The version marker is now only written when all setup steps succeed — on partial failure, the next launch re-runs the full setup (all steps are idempotent, so re-running already-succeeded steps is harmless).

Key Changes

• src/services/system/auto-sync.ts: Move markSynced() call inside a failures.length === 0 guard so the .synced-version marker is only written on full success
• Failure message: Updated from instructing users to manually reinstall (bun install -g @bastani/atomic) to informing them that setup will automatically retry on next launch — no manual intervention needed
• Module docstring: Updated to document the new write-on-success semantics

Behavior Before vs. After

Scenario	Before	After
All steps succeed	Marker written; setup skipped on next launch ✓	Same ✓
One or more steps fail	Marker written; partial failures silently ignored ✗	Marker not written; all steps retry on next launch ✓

Notes

• No breaking changes — this only affects retry behavior when setup is incomplete
• All setup steps (tmux/psmux, global agent configs, @playwright/cli, @llamaindex/liteparse, global skills) are idempotent, making the retry-all-on-failure approach safe and cheap

[claude]

Review

Small, well-scoped fix that addresses a real bug — partial failures were being silently latched as "done". The new behavior (write marker only on full success) is the right contract. Doc comment and user-facing message are both updated to match. The change is also exactly one focused commit on one file, which makes it easy to reason about.

I verified the idempotency claim for all five steps, and it holds:

• ensureTmuxInstalled early-returns when tmux/psmux/pmux is on PATH (src/lib/spawn.ts:140).
• upgradePlaywrightCli / upgradeLiteparse go through upgradeGlobalPackage which runs bun install -g --trust <pkg>@latest (src/lib/spawn.ts:105-115) — bun handles already-current packages cleanly.
• installGlobalAgents is pure copyDir/copyFile (src/services/system/agents.ts:61-94) — overwriting is fine.
• installGlobalSkills calls skills add ... -y then skills remove ... -y (src/services/system/skills.ts:47-77) — both tolerate re-runs.

Concerns worth flagging

1. Persistent-failure UX regression (medium)

If a step fails for a reason that doesn't self-resolve (no sudo for apt-get install tmux, corporate proxy blocks the npm registry, no bunx/npx on PATH for skills), then every non-info CLI invocation will now re-run all five steps and re-print the banner + summary. The old behavior was annoying-but-quiet after the first run; the new behavior is correct-but-noisy forever. Worth considering one of:

• An escape hatch env var (e.g. ATOMIC_SKIP_AUTO_SYNC=1) so a stuck user can move on.
• A bounded retry counter persisted alongside the marker (e.g. give up and mark synced after N consecutive failures, with a one-line note pointing at the recovery command).
• Caching per-step success so a successful step isn't re-attempted when only one other step is failing.

The PR description argues "they are idempotent, so re-running already-succeeded steps is harmless" — that's true for correctness, but each bun install -g @latest still hits the network, and ensureTmuxInstalled's Linux fallback chain can take a couple of seconds even when the early-PATH-check passes. Multiplied across every CLI launch, this is felt.

2. Concurrent invocations (low, pre-existing)

Two atomic processes started in parallel will both attempt the full setup — and now they'll both keep retrying until success. Pre-existing, but the new "retry forever" behavior amplifies the window. A simple lockfile next to .synced-version would address it.

3. Test coverage (low)

There's no auto-sync.test.ts. The behavior change here — "marker is written iff failures.length === 0" — is exactly the kind of conditional that a single unit test pins down forever. Given ATOMIC_SETTINGS_HOME already exists to make this testable (src/services/system/auto-sync.ts:49), a small spec covering (a) all-pass writes the marker, (b) any-fail does not, would be cheap insurance against future regressions.

Nits

• src/services/system/auto-sync.ts:112 — the failures constant is now computed before displayBlockBanner / printSummary, which is fine, but it does mean the early-return-via-marker logic and the user-facing output are interleaved. Not a blocker; just slightly harder to skim than before.
• The new retry message ("To retry now, re-run your command") is good, but for a user whose setup is failing due to environment (e.g. missing sudo), it doesn't hint at how to diagnose. Consider linking to a troubleshooting doc or pointing at the per-step error already shown above.

Verdict

LGTM — bug fix is correct and the comment/message updates accurately describe the new contract. Worth following up on concern #1 (escape hatch or bounded retries) before this lands in front of users with locked-down environments, but that can be a separate PR.