fix(cli): retry root bootstrap contention

[bcdonadio]

Summary

Retry transient, authenticated live-owner root-bootstrap lock contention across CLI commands so short overlaps no longer surface as immediate failures. Preserve the existing fail-closed handling for stale, ambiguous, tampered, or unsafe lock states and provide actionable diagnostics when bounded retries are exhausted.

Fixes #673.

Changes

• add one CLI-wide bounded retry path for concrete BootstrapLockContentionError instances
• remove duplicate daemon-client migration and foreground-only retry logic
• clarify safe operator recovery without recommending manual lock deletion
• cover transient recovery, exhaustion, and non-retryable lock states while retaining 100% coverage
• document the behavior and include a patch changeset

How to validate

1. Run npm run lint.
2. Run npm run typecheck.
3. Run npm run build.
4. Run npm run test:ci and confirm 100% lines, branches, functions, and statements for the complete collected scope.

Risk / rollout

• Risk: CLI startup can wait for at most 950 ms when a proven live process owns the authenticated bootstrap lock. Other lock failures remain immediate and fail closed.
• Rollout: ships as a patch release with no configuration or dependency changes. Revert this commit to restore the previous behavior.

Notes

• Changeset: .changeset/steady-bootstrap-retries.md
• Canonical MoM plan, implementation, adversarial, and second-pass reviews approved the patch before publication.

[qodo-code-review]

PR Summary by Qodo

Fix CLI root-bootstrap lock contention with bounded retries

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Retry verified live root-bootstrap lock contention across all non-help CLI invocations
• Remove duplicate migration during daemon-client creation to enforce one bootstrap boundary
• Improve user-facing contention diagnostics and document safe recovery behavior

Diagram

graph TD
  A["lcm invocation"] --> B["runCli()"] --> C["migrateLegacyHomeWithRetry()\n(20x, 50ms)"] --> D["runtime bootstrap\nlock check"] --> E{"Lock state"}
  E -->|"ok"| F["Command dispatch"] --> G["Daemon client (no 2nd migrate)"]
  E -->|"BootstrapLockContentionError"| C
  E -->|"ambiguous/tampered/reclaim"| H["handleCliError():\nprint safe message"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Share a single retry budget across multiple bootstrap boundaries

• ➕ Could preserve existing call sites while still bounding total wait time
• ➕ Allows finer-grained control if multiple subsystems had legitimate bootstrap needs
• ➖ Adds cross-cutting plumbing/state threading through command registration and daemon client creation
• ➖ Higher complexity than removing the redundant boundary; easier to regress or double-wait

2. Memoize successful migration for the process lifetime

• ➕ Minimal code churn; avoids repeat migration in later call sites
• ➕ Keeps daemon-client creation behavior unchanged to callers
• ➖ Introduces process-scoped mutable state and test-order coupling
• ➖ Less explicit than deleting the duplicate call; can break embedded/multi-runCli scenarios

3. Retry only for selected read-only commands (argv allowlist)

• ➕ Limits retries to lowest-risk commands
• ➕ May reduce perceived latency for mutating commands
• ➖ Duplicates command classification outside Commander and can silently miss new commands
• ➖ Bootstrap admission is a safety boundary independent of command mutability

Recommendation: The PR’s approach (one CLI-wide bounded retry at runCli(), plus removing the redundant daemon-client migration) is the best tradeoff: it fixes the transient failure for all commands, prevents double-wait amplification, and keeps security semantics centralized in runtime lock authentication (retry only for concrete BootstrapLockContentionError).

Files changed (9) +548 / -209

Bug fix (2) +39 / -82

lcm.tsGeneralize root-bootstrap retry and remove foreground-only/duplicate migration paths +35/-81

Generalize root-bootstrap retry and remove foreground-only/duplicate migration paths

• Replaces foreground-daemon-specific migration retry logic with a CLI-wide migrateLegacyHomeWithRetry() helper gated by BootstrapLockContentionError and fixed 20x/50ms timing. Removes the obsolete foreground argv classifier and the redundant migrateLegacyHomeIfNeeded() call from daemon-client creation, enforcing a single bootstrap boundary per invocation. Updates handleCliError() to print safe contention messages without dumping the full error object.

bin/lcm.ts

runtime-paths.tsStrengthen verified-live-owner contention message with safe recovery guidance +4/-1

Strengthen verified-live-owner contention message with safe recovery guidance

• Updates the BootstrapLockContentionError message thrown for authenticated live owner contention to include explicit guidance: verified live owner, no automatic recovery attempted, retry after competing operation, and do not delete the lock manually. Leaves lock acquisition/authentication/recovery mechanics unchanged.

src/runtime-paths.ts

Tests (3) +134 / -109

lcm-run-cli.test.tsExpand CLI tests for generalized retry policy and single bootstrap boundary +46/-92

Expand CLI tests for generalized retry policy and single bootstrap boundary

• Replaces foreground-only retry tests with coverage for ordinary commands (e.g., search) retrying transient BootstrapLockContentionError once the lock clears. Adds direct unit tests for migrateLegacyHomeWithRetry() bounds, attempt recording, sleep behavior, and non-retryable untyped failures. Extends handleCliError coverage to ensure contention errors render message-only output.

test/bin/lcm-run-cli.test.ts

coverage-cli-runtime-path-errors.test.tsAssert fail-closed lock error states are never treated as retryable contention +82/-15

Assert fail-closed lock error states are never treated as retryable contention

• Tightens runtime-path error tests to capture thrown values and assert they are not instances of BootstrapLockContentionError for ambiguous, malformed/tampered, and reclaim-race scenarios. Keeps assertions focused on message content while enforcing type-level non-retryability.

test/coverage-cli-runtime-path-errors.test.ts

runtime-paths.test.tsValidate live-owner contention uses BootstrapLockContentionError and safe message text +6/-2

Validate live-owner contention uses BootstrapLockContentionError and safe message text

• Strengthens the live-owner lock test to assert the exact BootstrapLockContentionError constructor when a trusted start witness exists, and validates the updated safe operator guidance strings. Preserves the ambiguous-owner fallback behavior when no trusted witness is available.

test/runtime-paths.test.ts

Documentation (3) +370 / -18

daemon-restart-recovery.mdDocument CLI-wide root-bootstrap contention behavior and safe operator recovery +27/-18

Document CLI-wide root-bootstrap contention behavior and safe operator recovery

• Rewrites the prior foreground-only daemon-start retry section to describe CLI-wide bounded retries for verified live-owner contention. Explicitly documents non-retryable fail-closed states and advises against manual bootstrap lock deletion.

docs/daemon-restart-recovery.md

2026-08-12-root-bootstrap-contention.mdAdd implementation plan for root-bootstrap contention retries +201/-0

Add implementation plan for root-bootstrap contention retries

• Adds an internal execution plan detailing constraints, TDD steps, security invariants, and verification commands for the change set. Serves as an auditable plan-of-record for the patch.

docs/superpowers/plans/2026-08-12-root-bootstrap-contention.md

2026-08-12-root-bootstrap-contention-design.mdAdd design spec explaining single-boundary retry strategy +142/-0

Add design spec explaining single-boundary retry strategy

• Adds a design document covering root cause, selected approach, rejected alternatives, and security guarantees. Specifies the concrete retry policy and user-facing exhaustion messaging.

docs/superpowers/specs/2026-08-12-root-bootstrap-contention-design.md

Other (1) +5 / -0

steady-bootstrap-retries.mdAdd patch changeset for CLI-wide bootstrap contention retries +5/-0

Add patch changeset for CLI-wide bootstrap contention retries

• Introduces a patch changeset documenting the new bounded retry behavior for authenticated root-bootstrap contention. Clarifies that unsafe/ambiguous lock states still fail closed.

.changeset/steady-bootstrap-retries.md

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR addresses transient root-bootstrap lock contention by adding a single bounded retry policy at the CLI entry boundary, so normal commands (e.g. lcm search) tolerate short overlaps with an authenticated live bootstrap while preserving fail-closed behavior for unsafe/ambiguous lock states.

Changes:

• Generalize root-bootstrap contention retry to all non-custom-help CLI invocations (20 attempts, 50ms delay; ~950ms max).
• Remove redundant daemon-client bootstrap migration so daemon-backed commands migrate only once per invocation.
• Update diagnostics, documentation, and tests to cover retry success/exhaustion and non-retryable lock states; add a patch changeset.

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
bin/lcm.ts	Adds CLI-wide bounded retry helper and removes redundant daemon-client migration; updates top-level error rendering.
src/runtime-paths.ts	Improves live-owner contention message to be actionable and explicitly fail-closed (no manual deletion guidance).
test/bin/lcm-run-cli.test.ts	Verifies single-boundary retry behavior, exhaustion limits, and safe error rendering.
test/runtime-paths.test.ts	Tightens assertions for verified-live-owner contention vs ambiguous-owner fallback.
test/coverage-cli-runtime-path-errors.test.ts	Ensures unsafe/ambiguous/tampered/reclaim-race states remain untyped (non-retryable) errors.
docs/daemon-restart-recovery.md	Documents CLI-wide bounded retry behavior and reiterates fail-closed recovery guidance.
docs/superpowers/specs/2026-08-12-root-bootstrap-contention-design.md	Adds the design rationale and security constraints for the retry policy.
docs/superpowers/plans/2026-08-12-root-bootstrap-contention.md	Adds the implementation plan and verification checklist for the change.
.changeset/steady-bootstrap-retries.md	Adds patch release note for the user-visible CLI behavior change.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.