[fix](alarm): make dismissal stick and count the devices
Four defects, three of them the same missing piece of the delivery ledger.
- listSubscriptions scanned nine destinations for ten columns and read a NULL
subscriber_user_id into a uuid. Every alert with an anonymous subscriber
answered 500 ("number of field descriptions must equal number of
destinations, got 10 and 9"), and the client renders a failed read as an
empty list — so 'Subscribed devices' said 0 while the row existed. The
listing now uses the same scan as every other subscription read.
- alarmStatus reported the alert's commanded state, not the caller's own. An
alert stays 'triggered' until the owner disarms or every subscriber acks, so
a device that had already dismissed the alarm was told active:true on every
poll and re-raised its takeover. active is now true only while the caller
still owes the newest trigger; the owner keeps seeing their own alert.
- ackAlarm required an eventId, which the native takeover launch does not
have. A dismissal that cannot name its event now clears the subscription's
whole outstanding set (AckAllDeliveries / AckAllForSubscriber), so the debt
cannot survive because the client could not echo a uuid.
- The device ledger probe (gated on DATABASE_URL, like the lifecycle probe)
pins the per-subscriber debt, the ack-all semantics and the cascade that
takes a device token with the destroyed alert.
Verified against the dev database and through Maestro: 2 subscriptions listed,
device A acked -> its status turns active:false while device B still gets the
409 check-in, ack-all without eventId clears the debt and drains the alert.
This commit is contained in:
+35
-14
@@ -63,15 +63,19 @@ func AckAlarm(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
validationError(w, "%v", err)
|
||||
return
|
||||
}
|
||||
eventRaw := fieldOrQuery(r, payload, "eventId")
|
||||
if eventRaw == "" {
|
||||
validationError(w, "eventId is required")
|
||||
return
|
||||
}
|
||||
eventID, err := uuid.Parse(eventRaw)
|
||||
if err != nil {
|
||||
validationError(w, "eventId is not a valid uuid")
|
||||
return
|
||||
// eventId is optional on purpose. The native takeover launch carries only the
|
||||
// alert, so a dismissal from that path has no event to name; "I have heard
|
||||
// this device's alarm" is then answered by clearing the caller's whole
|
||||
// outstanding set (see db.AckAllDeliveries). A stale debt that nobody can
|
||||
// name must not replay forever because the client could not echo a uuid.
|
||||
var eventID uuid.UUID
|
||||
if eventRaw := fieldOrQuery(r, payload, "eventId"); eventRaw != "" {
|
||||
parsed, err := uuid.Parse(eventRaw)
|
||||
if err != nil {
|
||||
validationError(w, "eventId is not a valid uuid")
|
||||
return
|
||||
}
|
||||
eventID = parsed
|
||||
}
|
||||
|
||||
alert, err := db.GetAlert(r.Context(), alertID)
|
||||
@@ -96,12 +100,21 @@ func AckAlarm(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
}
|
||||
}
|
||||
|
||||
resp := ackAlarmResponse{AlertID: alertID.String(), EventID: eventID.String()}
|
||||
resp := ackAlarmResponse{AlertID: alertID.String()}
|
||||
if eventID != uuid.Nil {
|
||||
resp.EventID = eventID.String()
|
||||
}
|
||||
|
||||
// A device token lives on the subscription, so the ledger row is addressed
|
||||
// directly: no user id, no device-id echo, nothing to spoof.
|
||||
if c.IsDevice {
|
||||
cleared, err := db.AckDelivery(r.Context(), alertID, c.Sub.ID, eventID)
|
||||
var cleared int64
|
||||
var err error
|
||||
if eventID == uuid.Nil {
|
||||
cleared, err = db.AckAllDeliveries(r.Context(), alertID, c.Sub.ID)
|
||||
} else {
|
||||
cleared, err = db.AckDelivery(r.Context(), alertID, c.Sub.ID, eventID)
|
||||
}
|
||||
if err != nil {
|
||||
dbError(w, action, err)
|
||||
return
|
||||
@@ -119,7 +132,8 @@ func AckAlarm(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
}
|
||||
resp.Destroyed = destroyed
|
||||
}
|
||||
Debug(action, "POST", "alert=%s event=%s acked=%d destroyed=%t (device)", alertID, eventID, resp.Acked, resp.Destroyed)
|
||||
Debug(action, "POST", "alert=%s event=%s all=%t acked=%d destroyed=%t (device)",
|
||||
alertID, resp.EventID, eventID == uuid.Nil, resp.Acked, resp.Destroyed)
|
||||
writeJSON(w, http.StatusOK, resp)
|
||||
return
|
||||
}
|
||||
@@ -132,7 +146,13 @@ func AckAlarm(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
validationError(w, "%v", err)
|
||||
return
|
||||
}
|
||||
cleared, err := db.AckAlarmDelivery(r.Context(), alertID, c.UserID, deviceid.Hash(raw), eventID)
|
||||
deviceHash := deviceid.Hash(raw)
|
||||
var cleared int64
|
||||
if eventID == uuid.Nil {
|
||||
cleared, err = db.AckAllForSubscriber(r.Context(), alertID, c.UserID, deviceHash)
|
||||
} else {
|
||||
cleared, err = db.AckAlarmDelivery(r.Context(), alertID, c.UserID, deviceHash, eventID)
|
||||
}
|
||||
if err != nil {
|
||||
dbError(w, action, err)
|
||||
return
|
||||
@@ -152,7 +172,8 @@ func AckAlarm(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
}
|
||||
}
|
||||
|
||||
Debug(action, "POST", "alert=%s event=%s acked=%d destroyed=%t", alertID, eventID, resp.Acked, resp.Destroyed)
|
||||
Debug(action, "POST", "alert=%s event=%s all=%t acked=%d destroyed=%t",
|
||||
alertID, resp.EventID, eventID == uuid.Nil, resp.Acked, resp.Destroyed)
|
||||
writeJSON(w, http.StatusOK, resp)
|
||||
}
|
||||
|
||||
|
||||
+27
-4
@@ -225,6 +225,12 @@ func AlarmStatus(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
// 409 lost in transit must not forfeit the alarm: the next check-in has to
|
||||
// find the same row. A trigger that has since been disarmed is not news, so
|
||||
// that case falls through to the normal body (active:false).
|
||||
deviceHash := ""
|
||||
if !c.IsDevice {
|
||||
if raw := strings.TrimSpace(r.URL.Query().Get("deviceId")); raw != "" {
|
||||
deviceHash = deviceid.Hash(raw)
|
||||
}
|
||||
}
|
||||
var pending *db.PendingDelivery
|
||||
if c.IsDevice {
|
||||
// The device token already identifies the subscription, so its whole
|
||||
@@ -238,10 +244,6 @@ func AlarmStatus(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
pending = &rows[0]
|
||||
}
|
||||
} else {
|
||||
deviceHash := ""
|
||||
if raw := strings.TrimSpace(r.URL.Query().Get("deviceId")); raw != "" {
|
||||
deviceHash = deviceid.Hash(raw)
|
||||
}
|
||||
var err error
|
||||
pending, err = db.PendingTrigger(r.Context(), alertID, c.UserID, deviceHash)
|
||||
if err != nil && !errors.Is(err, db.ErrNotFound) {
|
||||
@@ -261,6 +263,27 @@ func AlarmStatus(w http.ResponseWriter, r *http.Request, payload any) {
|
||||
// not live even if the stored status says "triggered".
|
||||
active = false
|
||||
}
|
||||
// "Active" is a statement about THIS caller, not about the alert's commanded
|
||||
// status. The alert stays triggered until the owner disarms it or every
|
||||
// subscriber acknowledges, so answering a device that already dismissed the
|
||||
// alarm with active:true makes a correct client re-raise the takeover on
|
||||
// every poll — the alarm that will not stay dismissed. A subscriber is
|
||||
// active only while it still owes the newest trigger; the owner (who has no
|
||||
// delivery row) keeps seeing their own alert as live.
|
||||
callerIsOwner := !c.IsDevice && res.Alert.AdminUserID == c.UserID
|
||||
if active && !callerIsOwner {
|
||||
var owed bool
|
||||
if c.IsDevice {
|
||||
owed, err = db.HasPendingDelivery(r.Context(), alertID, c.Sub.ID)
|
||||
} else {
|
||||
owed, err = db.HasPendingDeliveryForUser(r.Context(), alertID, c.UserID, deviceHash)
|
||||
}
|
||||
if err != nil {
|
||||
dbError(w, action, err)
|
||||
return
|
||||
}
|
||||
active = owed
|
||||
}
|
||||
|
||||
view := statusView{
|
||||
Active: active,
|
||||
|
||||
@@ -166,3 +166,82 @@ func AckAlarmDelivery(ctx context.Context, alertID, userID uuid.UUID, deviceID s
|
||||
}
|
||||
return tag.RowsAffected(), nil
|
||||
}
|
||||
|
||||
// HasPendingDelivery reports whether a subscription still owes an alarm — the
|
||||
// question `alarmStatus` has to ask before it calls an alert "active".
|
||||
//
|
||||
// An alert stays commanded "triggered" until the owner disarms it or every
|
||||
// subscriber acknowledges, so the commanded status alone is not the caller's
|
||||
// answer: a device that already dismissed the alarm must be told it owes
|
||||
// nothing, or it re-raises the same takeover forever.
|
||||
func HasPendingDelivery(ctx context.Context, alertID, subscriptionID uuid.UUID) (bool, error) {
|
||||
var owed bool
|
||||
err := Pool().QueryRow(ctx,
|
||||
`SELECT EXISTS (
|
||||
SELECT 1 FROM alarm.alarm_deliveries
|
||||
WHERE alert_id = $1 AND subscription_id = $2 AND acked_at IS NULL)`,
|
||||
alertID, subscriptionID).Scan(&owed)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return owed, nil
|
||||
}
|
||||
|
||||
// HasPendingDeliveryForUser is the account-path equivalent: the caller's debt is
|
||||
// addressed by (alert, user, device) exactly like the socket and the ack, with
|
||||
// an empty device hash meaning "any of this user's subscriptions on the alert".
|
||||
func HasPendingDeliveryForUser(ctx context.Context, alertID, userID uuid.UUID, deviceHash string) (bool, error) {
|
||||
var owed bool
|
||||
err := Pool().QueryRow(ctx,
|
||||
`SELECT EXISTS (
|
||||
SELECT 1
|
||||
FROM alarm.alarm_deliveries d
|
||||
JOIN alarm.subscriptions s ON s.id = d.subscription_id
|
||||
WHERE d.alert_id = $1
|
||||
AND d.acked_at IS NULL
|
||||
AND s.subscriber_user_id = $2
|
||||
AND ($3 = '' OR s.device_id = $3))`,
|
||||
alertID, userID, deviceHash).Scan(&owed)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return owed, nil
|
||||
}
|
||||
|
||||
// AckAllDeliveries clears every alarm this subscription still owes, and reports
|
||||
// how many rows it retired. It is the dismissal that does not know an event id —
|
||||
// a native takeover launch carries only the alert, and re-deriving "which
|
||||
// trigger was I ringing for" from the phone is guesswork. Acknowledging means
|
||||
// "this device has heard the alarm", so clearing the whole outstanding set is
|
||||
// the honest reading and the one that cannot leave a stale debt replaying.
|
||||
func AckAllDeliveries(ctx context.Context, alertID, subscriptionID uuid.UUID) (int64, error) {
|
||||
tag, err := Pool().Exec(ctx,
|
||||
`UPDATE alarm.alarm_deliveries
|
||||
SET acked_at = now()
|
||||
WHERE alert_id = $1 AND subscription_id = $2 AND acked_at IS NULL`,
|
||||
alertID, subscriptionID)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
return tag.RowsAffected(), nil
|
||||
}
|
||||
|
||||
// AckAllForSubscriber is AckAllDeliveries for the account path, addressed by
|
||||
// (alert, user, device) so a caller can only ever clear its own debt.
|
||||
func AckAllForSubscriber(ctx context.Context, alertID, userID uuid.UUID, deviceHash string) (int64, error) {
|
||||
tag, err := Pool().Exec(ctx,
|
||||
`UPDATE alarm.alarm_deliveries d
|
||||
SET acked_at = now()
|
||||
WHERE d.alert_id = $1
|
||||
AND d.acked_at IS NULL
|
||||
AND d.subscription_id IN (
|
||||
SELECT s.id FROM alarm.subscriptions s
|
||||
WHERE s.alert_id = $1
|
||||
AND s.subscriber_user_id = $2
|
||||
AND ($3 = '' OR s.device_id = $3))`,
|
||||
alertID, userID, deviceHash)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
return tag.RowsAffected(), nil
|
||||
}
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
// Copyright (C) 2026 Dcntrlized
|
||||
//
|
||||
// This program is free software: you can redistribute it and/or modify
|
||||
// it under the terms of the GNU Affero General Public License as published
|
||||
// by the Free Software Foundation, either version 3 of the License, or
|
||||
// (at your option) any later version.
|
||||
//
|
||||
// This program is distributed in the hope that it will be useful,
|
||||
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
// GNU Affero General Public License for more details.
|
||||
//
|
||||
// You should have received a copy of the GNU Affero General Public License
|
||||
// along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
|
||||
package db
|
||||
|
||||
// The device-ledger probe: what an anonymous subscriber's dismissal does to the
|
||||
// delivery ledger. Skipped unless DATABASE_URL points at a migrated database
|
||||
// (same gate as TestLedgerProbe):
|
||||
//
|
||||
// DATABASE_URL='postgres://postgres:…@127.0.0.1:55432/dcntrlized?sslmode=disable' \
|
||||
// go test -run TestDeviceLedgerProbe -count=1 -v ./db/
|
||||
//
|
||||
// It proves the three queries `alarmStatus` and `ackAlarm` now depend on:
|
||||
// - a device subscription with no user id still owes (and clears) alarms;
|
||||
// - an "ack all" dismissal retires the whole outstanding set, which is how a
|
||||
// native takeover launch (no event id) stops a replay loop;
|
||||
// - "does this caller owe the alarm?" answers per subscriber, so one device
|
||||
// dismissing does not silence another device's owed alarm.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
func TestDeviceLedgerProbe(t *testing.T) {
|
||||
dsn := os.Getenv("DATABASE_URL")
|
||||
if dsn == "" {
|
||||
t.Skip("DATABASE_URL not set; device ledger probe needs a migrated database")
|
||||
}
|
||||
ctx := context.Background()
|
||||
NewDB()
|
||||
t.Cleanup(Close)
|
||||
|
||||
admin := uuid.New()
|
||||
account := uuid.New()
|
||||
msg := "probe"
|
||||
a, err := CreateAlert(ctx, &Alert{
|
||||
AdminUserID: admin,
|
||||
Name: "probe device ledger",
|
||||
Message: &msg,
|
||||
AdminTokenHash: []byte("probe"),
|
||||
SubscribeToken: "probe-" + uuid.NewString(),
|
||||
ExpiresAt: time.Now().UTC().Add(time.Hour),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAlert: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
_, _ = Pool().Exec(context.Background(), `DELETE FROM alarm.alerts WHERE id = $1`, a.ID)
|
||||
})
|
||||
|
||||
tokenHash := strings.Repeat("c", 64)
|
||||
device, err := CreateSubscription(ctx, &Subscription{
|
||||
AlertID: a.ID,
|
||||
DeviceID: "sha256:" + strings.Repeat("d", 64),
|
||||
DeviceTokenHash: &tokenHash,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateSubscription (device): %v", err)
|
||||
}
|
||||
if _, err := CreateSubscription(ctx, &Subscription{
|
||||
AlertID: a.ID, SubscriberUserID: account, DeviceID: "sha256:" + strings.Repeat("e", 64),
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateSubscription (account): %v", err)
|
||||
}
|
||||
|
||||
_, event, err := TriggerAlert(ctx, a.ID, admin, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("TriggerAlert: %v", err)
|
||||
}
|
||||
if ok, err := HasPendingDelivery(ctx, a.ID, device.ID); err != nil || !ok {
|
||||
t.Fatalf("device owes after trigger: %v %v", ok, err)
|
||||
}
|
||||
if ok, err := HasPendingDeliveryForUser(ctx, a.ID, account, ""); err != nil || !ok {
|
||||
t.Fatalf("account owes after trigger: %v %v", ok, err)
|
||||
}
|
||||
|
||||
// The device dismissal that names no event clears its whole set; a repeat
|
||||
// (or a named ack afterwards) is an idempotent no-op.
|
||||
cleared, err := AckAllDeliveries(ctx, a.ID, device.ID)
|
||||
if err != nil || cleared != 1 {
|
||||
t.Fatalf("AckAllDeliveries = %d, %v; want 1", cleared, err)
|
||||
}
|
||||
if again, err := AckDelivery(ctx, a.ID, device.ID, event.ID); err != nil || again != 0 {
|
||||
t.Fatalf("AckDelivery after ack-all = %d, %v; want 0", again, err)
|
||||
}
|
||||
if ok, err := HasPendingDelivery(ctx, a.ID, device.ID); err != nil || ok {
|
||||
t.Fatalf("device still owes after ack-all: %v %v", ok, err)
|
||||
}
|
||||
replay, err := PendingReplay(ctx, a.ID, device.ID)
|
||||
if err != nil || len(replay) != 0 {
|
||||
t.Fatalf("device replay after ack-all = %d rows, %v; want 0", len(replay), err)
|
||||
}
|
||||
|
||||
// The other subscriber's debt is untouched: acking is per subscription.
|
||||
if destroyed, err := CompleteAlertDeliveries(ctx, a.ID); err != nil || destroyed {
|
||||
t.Fatalf("alert destroyed while another subscriber still owes: %v %v", destroyed, err)
|
||||
}
|
||||
if ok, err := HasPendingDeliveryForUser(ctx, a.ID, account, ""); err != nil || !ok {
|
||||
t.Fatalf("account debt lost when the device acked: %v %v", ok, err)
|
||||
}
|
||||
if got, err := AckAllForSubscriber(ctx, a.ID, account, ""); err != nil || got != 1 {
|
||||
t.Fatalf("AckAllForSubscriber = %d, %v; want 1", got, err)
|
||||
}
|
||||
if destroyed, err := CompleteAlertDeliveries(ctx, a.ID); err != nil || !destroyed {
|
||||
t.Fatalf("last ack did not destroy the alert: %v %v", destroyed, err)
|
||||
}
|
||||
|
||||
// The alert, its subscriptions and the device's token row are all gone.
|
||||
if ok, err := HasPendingDelivery(ctx, a.ID, device.ID); err != nil || ok {
|
||||
t.Fatalf("device owes on a destroyed alert: %v %v", ok, err)
|
||||
}
|
||||
if _, err := GetSubscriptionByToken(context.Background(), tokenHash); err == nil {
|
||||
t.Fatalf("device token still resolves after the alert was destroyed")
|
||||
}
|
||||
}
|
||||
@@ -169,12 +169,15 @@ func ListSubscriptions(ctx context.Context, alertID uuid.UUID) ([]Subscription,
|
||||
|
||||
out := make([]Subscription, 0, 4)
|
||||
for rows.Next() {
|
||||
var s Subscription
|
||||
if err := rows.Scan(&s.ID, &s.AlertID, &s.SubscriberUserID, &s.DeviceID, &s.DeviceLabel,
|
||||
&s.Platform, &s.Enabled, &s.LastSeenAt, &s.Created); err != nil {
|
||||
// Same scan as every other read: subscriber_user_id is NULL for a device
|
||||
// subscription (rev 10) and device_token_hash is part of the column set,
|
||||
// so a hand-written destination list here drifts and breaks the listing
|
||||
// for exactly the rows anonymous access creates.
|
||||
s, err := scanSubscription(rows)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out = append(out, s)
|
||||
out = append(out, *s)
|
||||
}
|
||||
return out, rows.Err()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user