Repository navigation
feat(daemon): fix #112 by integrating alert delivery into the daemon loop - #279
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds daemon-loop tests in ChangesAlert Dispatch Tests
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@Gezziy Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/daemon/loop.test.ts`:
- Around line 539-565: Add logging assertions to the daemon loop tests so the
delivery logging contract is covered. In the `startDaemon` scenarios around
`mockDeliverPendingAlerts`, assert that `logger().info(...)` is called with the
delivery summary after a successful dispatch and that `logger().error(...)` is
called when `mockDeliverPendingAlerts` rejects in the unexpected-failure path.
Use the existing `startDaemon`, `mockDeliverPendingAlerts`, and `logger()`
symbols to locate the relevant expectations in this suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94302136-d492-4888-a540-3c3113259369
📒 Files selected for processing (1)
tests/daemon/loop.test.ts
📜 Review details
🔇 Additional comments (1)
tests/daemon/loop.test.ts (1)
557-557: 🎯 Functional Correctness
resolves.not.toThrow()is fine here.startDaemon(...)returns a promise, so this assertion is a valid way to check the async path completes without error; no change needed.> Likely an incorrect or invalid review comment.
@Gezziy please resolve this |
|
done , merge boss |
3 participants
closes #112
##summary:
This PR closes issue #112 by wiring alert delivery into the daemon’s polling cycle. Before this change, a monitoring pass could finish without immediately dispatching any alerts that had been fired during that pass. The daemon now runs the delivery step right after
runMonitorCycle, which keeps alert propagation inside the same execution flow instead of deferring it to a later interval.The updated flow is straightforward: the daemon completes its monitoring work, invokes
deliverPendingAlerts(db, network), logs the delivery summary, and then continues with the rest of the maintenance steps. That matters because alert delivery is part of the daemon’s operational contract, and it needs to happen as soon as the polling cycle has produced actionable alerts. The implementation also keeps the same database handle flowing through the loop and the dispatcher so delivered alerts are marked in the shared database state.The test coverage verifies that daemon cycles trigger alert dispatch and that the loop stays resilient if delivery throws unexpectedly. That gives us confidence that issue #112 is addressed without changing the daemon’s failure behavior.
Changes Made
src/daemon/loop.tsdeliverPendingAlerts(db, network)immediately after eachrunMonitorCyclecompletion.tests/daemon/loop.test.ts#112Testing
Executed:
npm test -- tests/daemon/loop.test.tsResult: