Two critical production bugs fixed:
1. **Alert type enum → string conversion**
- Changed Alert.alert_type from Ecto.Enum to :string for flexibility
- Updated all queries to use "device_down"/"device_up" strings instead of atoms
- Fixed pattern matching in alerts.ex and device_monitor_worker.ex
- Updated 44+ test files to use string literals
2. **SQL array indexing syntax error in activity feed**
- Fixed PostgreSQL syntax: `array_agg(...)[1]` → `(array_agg(...))[1]`
- Prevents "syntax error at or near [" in activity feed queries
3. **Added comprehensive timer cleanup tests**
- Tests for mobile_qr_live.ex timer cleanup on terminate
- Tests for agent_live index timer cleanup
- Verifies memory leak fixes from previous commits
Files changed:
- lib/towerops/alerts.ex
- lib/towerops/alerts/alert.ex
- lib/towerops/activity_feed.ex
- lib/towerops/workers/device_monitor_worker.ex
- test/**/*_test.exs (44+ files with alert_type references)
- test/towerops_web/live/mobile_qr_live_test.exs
- test/towerops_web/live/agent_live_test.exs
- test/towerops/workers/device_poller_worker_test.exs
All tests passing except 1 unrelated brute force protection test.
When the Oban queue backs up, the 60-second uniqueness window expires
and duplicate jobs stack up per device. Switch to period: :infinity so
only one poll/monitor job exists per device at any time. Add replace
option to supersede stale scheduled jobs with updated scheduled_at.
Remove :executing from unique states so self-scheduling works while
the current job runs. Set max_attempts: 1 since retrying stale polls
is pointless.
Also fix all credo --strict issues across the codebase.
device_up alerts are informational recovery events, not ongoing issues.
Previously they were created without resolved_at, inflating the
unresolved alert count. Now they're born resolved.
Includes a data migration to fix existing stale device_up alerts.
- Escape HEEx template braces in GraphQL/API docs with raw(~S[...])
- Fix test assertions for updated marketing copy and UI text
- Extract helper functions in GraphQL resolvers to reduce nesting depth
- Create shared ErrorHelpers module for API controllers
- Fix ETS race condition in brute force whitelist cache for async tests
- Fix property test generators to use ASCII instead of printable unicode
- Add alert_severity helper to site_live/show
- Update accounts fixtures for explicit user confirmation
- create MonitoringCheck schema for the monitoring_checks table, fixing
silent data loss where ping results were dropped by the wrong schema
- update agent channel and device monitor worker to use new schema
- fix Stats queries to use MonitoringCheck instead of Check schema
- add list_online_cloud_pollers and list_cloud_polled_devices queries
- add CloudLatencyProbeWorker to dispatch cross-agent latency probes
every 8 hours via PubSub to all online cloud pollers
- scope AgentLatencyEvaluator to cloud pollers only with cloud_only
filter, lower min_checks (5), wider time window (48h), and
device-level assignments to avoid changing site/org defaults
- update cron schedule: probes at 0/8/16h, evaluation at 0:30/8:30/16:30
- refactor monitoring.ex and http_executor.ex to fix credo complexity
Always broadcast from update_device_status/2 so the device index
LiveView refreshes even when status hasn't changed. Add a 15-second
tick timer to keep "Last Checked" timestamps fresh between polls.
Scope alert PubSub topics to organization for defense-in-depth.
This commit addresses Phase 1 (CRITICAL - Data Loss Prevention) issues
from the comprehensive bugs and inconsistencies analysis.
CRITICAL FIXES:
1. Protobuf Schema Inconsistencies
- Added missing job_id field (field 5) to SnmpResult message
- Fixed AgentError message structure (job_id, message, timestamp)
- Removed obsolete transport field from SnmpDevice struct
- Regenerated protobuf code with protoc-gen-elixir
- Prevents crashes when agent sends SNMP results or errors
2. Interface/Sensor Discovery Timeout Data Loss
- Changed timeout behavior to return error instead of empty list
- Prevents deletion of ALL existing interfaces/sensors on slow networks
- Discovery fails gracefully instead of destroying historical data
- Addresses commit 7a57f7c sensor discovery pipeline data loss issue
3. Polling Offset Schedule Mismatch
- Fixed schedule_next_poll() to use offset only (not interval + offset)
- Maintains consistent polling intervals across all executions
- Prevents load bunching that offset was designed to prevent
- Example: 300s interval with offset=94s now consistently polls every 5m
4. Always Reschedule Race Condition
- Added should_continue_polling?() and should_continue_monitoring?()
- Only reschedule if device is still enabled and should be polled by Phoenix
- Prevents zombie jobs from continuing to poll disabled/deleted/reassigned devices
- Stops race condition where stop_polling() is bypassed by in-flight jobs
Files Changed:
- priv/proto/agent.proto
- lib/towerops/proto/agent.pb.ex (regenerated)
- lib/towerops_web/channels/agent_channel.ex
- lib/towerops/snmp/discovery.ex
- lib/towerops/workers/device_poller_worker.ex
- lib/towerops/workers/device_monitor_worker.ex
- test/towerops/workers/device_poller_worker_test.exs
Test Results: All passing
- device_poller_worker_test.exs: 8 tests, 0 failures
- device_monitor_worker_test.exs: 3 tests, 0 failures
- discovery_test.exs: 13 tests, 0 failures
Remaining Phases:
- Phase 2 (HIGH): 8 data integrity issues
- Phase 3 (MEDIUM): 10 reliability issues
- Phase 4 (LOW): 3 cleanup issues
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Adds @spec annotations to all Oban worker perform/1 functions to improve
static analysis and documentation. Return types are specific to each worker's
actual behavior:
Workers returning 🆗
- BackupSummaryWorker
- BackupTimeoutWorker
- DeviceMonitorWorker
- DevicePollerWorker
- JobHealthCheckWorker
- NeighborCleanupWorker
Workers with complex returns:
- DiscoveryWorker: :ok | :discard
- FirmwareVersionFetcherWorker: :ok | {:error, term()}
- LoginHistoryCleanupWorker: {:ok, %{deleted: non_neg_integer(), anonymized_deleted: non_neg_integer()}}
- MikrotikBackupWorker: :ok | {:error, String.t()}
- SessionCleanupWorker: {:ok, %{sessions_deleted: non_neg_integer()}}
Analysis found that none of these workers have complex job parameters that
would benefit from embedded schemas - all use either no parameters or simple
device_id strings passed in job args.
StaleAgentWorker already had a typespec on find_stale_agents/0 from previous work.
All 98 worker tests passing after changes.
Fixed dialyzer warnings across multiple files by properly handling return
values from functions that are called for side effects.
Changes:
- lib/towerops/agents.ex: Explicitly match broadcast and if-expression returns
- lib/towerops/devices.ex: Match worker start function returns in if-expressions
- lib/towerops/workers/agent_latency_evaluator.ex: Match broadcast return
- lib/towerops/workers/device_monitor_worker.ex: Match perform_check and handle_status_change returns, handle schedule_next_check errors
- lib/towerops/workers/device_poller_worker.ex: Match all PubSub broadcast returns, handle schedule_next_poll errors, remove unreachable pattern
- lib/towerops/workers/stale_agent_worker.ex: Match process_stale_agents and broadcast returns
- lib/towerops_web/channels/agent_channel.ex: Match PubSub.subscribe return, improve get_addr pattern matching, remove unreachable format_ip clause
- lib/towerops_web/live/device_live/form.ex: Match enqueue_discovery return, replace compile-time @dev_env with runtime check
Pattern applied:
- PubSub broadcasts: `_ = Phoenix.PubSub.broadcast(...)`
- If-expressions with side effects: `_ = if condition do ... end`
- Critical operations: Added error logging with case statements
All 3625 tests passing. No dialyzer warnings remaining in lib/towerops or lib/towerops_web.
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
