With 1 replica, the old pod was torn down too quickly after the new pod
passed its first health check. Increase successThreshold to 3 (15s of
consecutive checks) and minReadySeconds to 30 to ensure Traefik has fully
propagated the new endpoint before the old pod is terminated.
Apply patterns from testingliveview.com — replace render_click/submit/change
event-name calls with element/form-targeted equivalents that validate the
actual DOM wiring, not just handler logic. Adds stable test selectors
(id="...") to interactive elements across 10 LiveView templates.
Surfaces (and fixes) several real issues that the bypass-style tests masked:
- Removes dead regenerate_token handler in agent_live/index.ex (no UI ever
triggered it) and its corresponding test.
- Removes dead refresh_topology test in network_map_live_test (event was
referenced only in the test — no handler, no button, no JS anywhere).
- Corrects "shows notification tab with add device modal" — modal lives on
the security tab; original test mounted ?tab=notifications and only
worked because direct event calls bypassed tab gating.
- Corrects form input shape mismatches in agent_live/index_test that the
handler's defensive case clauses had been masking.
- Expands setup data for org/settings_live_test (snmp_community,
default_agent_token_id, multi-org membership) so the gated buttons
actually render.
11 direct render_click/submit/change calls remain — all intentional,
documented server-side guard tests (tampered POSTs, race conditions,
defensive idempotent handlers).
Full LiveView test suite: 1300 tests, 0 failures.
The HelmRelease/HelmRepository CRs targeted flux-system; with flux uninstalled
they were never being applied. The gitlab-agent workload is not deployed on
the cluster.
ArgoCD refuses to clone repos containing tracked symlinks that point outside
the working tree. The .pre-commit-config.yaml symlink resolved to /nix/store/...
on the local dev machine and broke remote checkouts. Generate it locally via
direnv/nix instead.
Performance:
- schedule_live: preload page once instead of get_schedule!/1 per row
- alert_live + alerts: DB-side status filter; Repo.aggregate counts replace length/Enum.count over 500-row fetches on every event
- dashboard_live: drop duplicate get_device_status_counts; cap active alerts to 20 + use count_active_alerts/1
- maintenance.active_windows_for_device: 3 round-trips collapsed into one query (per-site-id branch keeps Postgres parameter types unambiguous)
- sites.build_site_tree: O(N^2) -> O(N) via group_by(parent_site_id)
- accounts.sole_owner_organizations: single group_by + having instead of per-org Repo.aggregate loop
- agents + agent_live (org + admin): count_assigned_devices_batch/1 + count_agent_polling_targets/1 (no preloads)
- alert_digest_worker: list_alerts_by_ids/1 batches digest fetch
- gaiia: distinct: true at DB; limit 50 on bidirectional ilike
Indexes:
- maintenance_windows(organization_id, starts_at, ends_at) WHERE suppress_alerts = true
- alerts(check_id) WHERE resolved_at IS NULL
Antipatterns:
- agents.delete_agent_token: PubSub.broadcast moved outside Repo.transaction so a rollback no longer leaves subscribers acting on a non-existent deletion
- integrations_controller.to_atom_keys: replaced String.to_existing_atom on user-controlled JSON keys with explicit allowlist
- 9 Task.start callsites converted to Task.Supervisor.start_child(Towerops.TaskSupervisor, ...) so background DB writes survive shutdown (test-mode discovery shims left as-is for sandbox semantics)
Captures a high-level review of module decomposition, context-layer
boundaries, query patterns, error handling, and test coverage gaps to
inform future refactor work.
Three deferred followups from the chunk-4 plan, all together:
A. Restrictions editor
- Resolver: time_of_week support (weekday set + time-of-day window),
in addition to the existing time_of_day.
- New OnCall.update_layer_restrictions/2 + clear_layer_restrictions/1.
- schedule_live/show: per-layer Restrict on-call shifts form
(Always on call / Time of day / Time of week with Mon-Sun checkboxes),
changes patched live and persisted via the resolver.
B. Drag-and-drop reorder
- New OnCall.reorder_layers/2 + reorder_escalation_rules/2 (atomic
transaction; validates id count + membership).
- assets/js/sortable_list.ts: minimal HTML5 drag-and-drop hook (no
external dep; mirrors the existing DeviceListReorder pattern),
registered as SortableList in app.ts.
- schedule_live/show + escalation_policy_live/show: layer/level cards
now carry data-id + draggable + a visible drag handle. Up/down arrow
buttons remain as a fallback for users on assistive tech.
C. Unified new-schedule UX
- schedule_live/show pre-opens the Add Layer form when the schedule
has zero layers, so the new-schedule -> add-layers flow lands ready
to act on. Two existing tests updated to seed a layer first; two
new tests verify the auto-open behaviour both ways.
Full suite 10,231 / 0 failures. Format/dialyzer clean.
ScheduleLive.Index
- Name search input above the date nav (phx-debounce 300ms).
- Standard <.pagination> footer (10 per page) under the gantt cards.
- Both reflected in URL params (?search= and ?page=) for shareable
links; defaults are stripped from the URL.
- Refactored: load_schedules_with_timeline/3 -> hydrate_schedules/3
so filter/paginate/hydrate are clearly separate steps.
Tests
- 2 new ExUnit integration tests (search filter narrows results +
search input renders).
- e2e/tests/schedules.spec.ts: switched from table-row selectors to
card-link selectors (schedules tab is now gantt cards, not a table),
added smoke tests for date nav controls + search input + REPEAT
footer + Services sidebar, renamed "Add Rule" -> "Add Level".
Full suite 10,216 / 0 failures. Format/dialyzer clean.
OnCall context
- move_layer/2: up/down position swap for schedule layers, mirror of
move_escalation_rule/2 (tested for both directions + boundary no-op).
ScheduleLive.Show
- New move_layer event handler.
- Up/down arrow buttons on each layer card; the top arrow is disabled on
the first layer, the bottom on the last (matches the escalation policy
level reorder UX shipped in chunk 1).
- Replaced two per-hour walks (build_layer_spans + build_final_spans -
~672 resolve calls per render at 14 days) with Resolver.resolve_range/3
+ a small segments_to_spans/3 mapper. Per-layer view wraps each layer
in a temporary single-layer %Schedule{} so the resolver code path is
shared and overrides don't bleed into the per-layer strip.
Tests: 3 new context tests for move_layer, 1 new integration test for
the layer reorder UI. Full suite 10,214 / 0 failures. Format/dialyzer
clean.
Resolver
- Resolver.resolve_range/3: returns the contiguous on-call segments for a
schedule across [start_at, end_at). Walks layer handoffs + override
boundaries, drops gap segments, and merges adjacent same-user segments.
UserColor module
- New Towerops.OnCall.UserColor with deterministic id -> color mapping
(Tailwind class + matching #RRGGBB hex). Refactored ScheduleLive.Show
to use it instead of its own per-page index palette so the same person
gets the same color across the schedules list and detail pages.
ScheduleLive.Index
- Date navigation: Today / prev / next + 1/2/4-week range selector.
- URL-driven: ?start=YYYY-MM-DD&range=N for shareable views; falls back
to defaults on malformed values.
- Per-row card: schedule name link, on-call-now avatar swatch, day-of-week
header strip, gantt strip rendered as a CSS grid with one column per day,
Today marker overlay.
Tests: 5 new resolver tests, 9 new UserColor tests, 5 new ScheduleLive
integration tests. Full suite: 10,210 / 0 failures. Format/dialyzer clean.
Also: cleaned up two stale inline comments in k8s/deployment.yaml that
disagreed with the values they sat next to (replicas/maxSurge).
- k8s/deployment.yaml: bump startup-probe failureThreshold from 12 to 24
(70s -> 130s max startup time) to give the app more headroom on cold start.
- lib/towerops/profiles/yaml_profiles.ex: defer YAML profile loading from
init/1 to a handle_info(:load_profiles, ...) so the supervisor (and Bandit)
come up immediately and the profiles populate in the background within ~60s.
- e2e/Makefile: convenience target wrappers around the existing npm scripts.
Chunk 1 of the on-call/escalation redesign plan
(docs/plans/2026-04-28-on-call-pagerduty-style-redesign.md):
Schema
- on_call.ex: list_devices_using_policy/2 (direct + org-default inheritance)
- on_call.ex: move_escalation_rule/2 for up/down level reordering
- list_escalation_policies/1 now preloads :rules
Show page
- Levels rendered as numbered cards with up/down/delete controls
- "Notify the following users immediately and escalate after N minutes."
- REPEAT footer surfaces repeat_count inline (no longer a standalone column)
- Right-side Services sidebar lists devices using the policy
- Header line shows the handoff notifications mode (when in use by a service /
always / never)
Edit form
- Removed standalone Repeat Count input
- Added handoff notifications mode select
- Added inline "If no one acknowledges, repeat this policy N time(s)" block
Index views (policies tab + standalone)
- Replaced Repeat Count column with a Levels (rule count) column
Other
- rate_limit.ex: bind unmatched :ets.new/2 + schedule_clean/1 returns to fix
dialyzer :unmatched_returns warning under the project's strict flags.
All 10,191 tests pass; mix format clean; mix compile --warnings-as-errors clean;
mix dialyzer passes.
* UISP diff_snapshots test: backdate first snapshot via UPDATE instead of
Process.sleep(1_100) to get distinct second-precision snapshot_at values.
* PagerDuty.Client.send_event_with_retry/2: check max_retries BEFORE sleeping
on 429. Previously a 429 response always slept once (~1s) before checking
the retry cap, so even with @max_retries = 0 in test env every 429 test
paid that cost.
Suite time: 65.3s → 33.9s (-48%). No test is >400ms now.
Req's default `retry: :safe_transient` waits ~7s across exponential backoff
when the stubbed response is a 5xx or connection error. Tests that asserted
error-status behavior paid that price on every run.
Disable retries in test env for every direct Req caller:
* `Towerops.HTTP` (covers weather/netbox/geocoding/http_executor/pagerduty)
* `CnMaestro.Client`, `Gaiia.Client`, `Preseem.Client`, `Sonar.Client`,
`Splynx.Client`, `Agents.ReleaseChecker`, `Billing.StripeClient`
Use `Keyword.put_new(:retry, false)` so any test explicitly opting into
retry behavior is respected.
Suite time: 82.8s → 65.3s. Slowest test dropped from 7050ms to 1447ms.
Promoted pure presentation and utility helpers from `defp` to `def @doc false`
across ~20 LiveViews, Oban workers, and sync modules so they're reachable from
unit tests. Refactored several `cond` blocks into idiomatic function heads with
guards. Added ~250 new test cases in new files under test/towerops and
test/towerops_web, including DB-backed tests for CnMaestro.Sync and
AlertNotificationWorker, and removed dead LiveView tab components and
CapacityLive (no callers anywhere in lib/test).
Configured mix.exs test_coverage.ignore_modules to exclude vendored third-party
code (SnmpKit, protobuf-generated Towerops.Agent.*, Absinthe GraphQL types,
Phoenix HTML modules, Inspect protocol impls) from coverage calculations —
these are not our project code.
Coverage: 66.93% → 70.09%. Full suite: 10,127 tests, 0 failures.
The remaining @dialyzer {:no_opaque, ...} / {:nowarn_function, ...}
directives in accounts.ex, devices.ex, organizations.ex all masked the
same upstream issue: Ecto.Multi.new/0 produces a struct containing a
literal %MapSet{map: %{}} whose concrete shape violates MapSet's
@opaque t at every subsequent Ecto.Multi.* call boundary.
Tried first: public helper function with @spec Ecto.Multi.t() — confirmed
dialyzer uses success typing of the body, not the spec. Doesn't help.
Real fix: drop Ecto.Multi entirely for these transactions. Each was
performing a small, sequential set of Repo operations that map cleanly
to a plain `Repo.transaction(fn -> ... end)` block with Repo.rollback
on errors. No Multi machinery is actually needed:
- accounts.ex:
- register_user/1: insert + 0-2 consent grants
- register_user_with_organization/1: insert + create_org + consents
- confirm_user_transaction/1: update + delete_all tokens
- organizations.ex:
- do_create_organization/4: lock user → limit check → insert org + membership
- set_default_organization/2: two update_alls
- accept_invitation/2: update invitation + insert membership
- devices.ex:
- create_device/2: maybe lock org → maybe quota check → insert
Caught a real consent-failure rollback bug along the way: initial
refactor discarded the return of grant_consents_sequential/2, which
meant FK violations on consent insert would abort the transaction at
the Postgres level but the caller still saw {:ok, user} — later
operations in the transaction silently failed with 25P02. Fixed by
propagating the {:error, changeset} up so Repo.rollback is called.
Verified by the AccountsDefensiveCoverageTest regression tests.
Behavior is preserved — same transactional guarantees, same success
/ error tuple shapes. Net -86 LOC and no suppressions.
mix dialyzer: `done (passed successfully)` with zero @dialyzer
directives in lib/.
Changes to eliminate @dialyzer suppressions by fixing underlying causes:
NIF stubs (towerops_native.ex, mib_translator.ex):
- Change stubs to :erlang.nif_error(:nif_not_loaded) (no_return type).
Real NIF replaces stubs at load time; calls to unloaded stubs now fail
loudly instead of returning fake data. Lets dialyzer trust @spec.
- Remove @dialyzer :nowarn_function on three NIFs and on translate/1.
Discovery sync_* functions (snmp/discovery.ex, channels/agent_channel.ex):
- agent_channel passes %{device_id: _, interfaces: _} and %{id: _} maps
into Discovery.sync_ip_addresses/sync_processors/sync_storage, which
@spec'd only %Device{}. Add narrow map-type unions (ip_sync_device,
snmp_device_ref) reflecting what the functions actually access.
- Remove @dialyzer :nowarn_function on three agent_channel helpers.
remote_ip.ex — real bug caught and fixed:
- `:ranch.get_addr(socket.transport_pid)` was always raising since
Bandit uses ThousandIsland, not Ranch; the rescue _ -> nil silently
returned nil every time. Switched to Phoenix's documented
:peer_data connect_info (already enabled in endpoint.ex) via
socket.assigns; remote IP now actually works.
- Remove remote_ip.ex entry from .dialyzer_ignore.exs.
Accounts / Organizations (Ecto.Multi opacity):
- Add @specs to Multi-building helpers, refactor into pipe chains.
- 6 @dialyzer :nowarn_function → 0, but 7 :no_opaque remain. Root
cause is upstream: Ecto.Multi.new/0 returns a struct with a literal
%MapSet{} whose @opaque internal representation trips dialyzer on
every subsequent Multi.* call. Unfixable without an Ecto patch or
bypassing Multi entirely. Comments document the specific upstream
issue rather than a vague "Ecto.Multi opacity" claim.
Devices.ex:
- Adding @specs made it worse (call_without_opaque → contract_with_
opaque); inlining the Multi didn't help either — same MapSet root
cause. Suppression kept with a sharper comment.
Categories addressed:
- pattern_match / pattern_match_cov (32): remove dead case/with clauses
that dialyzer proved unreachable from the caller types.
- contract_supertype / extra_range / invalid_contract /
contract_with_opaque (25): narrow @spec declarations to match actual
success typings.
- call / call_without_opaque (18): fix bad calls, narrow User.t to
allow nil for in-memory changeset structs, suppress Ecto.Multi
opaque-type false positives with targeted @dialyzer directives.
- guard_fail / no_return / unused_fun / unknown_function (13): remove
dead || fallbacks, simplify always-true params, cascade-resolve
no_returns via the underlying pattern_match and call fixes.
Real production bug fixed: StormDetector.handle_cast/2 had swapped
`:queue.in` args (`queue |> :queue.in(ts)` which desugars to
`:queue.in(queue, ts)` — wrong argument order). Alert timestamps
were never being enqueued, so storm detection would fail at runtime.
Corrected to `ts |> :queue.in(queue)`.
.dialyzer_ignore.exs: suppress two genuine dep-PLT gaps
(:ranch.get_addr/1 false positive from Bandit's transitive ranch,
and the Cloak.Vault GenServer callback_info on the CI build path).
`mix dialyzer` now: Total errors: 114, Skipped: 114 — passes clean.
Warnings: 88 → 0.
Prefix fire-and-forget calls (PubSub.broadcast, Oban enqueue, Logger,
update_*, etc.) with `_ =` so dialyzer knows the return value is
intentionally discarded. Wrap a few if/case expressions whose
branches produce mixed types the same way.
No behavior changes — only explicit acknowledgement of discarded
returns.
Warnings: 242 → 88.
- Add @type t :: %__MODULE__{} to schemas referenced by callers:
ApiToken, Monitoring.Check, OnCall.Schedule, SiteOutage,
NotificationDigest, Snmp.WirelessClient, Topology.DeviceLink,
Agent.AgentJob, Agent.MikrotikResult.
- Proto.Types: qualify forward references to nested modules with
Types. prefix, and fix Metric typo (should be union type metric()).
- data_loaders.ex / chart_builders.ex: Snmp.SNMPDevice.t was a ghost
spec — module is Snmp.Device.
Warnings: 328 → 242.
Ignore file was silencing every warning under lib/towerops/**. Root
cause was plt_add_deps: :apps_direct missing Plug/Phoenix/Oban/Decimal
/Redix/ssl/public_key — hundreds of false `unknown_function` warnings
were being papered over.
Expand plt_add_apps to cover the common deps, narrow the ignore file
to real dep-PLT gaps only, and fix two concrete bugs uncovered by the
change:
- ScopedResource.fetch_preload/4: spec was atom() | [atom()] but
callers pass keyword lists (e.g. [rules: :targets]).
- ToweropsNative: tag NIF stubs with @dialyzer :nowarn_function so
dialyzer trusts the @spec instead of the fallback body.
Remaining 328 real warnings surface for follow-up.
Intercepts requests with Accept: text/markdown at endpoint level (before
the router's :accepts plug rejects them) and returns hardcoded markdown
representations for /, /privacy, and /terms.
