* feat(events): let websocket clients declare channels on connect Observing the events hub over `/api/v1/events/ws` costs more than it should. A read-only consumer has to send a `subscribe` frame within five seconds or be closed with a policy violation, which means implementing the handshake and holding the write half of the socket open purely to satisfy it. That cost is contract, not transport: `subscribe` is the only inbound message this endpoint accepts. Accept the selection on the URL instead. `?channels=catalog,user_state` subscribes on connect, answers with the same `subscribed` frame and per-channel snapshots the handshake produces, and is never put on the grace-period clock. A connection that declares nothing is unchanged — it still owes a subscribe frame within five seconds. Two supporting changes: - Channel selection now resolves through one shared function used by both paths, so the URL and handshake cannot drift on who may subscribe to what. Role, profile-binding, and validity checks are unchanged. - An unrecognized channel name is reported in the existing `rejected` array as `unknown_channel` rather than closing the connection. Closing took down every other channel the client held over one bad name, and a client cannot always know which channels its role allows before asking. Forbidden and profile-scoped channels were already handled this way. `required_action` in the hello frame is `"none"` for a declared connection and `"subscribe"` otherwise; the web type is widened to match. No wire field changes type or disappears, so this stays additive under the v1 rules. Part of #523 AI disclosure: tool Claude Code, model claude-opus-5, fully AI-generated, reviewed and verified by the author before submission. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(events): address review findings on declared-channel subscriptions Three findings from automated review, all verified against the code before acting on them. **Start the reader before declared-channel snapshots (regression).** configureWebSocket installs an absolute read deadline that only pongs extend, and gorilla processes pongs solely inside ReadMessage (conn.go:950, reached only via advanceFrame). The declared path built its snapshots before starting the reader goroutine, so a snapshot slower than the deadline — a loaded jobs/sessions/scans/history query — would kill an otherwise healthy connection the instant reading began. The handshake path never had this problem because its snapshots run downstream of an active reader. Regression test stalls a tasks snapshot past the deadline; it fails with the previous ordering. **Advertise the feature through a capability endpoint.** Adding a client-visible subscription mode without one leaves a read-only client unable to tell, before connecting, whether ?channels= will be honored: an older server ignores it and closes the connection after the grace period, so the client must retain the very handshake this removes. GET /api/v1/events/capability reports both modes, the grace period the handler actually enforces, and the known channels, following the existing per-subsystem convention. **Deduplicate rejections, not just acceptances.** Asking twice for one forbidden channel produced two identical `rejected` entries. Pre-existing — the dedup check sat after the rejection branches — but cheap to correct in the function this PR extracted. Part of #523 AI disclosure: tool Claude Code, model claude-opus-5, fully AI-generated, reviewed and verified by the author before submission. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(events): frame the capability endpoint as a staleness probe Clients are expected to run a current build rather than negotiate down to an old server, so the endpoint is not a branch-on-capability contract. Its value is letting a client distinguish "this server does not do that" from "the connection failed" — the two are indistinguishable from the socket alone, since an older server ignores ?channels= and then closes on the grace period — so it can tell the user the deployment is out of date instead of failing opaquely. Comment-only; no behavior or wire change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(events): bound the subscribed answer and reap unsubscribed connections Second review pass on the declared-channel path. Four issues, all at the edges of the new URL surface rather than in the design itself. Rejections amplified the request. Making an unknown channel non-fatal removed the brake that used to close the connection on the first bad name, and every refusal quotes the name it refuses — so a large ?channels= of distinct garbage produced a far larger `subscribed` frame, buffered server-side. Cap a selection at 32 distinct channels, report the overrun once instead of per name, truncate an echoed name at 64 bytes, and set a 64 KiB read limit on the socket so an oversize frame cannot be buffered whole before it is rejected. The grace period was disarmed by declaring, not by subscribing. Both `?channels=` with no names and a non-admin naming only an admin channel came up subscribed to nothing and were never reaped, each holding a hub subscriber, two goroutines, and an envelope channel that every published event fans into. Disarm on holding a subscription instead. Selection now resolves before the hello frame — it is pure, so nothing moves ahead of the reader — which lets required_action say "subscribe" when the connection really does still owe one. Repeating the parameter dropped channels silently. `?channels=a&channels=b` honored only the first and reported nothing rejected. Read every occurrence. The capability endpoint advertised `plugins`, which is host-to-plugin runtime dispatch and is granted to no role. An admin following the endpoint's stated purpose got `forbidden` while already being admin, and the hardcoded "Admin access required" made it a dead end rather than a soft failure. Split evt.ClientChannels out of AllChannels, advertise that, and word the refusal so it does not promise a remedy that does not exist. A test pins that an admin can subscribe to everything the endpoint names. Each guard was verified to bite by reverting it and watching the test fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
67 lines
3.2 KiB
Go
67 lines
3.2 KiB
Go
package handlers
|
|
|
|
import (
|
|
"encoding/json"
|
|
"net/http"
|
|
|
|
evt "github.com/Silo-Server/silo-server/internal/events"
|
|
)
|
|
|
|
// eventsCapabilityResponse describes how a client may subscribe to the events
|
|
// websocket.
|
|
//
|
|
// Clients are expected to run a current build rather than negotiate down to an
|
|
// old server, so this is not a branch-on-capability contract. It is how a
|
|
// client tells the difference between "this server does not do that" and "the
|
|
// connection failed", which is what lets it say the deployment is out of date
|
|
// instead of failing opaquely: a server predating declared channels ignores
|
|
// ?channels=, answers required_action:"subscribe", and closes the connection
|
|
// after the grace period, which is indistinguishable from a broken socket.
|
|
type eventsCapabilityResponse struct {
|
|
SchemaVersion int `json:"schema_version"`
|
|
// SubscribeFrame reports the handshake: connect, then send a subscribe
|
|
// frame. Always true; named so a future removal is detectable rather than
|
|
// silent.
|
|
SubscribeFrame bool `json:"subscribe_frame"`
|
|
// DeclaredChannels reports that ?channels= is honored on connect, that such
|
|
// a connection is exempt from the subscribe grace period, and that its
|
|
// hello frame carries required_action:"none".
|
|
DeclaredChannels bool `json:"declared_channels"`
|
|
// SubscribeGracePeriodSeconds is how long a connection holding no
|
|
// subscription may stay silent before it is closed. 0 would mean no
|
|
// deadline.
|
|
SubscribeGracePeriodSeconds int `json:"subscribe_grace_period_seconds"`
|
|
// MaxRequestedChannels is the most channels one selection may name, on the
|
|
// URL or in a subscribe frame. Names past it are answered with a single
|
|
// too_many_channels rejection rather than one per name.
|
|
MaxRequestedChannels int `json:"max_requested_channels"`
|
|
// Channels is every channel a client may ask for on this server,
|
|
// independent of role — not every channel the server has: the plugins
|
|
// channel is host-to-plugin runtime dispatch and is granted to no role, so
|
|
// naming it here would advertise a request that can only be refused. What
|
|
// the caller may actually subscribe to arrives as available_channels in the
|
|
// hello frame, which is role-filtered.
|
|
Channels []evt.EventChannel `json:"channels"`
|
|
}
|
|
|
|
// HandleCapability reports the events websocket's subscription capabilities.
|
|
//
|
|
// Per the v1 rules, new functionality is feature-detected rather than inferred
|
|
// from a version. This follows the existing per-subsystem convention
|
|
// (/notifications/capability, /playback/capability, /downloads/capability).
|
|
//
|
|
// A client that finds declared_channels false is talking to a server older than
|
|
// its own expectations; the useful response is to tell the user to update the
|
|
// server, not to silently fall back.
|
|
func (h *EventsHandler) HandleCapability(w http.ResponseWriter, r *http.Request) {
|
|
w.Header().Set("Content-Type", "application/json")
|
|
_ = json.NewEncoder(w).Encode(eventsCapabilityResponse{
|
|
SchemaVersion: 1,
|
|
SubscribeFrame: true,
|
|
DeclaredChannels: true,
|
|
SubscribeGracePeriodSeconds: int(subscribeGracePeriod.Seconds()),
|
|
MaxRequestedChannels: maxRequestedChannels,
|
|
Channels: evt.ClientChannels,
|
|
})
|
|
}
|