diff --git a/.serena/memories/backend/websocket-subscriptions.md b/.serena/memories/backend/websocket-subscriptions.md index 0c44296d47..9d10991695 100644 --- a/.serena/memories/backend/websocket-subscriptions.md +++ b/.serena/memories/backend/websocket-subscriptions.md @@ -28,7 +28,7 @@ Which mutations announce: `delete-team-member`, `leave-team`, `update-team-membe - `take!` is the proven relay path here. `msgbus.clj` does use `sp/alts!` over data channels in production, so there is no general defect — but the relay keeps the simple blocking take plus an independent scheduled task instead. - `ct/diff a b` returns **b − a** (`Duration/between`), so "time until a future instant" is `(inst-ms (ct/diff (ct/now) future))`. - `ct/duration` unit keywords are singular: `{:millis 150}`, `{:minutes 5}`, never `:milliseconds`/`:days`. -- `still-authorized?` takes the predicate itself: `(boolean (authorized?))`. Calling `(boolean pred)` on the closure is always true and silently disables the interval — this exact bug shipped once and no test caught it until the resubscribe-counting test existed. +- `still-authorized?` takes the check itself: `(boolean (check-access))`. Calling `(boolean pred)` on the closure is always true and silently disables the interval — this exact bug shipped once and no test caught it until the resubscribe-counting test existed. Naming rule here: no `?` for the action that queries and can fail (`check-access`), `?` only for the safe yes/no answer (`still-authorized?`). ## Tests diff --git a/backend/src/app/http/websocket.clj b/backend/src/app/http/websocket.clj index 638f3e1e75..206125cbad 100644 --- a/backend/src/app/http/websocket.clj +++ b/backend/src/app/http/websocket.clj @@ -162,7 +162,7 @@ (cf/get :subscription-revalidation-interval default-revalidation-interval)) (defn- still-authorized? - "Runs the `authorized?` permission predicate, reporting instead of + "Runs the `check-access` permission check, reporting instead of throwing when the lookup itself fails. A transient database error must not close a subscription that is still @@ -170,9 +170,9 @@ that fail open, deliberately: the announcement path (`watch-revocations` below) also logs and moves on when a re-check throws. An interrupt is not a lookup failure and is rethrown." - [authorized?] + [check-access] (try - (boolean (authorized?)) + (boolean (check-access)) (catch InterruptedException cause (throw cause)) (catch Throwable cause @@ -241,17 +241,17 @@ (defn- start-relay "Forwards `channel` into the client output channel of `wsp`, and keeps - a re-check of `authorized?` running on the interval. When access no + a re-check of `check-access` running on the interval. When access no longer holds, `close!` tears the subscription down, which closes the channel and ends the loop. `on-forward` runs for every forwarded message, which is how the file relay announces presence." - [{:keys [::ws/output-ch] :as wsp} channel authorized? close! on-forward] + [{:keys [::ws/output-ch] :as wsp} channel check-access close! on-forward] (schedule-revalidation #(not (sp/closed? channel)) (inst-ms (revalidation-interval)) (fn [] - (when-not (still-authorized? authorized?) + (when-not (still-authorized? check-access) (l/info :hint "closing websocket subscription on re-check" :profile-id (::profile-id wsp))