fix(app): stabilize daemon lifecycle setup

[YUHAO-corn]

Fixes #2151.

Root cause: the daemon lifecycle setup effect depended on callbacks that were recreated when daemon health state changed. During startup/recovery, status, recovery, and attempt updates could therefore tear down and reinstall the lifecycle listener/timers, producing repeated setup/cleanup logs.

This patch keeps the listener/timer setup stable across ordinary daemon state updates by reading the latest lifecycle state through a ref. The setup effect can still restart when installation-level inputs change, such as the user id or auto-start setting.

Validation:

• pnpm --dir app exec vitest run --config test/vitest.config.ts src/hooks/tests/useDaemonLifecycle.test.ts
• pnpm --dir app compile
• pnpm --dir app exec eslint src/hooks/useDaemonLifecycle.ts src/hooks/tests/useDaemonLifecycle.test.ts
• pnpm --dir app exec prettier --check src/hooks/useDaemonLifecycle.ts src/hooks/tests/useDaemonLifecycle.test.ts
• git diff --check

Summary by CodeRabbit

• Tests

  • Added tests for daemon lifecycle covering auto-start, retry scheduling, and background/foreground pause-resume behavior to ensure stable lifecycle initialization and cleanup.

• Improvements

  • Improved daemon lifecycle logic to reduce stale-state issues, making startup, retry, and recovery behavior more reliable and ensuring visibility-change handling and cleanup occur exactly once.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb9445b3-c1af-4163-8edb-48bdbcbd158b

📥 Commits

Reviewing files that changed from the base of the PR and between 7cdee57 and 6a7494f.

📒 Files selected for processing (2)

• app/src/hooks/__tests__/useDaemonLifecycle.test.ts
• app/src/hooks/useDaemonLifecycle.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• app/src/hooks/useDaemonLifecycle.ts

---

📝 Walkthrough

Walkthrough

The hook centralizes lifecycle state into a ref (latestLifecycleRef), updates callbacks/effects to read from it to avoid stale closures, and adds tests validating retry scheduling and that lifecycle setup/cleanup runs only once across state updates.

Changes

Daemon lifecycle stability fix

Layer / File(s)	Summary
Ref-backed state foundation app/src/hooks/useDaemonLifecycle.ts	startDaemon is destructured from useDaemonHealth(userId). New latestLifecycleRef syncs isAutoStartEnabled, status, isRecovering, connectionAttempts, uid, and startDaemon so callbacks read current values.
Callback refactoring to read from ref app/src/hooks/useDaemonLifecycle.ts	attemptAutoStart, scheduleRetry, handleVisibilityChange, and the retry timeout callback now read required state from latestLifecycleRef.current and invoke the ref-provided startDaemon().
Simplified effect dependencies app/src/hooks/useDaemonLifecycle.ts	useEffect/useCallback dependency arrays are reduced to stable derived deps (calculateRetryDelay, attemptAutoStart, handleVisibilityChange) preventing spurious lifecycle re-initializations.
Lifecycle tests: retry & visibility stability app/src/hooks/__tests__/useDaemonLifecycle.test.ts	Adds tests for retry scheduling (ensuring startDaemon is called and connection attempts reset) and background/foreground pause-resume (verifying setup runs once across state updates and cleanup/listener removal occur on unmount).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A ref held tight, a closure freed—
No loops of setup, no stale seed.
State flows fresh, effects stay true,
The daemon rests (as daemons do)! 🌙

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: stabilizing daemon lifecycle setup by fixing a setup/cleanup loop.
Linked Issues check	✅ Passed	Changes directly address all coding requirements from #2151: eliminate setup/cleanup loop via ref-backed state management, stabilize lifecycle initialization, prevent teardown during state updates, and add test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the daemon lifecycle loop: modifications to useDaemonLifecycle.ts implement the ref-based fix, and test additions verify the stability requirements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

app/src/hooks/useDaemonLifecycle.ts (1)

96-96: 💤 Low value

Consider adding an eslint-disable comment for clarity.

The empty dependency array is intentional since the callback reads from latestLifecycleRef.current. Adding a brief comment would help future maintainers understand this is deliberate and not an oversight.

📝 Suggested documentation

-  }, []);
+  // Dependencies intentionally empty - reads current state from latestLifecycleRef
+  // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/hooks/useDaemonLifecycle.ts` at line 96, In useDaemonLifecycle, the
empty dependency array on the effect is intentional because the effect reads
from latestLifecycleRef.current; add a brief inline eslint-disable comment
(e.g., // eslint-disable-next-line react-hooks/exhaustive-deps — intentional:
uses latestLifecycleRef) or a short explanatory comment above the useEffect to
clarify why the dependency array is empty so future maintainers understand this
is deliberate in useDaemonLifecycle and not an oversight.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/src/hooks/useDaemonLifecycle.ts`:
- Line 96: In useDaemonLifecycle, the empty dependency array on the effect is
intentional because the effect reads from latestLifecycleRef.current; add a
brief inline eslint-disable comment (e.g., // eslint-disable-next-line
react-hooks/exhaustive-deps — intentional: uses latestLifecycleRef) or a short
explanatory comment above the useEffect to clarify why the dependency array is
empty so future maintainers understand this is deliberate in useDaemonLifecycle
and not an oversight.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4d8a6430-d5f6-4939-98dd-44c3f11248b5

📥 Commits

Reviewing files that changed from the base of the PR and between ce227c4 and 7cdee57.

📒 Files selected for processing (2)

• app/src/hooks/__tests__/useDaemonLifecycle.test.ts
• app/src/hooks/useDaemonLifecycle.ts

[graycyrus]

Review — graycyrus

Clean fix for the daemon lifecycle setup/cleanup loop (#2151). The ref-based pattern (latestLifecycleRef) is the standard React approach for stabilizing callbacks that depend on frequently-changing state — well applied here.

What changed

File	Summary
useDaemonLifecycle.ts	Destructure startDaemon directly; add latestLifecycleRef syncing all lifecycle state; callbacks read from ref instead of closures; dependency arrays reduced to stable values
useDaemonLifecycle.test.ts	Two new tests: retry scheduling + lifecycle stability (setup/cleanup runs exactly once across state transitions)

Verification

• Issue alignment: All acceptance criteria from #2151 addressed — loop eliminated, CPU stable, lifecycle idempotent, cleanup scoped to unmount, daemon behavior preserved, regression tests added, coverage gate passes.
• Effect dependency analysis: Main effect re-runs only on isAutoStartEnabled / uid changes (installation-level inputs). attemptAutoStart and handleVisibilityChange are stable refs. Correct.
• Race safety: scheduleRetry clears existing timeout before scheduling new one — no concurrent retry risk.
• CI: All checks green including coverage gate.

No findings beyond what CodeRabbit already noted. LGTM — moving to to-be-approved/.