[devswha]

A follow-up to PR #1768, wave 5 of the App.tsx split. The plan is .omo/plans/app-stores-split.md in the maintainer's workspace.

The plan's rule R1 says every contiguous block moved out of src/App.tsx becomes its own hook call, placed at that block's original line, so effect and listener order stay exactly as they were. PR #1768 broke that rule:

• What it did: it gathered six separate blocks into one useAlerts call at the notifications line.
• What changed as a result: three effects ran earlier in a commit than before:
  • the alert-sound player effect;
  • the sound-unlock effect;
  • the push re-registration effect, which also moved ahead of the event-stream effect.
• Harm found: none. PR refactor(app): move the alerts and notification state into a hook #1768's body and the merge pass's review found no behaviour change from this.

This PR restores the exact order anyway, so no such argument is needed.

What changes

src/lib/useAlerts.ts now exports one hook per block. Each is called in App.tsx at its block's original line (the line numbers are from before PR #1768):

Block	Hook	Called where the block stood
alert prefs (alerts, alertsRef, alertsOn, alertsOnRef)	useAlertPrefs({ settings })	right after useSettings()
notifications, pushOn, pushOnRef	useNotificationState()	after the connection chip
status and turn refs, in-app and sound switches	useAlertRefs({ settings })	after lockedRef, before followFocusRef
dropIn, the sound player effect, chime, the sound-unlock effect, plus the three event-stream callbacks	useAlertSignals({ … })	before the event-stream effect
enableNotifications, disableNotifications, the push re-registration effect	useNotificationSwitch({ … })	after the event-stream effect and handleServerMessage
the bell, bellVisible, alertsOffDot (no hooks)	plain alertsBell({ … })	where the bell was computed

• Moved lines: none changes. Each hook's body is its block's lines from PR refactor(app): move the alerts and notification state into a hook #1768.
• Effects and callbacks: every effect is back at its original line. The three event-stream callbacks (onRoster, onPaneStatus, onPaneExited) are useCallback([]) hooks without effects, so where they sit inside useAlertSignals changes no order.
• Hook ceiling: unchanged at 144, because the same hook calls left App.

Checks

• --color-moved: git diff --color-moved=plain --color-moved-ws=allow-indentation-change origin/main...HEAD shows only:

  • the single call replaced by six calls;
  • the import line;
  • useAlerts split into hook signatures, params interfaces, doc comments and returns.

  The block bodies show as moved, or as unchanged in place.

• Moved blocks are exact: all six blocks match App.tsx before PR refactor(app): move the alerts and notification state into a hook #1768 (53b85127e) byte for byte, at lines 225-232, 496-500, 509-518, 604-647, 744-787 and 1317-1343.

• Positions: the line before each new call is the line that came before its block on 53b85127e.

• Typecheck: bun run typecheck passes.

• Unit tests: 129 pass, 0 fail across 9 files, from:

  HERDR_TEST_MODE=unit bun test src/app-hooks.test.ts src/conventions.test.ts src/lib/notifications.test.ts src/lib/droplet.test.ts src/lib/alertTurns.test.ts src/lib/alertSound.test.ts src/lib/push.test.ts src/lib/notificationTarget.test.ts src/lib/i18n.test.ts

• bun run check fast: passes (unit 6253 pass, 0 fail, 13 skip in 443 files, 8 at a time, 68.4s)

• Alert browser scripts: each run with bun run check run --build bun scripts/<name>.ts: alert-sound-regression, droplet-regression, droplet-demo-regression, notification-startup-regression, notification-view-regression and push-settings-regression each exit 0.

No changelog fragment: users see no change.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: refactor/app-alerts-hook-positions
• File: .coderabbit.yaml
• Commit: 8758731
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4305832f-b1c2-4066-8577-2c4f5879089d

📥 Commits

Reviewing files that changed from the base of the PR and between d64e9ed and 8758731.

📒 Files selected for processing: 2

• src/App.tsx
• src/lib/useAlerts.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Summary

Summary by CodeRabbit

• Refactor
  • Alert preferences, notification status, alert activity, notification controls, and bell presentation are now handled separately. Existing notification enablement, disablement, and bell status behavior are preserved.
  • No changes to user-facing features or behavior are indicated in this update.

The combined useAlerts hook is replaced by separate hooks for alert preferences, notification state, alert references, alert signals, notification controls, and bell presentation. App wires these helpers into alert handling and header rendering.

Changes: Alert hook refactor

Layer / File(s)	Summary
Alert state and references src/lib/useAlerts.ts, src/App.tsx	Alert preferences, notification state, and pane-level refs are exposed through separate hooks. App uses these helpers.
Alert signal handling src/lib/useAlerts.ts, src/App.tsx	useAlertSignals receives alert preferences and refs, and returns event and pane handlers. App wires the helper to alert state and layout refs.
Notification controls and bell src/lib/useAlerts.ts, src/App.tsx	Notification enable and disable actions are separated from bell presentation state. App uses the actions and derives bell state through alertsBell.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Refactor

Suggested reviewers: kratam

Merge Risk: ⚪ Minimal · up to 87587

No actionable alert behavior regression remains identified; the change is ready for normal merge checks.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly describes the main change: splitting the alerts hook and calling each hook where its block originally stood.
Description check		The description explains the change and resulting behavior, then lists validation commands and results. It covers the template’s required Change and Validation information.
Docstring Coverage		Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

[devswha]

Native Windows install failed in scripts/windows-plugin.test.ts, in the test "starts after its launcher exits, stops its owned tree and restarts on Windows": could not confirm Ovis stopped; its pid file was kept.

That test covers the Windows plugin launcher's process lifecycle on a GitHub-hosted Windows runner. This PR changes only src/App.tsx and src/lib/useAlerts.ts, which are browser client code that this job does not run. Main's CI, including this job, passed on d64e9ed7f, the base of this PR's merge commit.

I'm rerunning only the failed job once the run finishes.

[devswha]

Merge pass: follow-up to #1768. A gate reviewer on a different model than the author's approved it.

• Order restored. The reviewer substituted each hook's body, and alertsBell's, back into App and compared with App.tsx at 5e44913 (before refactor(app): move the alerts and notification state into a hook #1768). The component body matches.
  • All 34 effects are back at their original positions.
  • The only addition is the three [] event-stream callbacks inside useAlertSignals. They have no effects.
• No other changes found.
  • alertsBell calls no hooks.
  • No hook argument is used before it is declared (no TDZ).
  • Counted on the TypeScript AST, App still has 144 hook calls.
• Tests at this head. 76 unit tests pass (app-hooks, conventions, droplet, notifications, alertTurns, alertSound), and the typecheck passes.

Native Windows install first failed on the known flaky stop/restart test (windows-plugin.test.ts:180, "could not confirm Ovis stopped"), which this PR doesn't touch. It passed on the re-run.

All required checks are green and no review thread is open.