feat(playback): stream abuse control — authoritative monitoring + kill switch

[CoffeeKnyte]

Twenty-seven commits across six batches: server-observed stream monitoring, a durable kill switch, and the async over-cap enforcer — plus five rounds of adversarial review that corrected a lot of what the earlier docs claimed.

Problem

People could quietly exceed their stream limit, and the server couldn't tell.
A household on a shared account, or someone sharing their login, could run more simultaneous streams than their plan allows. The server counted streams only on the machine that happened to serve them, so when Silo ran as more than one API replica each one saw its own slice and none saw the whole picture. Nobody was over the limit as far as any single replica was concerned.

A client could hide a stream by lying about it. Playback progress ("I'm 12 minutes in") is reported by the app. The server was partly trusting those reports to decide whether a stream was still alive. A modified client could keep pulling video while withholding progress, or keep sending progress while pulling nothing — and in the second case the phantom actually outranked real streams, so when the server trimmed someone back to their limit it killed a genuine viewer's stream and left the fake one running.

Banning someone didn't stop what was already playing. An operator could revoke a user's access and the next request would be refused — but a stream already in flight kept pouring, in some cases for hours, because the connection was never hung up. On the audiobook routes the "hang up" step silently did nothing at all.

Un-banning someone didn't always stick. Lifting a ban worked until any replica or restart re-read older state, at which point the ban could come back on its own.

Bulk downloading was invisible past a certain point. The server tracks in-flight file downloads in a fixed-size list. Once that list filled — which one determined person could cause on their own — every further download was served without being recorded. Downloads became invisible to operators exactly when someone was hammering the server hardest.

The documentation claimed more than the code did. Several architecture documents stated the kill switch held everywhere, that monitoring never trusted the client, and that the raw material for re-stream detection was already collected and just needed consuming. None of those were true. Where this PR could not make them true, it now says so plainly rather than quietly leaving the claim standing.

Solution

fix(playback): make session liveness server-observed (fc65dcaf)

streammonitor.LiveLocalSessions substituted Session.LastActivityAt for a zero LastServedAt, and LastActivityAt is advanced by UpdateProgress and by the realtime WebSocket hello/ack/result handlers (internal/api/handlers/session_ws.go:139,152,168). Because streamenforcer.selectVictims keeps the limit most-recently-served streams, a progress-only phantom sorted ahead of a real stream. Reaping had the same root cause — sessionIsInactiveLocked keyed idleness on LastActivityAt.

Per decision A5 (Option C), the projection now emits LastServedAt verbatim (empty when never served, so it sorts stalest), and sessionIsInactiveLocked measures from LastServedAt falling back only to StartedAt.

Two details that took judgment:

• The never-served window is a separate configurable knob (DefaultUnservedSessionGrace, 2m, via SetUnservedSessionGrace) rather than a hardcoded floor, because a floor silently overrides SetLivenessGracePeriods' documented contract (internal/playback/session.go:283).
• Paused sessions holding an open realtime/WebSocket connection stay exempt from reaping. internal/playback/session_paused_grace_test.go encodes issue Playback freezes / won't resume on player state changes (resume after long pause, intro-skip, manual seek) #243, where reaping a paused transcode froze clients. An open, ping-checked connection is server-observed, unlike a reported position, so this stays within Option C while preserving that fix.

fix(playback): share the live-stream picture across replicas (12f5d5f8)

nodesessions.NewTracker is constructed only in proxy/transcode mode (cmd/silo/main.go:666), so integrated streams never reached the shared silo:sessions: namespace.

• internal/nodesessions/publisher.go re-SETs every live record each 10s tick (renewing the 60s TTL) and diffs only deletions.
• Its key namespace comes from a per-process instance id, not resolveNodeIdentity() (cmd/silo/main.go:126). That helper returns SILO_NODE_NAME/NODE_NAME/hostname, so an operator setting it in shared env and scaling would give every replica the same namespace — replicas would then delete each other's records, worse than the blindness being fixed.
• internal/streamenforcer/coordinator.go elects one evaluator per tick with a renewable Lua lease (a plain SET NX locks the holder out of its own next tick), and each pass is bounded by the interval so it cannot outlive the lease.

It also closes the cross-replica gap Batch 3 recorded as a known limitation: mirrorToRedis was an unconditional SET, so two replicas revoking the same key could lose the stronger kill — and edges learn kills only from Redis. It now merges server-side in Lua with the same monotonic semantics as applyLocal. Two things that are easy to get wrong: the comparison uses exact (unix_sec, nsec) pairs, because RFC3339 strings cannot be compared lexicographically (Go omits trailing zeros, so …:00Z sorts after …:00.5Z) and millisecond truncation could retain an older cutoff; and the script merges before deciding to delete, because deleting on a lapsed incoming revocation could remove a live permanent kill written by another replica.

fix(playback): fail closed on transfer-registry saturation (cd9e358b)

Per A7: a per-user concurrent-transfer cap (playback.max_user_concurrent_transfers, default 24) checked before the global limit, and all five call sites refuse rather than serve unmonitored — 429 transfer_limit_exceeded / 503 monitoring_unavailable, both with Retry-After. The handoff listed four sites; internal/api/handlers/ebook_reader.go:167 was the missing fifth.

Two latent bugs fell out of doing it properly: End now decrements the per-user count only when it actually removed an entry, and the two download handlers registered defer transfers.End(transfer.ID) before the service called Begin (internal/api/handlers/downloads.go:450,505), so a duplicate id would have removed another request's live record. Begin moved into the handler beside its End, with Registry.Annotate preserving the service's late enrichment.

Saturation is warned inside the registry — rate-limited per user, carrying route and user id — rather than once per rejected request at each call site. A7 asked for the latter, but an actor parked at its cap would then generate unbounded warning traffic, a log-amplification vector of its own. No information is lost; both fields are already on Transfer.

docs(playback): correct the claim that restream fingerprints are ready to consume (2bbe6021)

Scoping the re-stream heuristic against the real code showed the premise was false. ClientIP is one value per session, not a set: integrated mode stamps it at session creation and no media request updates it (internal/api/handlers/stream.go never touches ClientIP); the edge tracker overwrites records[sessionID] on every Track/EnsureEphemeral; mergeStreams collapses multi-node records to one winner. Several devices pulling one session present as a single address, so no consumer of the existing snapshot can see fan-out. Three documents said otherwise and are corrected.

fix(proxy): resolve edge client IP through the trusted-proxy boundary (d01b5de3)

Found while scoping the above, but independent of it. The edge recorded the connecting peer's address for every monitoring record and ignored forwarding headers, so behind an ingress or load balancer every viewer presented as the same address — the admin session list showed one indistinguishable client per node. The native surface has resolved this correctly via internal/clientip for a while; the edge simply never used it.

Forwarding headers are consulted only when the connecting peer is itself in clientip.trusted_proxies, so a client cannot choose the address it is recorded under. If the trust list fails to load the resolver keeps an empty list, ignoring headers entirely — failing closed on trust rather than failing startup. Reloaded on the node config watcher's change hook so the edge follows the hot-reloadable setting without a restart. Only proxy mode is affected; internal/transcodenode records no client address.

Risk / follow-ups

• Re-stream detection is not implemented, and is filed with its full findings as playback: re-stream detection (C16 fan-out) — the fingerprints are not consumable as-is #522 rather than half-built. The scoping premise was false: it needs bounded viewer observations collected at authenticated media-serve time — a hot-path change — not a snapshot consumer. An IP signal can only ever catch shared session-URL fan-out (C16); C14, a downstream proxy re-broadcasting one pulled stream, is invisible by construction. The edge trust-boundary half of that work is done here (d01b5de3).
• Synchronous admission is still per-process. A6 gave every replica the same monitoring picture, but StartSession's cap check reads only the local SessionManager, so cross-replica excess is trimmed asynchronously (~120s) rather than refused at the door.
• Paused sessions now reap 30m after the last served byte unless a realtime connection is open. A progress-only client pausing longer resumes via the reconstruct path.
• last_served_at is omitted for integrated sessions that have never served. The admin session list shares the enforcer's projection. The field is unchanged when real bytes exist.
• 429/503 are new failure statuses on existing endpoints — additive, nothing repurposed, but client-visible. silo-android and silo-apple need follow-up to retry with backoff on Retry-After; the cap is advertised on GET /downloads/capability for feature detection.
• The enforcer lease is advisory — a Redis error means every replica evaluates, which is the pre-A6 behavior. The Redis revocation merge falls back to a plain SET (logged once) if Lua cannot run.
• An over-cap kill now lasts the token's full reconstructable life (A1), so a wrong count no longer self-heals in 5 minutes. That is why every other known miscount source was fixed first.
• A user cutoff cannot cut an API-key-owned pour — API-key credentials carry no issue time, so they take the zero credential time and IsRevoked fails open. Deliberate; better than substituting time.Now(), which would actively defeat the cutoff.
• In-flight cuts land within ~5s, not instantly — the watcher polls on Options.WatchInterval.
• Ordered bounded projection queue (fix(libraryingest): TV full scans cancelled at settle-window boundary #15) and cutting an RSS feed's in-flight pour on close (feat(ui): shared PageBack component for consistent back navigation #12) are deliberately deferred; both need their whole lifecycle designed together, and a naive fire-and-forget projection is what caused the ghost-session defect fixed in Batch 2.
• internal/access and internal/jellycompat test packages do not compile on origin/main (pre-existing, from feat: emailed invitations, claim + household setup, and server-driven onboarding tour #501: stale UserStore doubles missing GetOnboardingState). Consequence: the entire jellycompat test package never runs, so compat changes here are verified by reading only. Worth fixing separately.

Verification

Raw output, run on this branch at 2bbe6021:

$ go build ./...
(exit 0)

$ make verify-local-paths
scripts/check-local-path-leaks.sh
(exit 0)

$ make migrate-validate
go run github.com/pressly/goose/v3/cmd/goose@v3.27.1 -dir migrations/sql validate
(exit 0)

$ PATH=$PATH:~/go/bin golangci-lint run --new-from-rev=origin/main
internal/access/resolver_test.go:438:28: cannot use stubStore{} (value of struct type stubStore) as userstore.UserStore value in struct literal: stubStore does not implement userstore.UserStore (missing method GetOnboardingState) (typecheck)
package access
1 issues:
* typecheck: 1

$ go test ./internal/... 2>&1 | grep -E '^(--- FAIL|FAIL|ok )' | grep -v '^ok '
FAIL	github.com/Silo-Server/silo-server/internal/access [build failed]
--- FAIL: TestHandleReplanPlaybackV3SeekFailureRecoveryNeverChangesMediaVersion (0.00s)
FAIL
FAIL	github.com/Silo-Server/silo-server/internal/api/handlers	23.999s
FAIL	github.com/Silo-Server/silo-server/internal/jellycompat [build failed]
--- FAIL: TestServeDirectPlayChangedEntityRejectsOldIfRange (0.00s)
FAIL
FAIL	github.com/Silo-Server/silo-server/internal/playback	8.133s
FAIL

All four remaining failures are pre-existing on origin/main, confirmed against a clean worktree: the two build failures above, and two timing/environment-sensitive tests (TestServeDirectPlayChangedEntityRejectsOldIfRange hashes Ctim at the kernel's coarse clock granularity, so two rapid rewrites land in the same tick).

• go test -race is clean across every touched package: playback, streammonitor, streamenforcer, streamrevoke, nodesessions, transfers, downloads, config, audiobooks/abs, api/..., cmd/silo.
• The Lua-executing tests were run against a real Redis 8.6.2 (EVAL + cjson confirmed) with SILO_TEST_REDIS_ADDR=127.0.0.1:6380; all seven pass. They t.Skip when that variable is unset.
• Every new regression test was verified non-vacuous by reverting its fix and confirming failure. For the Redis merge specifically, bypassing the Lua back to the old unconditional SET fails four of the five merge tests.

Two verification gaps, stated rather than omitted:

• pnpm is absent on this host, so make build, pnpm run lint and pnpm run format:check could not run locally. No frontend files are changed in this PR, so this is a completeness gap, not an untested change.
• This repository has no CI job that runs go test — .github/workflows/ contains only build, labeler and bot jobs. All Go test evidence above is therefore local, and the gated Redis tests will skip everywhere until a Redis service is added to CI. Worth fixing separately; it is why the jellycompat compile break has gone unnoticed.

AI Disclosure

• Tool(s): Claude Code, Codex CLI
• Model(s): claude-opus-5 (planning, reconciliation, review, verification), gpt-5.6-sol at medium effort (adversarial plan review and implementation)
• Involvement: AI-assisted — cross-model relay. Claude planned and reviewed; Codex critiqued each plan before any code was written and then implemented it; Claude re-ran every verification independently and hand-verified each finding against the code.
• Adversarial review: The full round-by-round record, including rejected claims, is in docs/architecture/stream-abuse-matrix.md under "AI-use disclosure". Highlights:
  • Codex caught that fixing GAP-10 alone was inert, because the same Unwrap() that makes SetWriteDeadline work also lets bump() erase the cut; an ABA bug in a refcount design; and that narrowing opMu was unsafe.
  • On the final batch it caught that the proposed Lua merge's ttl <= 0 ⇒ DEL could delete a stronger concurrent revocation, and that this repo's Redis test doubles are hand-written ProcessHook fakes that cannot execute Lua — so the originally proposed merge tests would have proven nothing. Both fixed; the tests now run against real Redis.
  • Claude's review of Codex's work found a failing real-socket ebook test Codex reported as passing (its sandbox cannot open httptest listeners), GAP-12 left unfixed on the proxy edge path, and a test asserting last_served_at == LastActivityAt that encoded the very fallback being removed.
  • Claims rejected on verification and deliberately not acted on: that chi's middleware.Compress breaks the Unwrap chain (chi v5.2.5 implements it at middleware/compress.go:374); that client progress advances Session.LastServedAt; and that the new setting required an Admin UI field, which the branch's own precedent (playback.over_cap_revocation_ttl) contradicts.
  • Codex's sandbox blocks loopback TCP and cannot initialise Sonyflake, so it reported those tests as unrun rather than passing. Every one was re-run here.

Part of #305

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR wires a centralized stream-revocation store through playback, downloads, transcode, proxy, jellycompat, and admin paths, adds live-session snapshot/enforcement plumbing, and updates the architecture docs to match the shipped monitoring and kill-switch behavior.

Changes

Stream monitoring and kill-switch feature

Layer / File(s)	Summary
Revocation and origin contracts internal/streamrevoke/*, internal/streamtoken/token.go, internal/playback/session.go, migrations/sql/20260705025758_stream_revocations.sql	Adds the revocation store, durable Postgres backend, stream token claims, playback origin fields, and the revocation table migration.
Live monitoring and cap enforcement internal/streammonitor/*, internal/nodesessions/tracker.go, internal/streamenforcer/*	Adds live-stream snapshots, deduplication, enriched session tracking, and asynchronous over-cap enforcement.
Proxy and transcode-node revocation enforcement internal/proxy/*, internal/transcodenode/server.go	Adds revocation checks, in-flight cut hooks, byte accounting, and request/session metadata in proxy and transcode-node serving.
API routing, handlers, and middleware wiring internal/api/*	Adds integrated-mode revocation gating, admin termination revocation, local session listing, and response-writer unwrapping.
cmd/silo/main.go orchestration wiring cmd/silo/main.go	Constructs revocation stores for proxy, transcode, and integrated modes, starts the enforcer, injects dependencies, and propagates user-session revocations.
Architecture and plan documentation docs/architecture/playback-paths-monitoring-kill-matrix.md, docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md	Adds the playback monitoring and kill-switch matrix and updates the implementation plan to shipped status and follow-ups.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Enforcer as streamenforcer.Enforcer
  participant Source as streammonitor.Source
  participant Revoker as streamrevoke.Store
  participant Edge as proxy/transcode/API/jellycompat
  Enforcer->>Source: Snapshot(ctx)
  Source-->>Enforcer: live streams by user
  Enforcer->>Revoker: RevokeSessionFor(sessionID, reason, ttl)
  Edge->>Revoker: Refuse(sessionID, userID, startedAt)
  Edge->>Revoker: WatchAndCut(writer, sessionID, userID, startedAt)

Loading

Possibly related issues

• Protect against stream abuse: authoritative monitoring + kill switch for cap violations, admin terminate, and re-streaming #305: Matches the same runtime wiring for stream monitoring and the kill-switch system, including revocation stores and the async enforcer.

Possibly related PRs

• Silo-Server/silo-server#174: Both PRs modify the playback stop/terminate control path to enforce revocation/denial.
• Silo-Server/silo-server#258: Both PRs touch internal/api/handlers/downloads.go and its routing/wiring.
• Silo-Server/silo-server#262: Both PRs update Jellycompat playback liveness handling around BeginTransport/EndTransport.

Suggested labels: v1

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the PR's primary changes: authoritative playback monitoring and a stream revocation kill switch.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/sauron-async-enforcer

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69d616b964

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Fall back to the authenticated user for native cuts

For native direct/remux streams whose URL has no valid st token (for example a legacy/bare stream URL or signing disabled), this returns 0 even though these routes run after API auth has populated the request context. OnUserSessionsRevoked only writes a user-level revocation, so Refuse/WatchAndCut will not match and an already-open stream can keep running after an admin disables or changes that user; use apimw.GetUserID(r.Context()) as the fallback when the stream token is absent or invalid.

Useful? React with 👍 / 👎.

[CoffeeKnyte]

Fixed in 1232f24. streamRequestUserID now falls back to the authenticated user (apimw.GetUserID(r.Context())) when there's no valid st token, so a per-user revocation still bites a bare/legacy native stream URL.

[CoffeeKnyte]

Still fixed after the rebase (branch re-pushed; SHAs changed). streamRequestIdentity falls back to apimw.GetUserID(r.Context()) plus the request entry time when no valid st token is present — internal/api/router.go:3537.

[chatgpt-codex-connector]

Preserve Redis revocation expiry monotonically

This Redis SET always stores the incoming revocation, so a short over-cap re-revocation can replace the Redis mirror for an earlier 24h admin kill on the same session. Edges that restart or warm from Redis during that window (and Redis-only deployments without durable re-arm) can learn only the 5m expiry and let the killed stream reconnect early; mirror the later of the existing and new expiries, matching applyLocal and the Postgres upsert.

Useful? React with 👍 / 👎.

[CoffeeKnyte]

Fixed in 1232f24. Revoke now mirrors the merged copy (s.effective(...)) to Redis rather than the incoming one, so a short over-cap re-revoke can no longer shorten a longer admin kill's Redis TTL — matching applyLocal and the durable GREATEST upsert.

[CoffeeKnyte]

Still fixed after the rebase. Revoke mirrors the monotonically-merged copy, s.mirrorToRedis(ctx, s.effective(r.key())) — internal/streamrevoke/store.go:348, and the same at the two warm/reconcile sites (597, 678).

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (2)

internal/streamrevoke/store.go (1)

170-196: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

stop() is not safe to call twice.

The returned func() { close(done) } will panic if invoked more than once (e.g. a caller defers it and also calls it explicitly on an error path). Since this is the shared cut-helper for every long-pour serving surface, a double-call anywhere would crash that node.

♻️ Optional hardening with sync.Once

+	var once sync.Once
-	return func() { close(done) }
+	return func() { once.Do(func() { close(done) }) }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/streamrevoke/store.go` around lines 170 - 196, The returned cleanup
function from Store.WatchAndCut is not idempotent, so calling it twice can panic
when it closes the same done channel more than once. Update WatchAndCut to make
the stop/cut closure safe for repeated calls, using a one-time guard such as
sync.Once around the close(done) path, while keeping the existing revoke-check
and ticker goroutine behavior unchanged.

internal/proxy/server.go (1)

191-228: 🚀 Performance & Scalability | 🔵 Trivial

Wrapping the writer disables sendfile zero-copy for direct-play/remux.

http.ServeFile normally serves from *os.File via the underlying ResponseWriter's io.ReaderFrom (sendfile). Because sessionByteWriter only implements Write, the copy now falls back to userspace buffering for every direct-play/remux pour — the highest-throughput path. You can keep byte attribution and zero-copy by forwarding to the underlying ReaderFrom via io.Copy (which selects sendfile when the destination supports it) and counting the returned bytes.

♻️ Preserve sendfile while counting bytes

 func (w *sessionByteWriter) Unwrap() http.ResponseWriter {
 	return w.ResponseWriter
 }
+
+// ReadFrom preserves the underlying writer's sendfile fast path (http.ServeFile
+// serves *os.File via io.ReaderFrom) while still attributing served bytes.
+func (w *sessionByteWriter) ReadFrom(src io.Reader) (int64, error) {
+	n, err := io.Copy(w.ResponseWriter, src) // selects sendfile if supported
+	if n > 0 {
+		w.acc += n
+		if w.acc >= 1<<20 {
+			w.tracker.AddBytes(w.sessionID, w.acc)
+			w.acc = 0
+		}
+	}
+	return n, err
+}

Please confirm internal/nodesessions.Tracker exposes AddBytes(sessionID string, n int64) and that the underlying edge ResponseWriter chain still exposes ReadFrom after this wrapper (verify with the sendfile path in net/http).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/proxy/server.go` around lines 191 - 228, The sessionByteWriter
wrapper currently only exposes Write, which breaks the net/http sendfile fast
path for direct-play/remux. Update sessionByteWriter.Write or add a ReadFrom
passthrough so the wrapper forwards to the underlying ResponseWriter’s
ReaderFrom/ReadFrom when available, while still counting bytes via
tracker.AddBytes(sessionID, n). Keep sessionByteWriter.Unwrap intact and ensure
the wrapped edge ResponseWriter chain still preserves ReadFrom support.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/architecture/playback-paths-monitoring-kill-matrix.md`:
- Line 15: Clarify the token-protocol statement in the playback-paths monitoring
kill matrix so it does not conflict with the coverage note about the route
Origin claim being present in the token. Update the surrounding wording to
either state that the Origin claim is server-internal/opaque to clients or
soften the “client-facing token protocol is unchanged” promise, and make the
terminology consistent in that section.
- Around line 12-13: Update the durability summary in the playback-paths
monitoring kill matrix so it matches the shipped kill path state. In the
affected narrative section, revise the wording around the durable Postgres
mirror to reflect that it is now part of the implemented restart-surviving kill
flow, not merely an optional mirror. Keep the terminology consistent with the
status banner and the as-built delta elsewhere in the document so readers get
one clear answer.

In `@internal/api/handlers/playback.go`:
- Around line 2875-2885: The playback segment handler currently ignores a failed
h.sessionMgr.BeginTransport(sessionID) call and continues silently, leaving no
trace when the liveness marker is missing. Update the segment-serving path in
playback.go to log the BeginTransport error with enough context (sessionID and
segmentPath) before serving the file, while still preserving the existing
fallback behavior and deferred EndTransport only on success.

In `@internal/api/router.go`:
- Around line 2184-2190: The subtitle and font stream endpoints are still
bypassing revocation checks, unlike the main stream routes. Update the route
registrations in the stream routes block so `streamHandler.HandleSubtitle` and
`streamHandler.HandleSubtitleFonts` are wrapped with
`guardRevocationCut`/`guardRevocation` using `deps.RevocationStore` and
`configJWTSecret(deps)`, matching the existing protection used by
`HandleStream`.

In `@internal/streamrevoke/store.go`:
- Around line 338-341: `RevokeUser` should reject non-positive user IDs to match
`IsRevoked`’s `KindUser` behavior and avoid creating ineffective revocations.
Add a guard at the start of `Store.RevokeUser` that checks `userID <= 0` and
returns an error instead of calling `Revoke`; keep the existing
`userKey(userID)` flow only for valid positive IDs. Use the existing
`RevokeUser`, `Revoke`, and `userKey` symbols to locate the change.
- Around line 292-319: mirrorToRedis currently treats zero ExpiresAt as already
expired because time.Until returns a negative duration, so permanent revocations
get deleted from Redis instead of being stored. Update the mirrorToRedis flow in
Store to detect r.ExpiresAt.IsZero() before the TTL check and write the Redis
key without an expiration in that case, while keeping the existing TTL-based Set
for non-permanent revocations and the Del path only for truly expired entries.

---

Nitpick comments:
In `@internal/proxy/server.go`:
- Around line 191-228: The sessionByteWriter wrapper currently only exposes
Write, which breaks the net/http sendfile fast path for direct-play/remux.
Update sessionByteWriter.Write or add a ReadFrom passthrough so the wrapper
forwards to the underlying ResponseWriter’s ReaderFrom/ReadFrom when available,
while still counting bytes via tracker.AddBytes(sessionID, n). Keep
sessionByteWriter.Unwrap intact and ensure the wrapped edge ResponseWriter chain
still preserves ReadFrom support.

In `@internal/streamrevoke/store.go`:
- Around line 170-196: The returned cleanup function from Store.WatchAndCut is
not idempotent, so calling it twice can panic when it closes the same done
channel more than once. Update WatchAndCut to make the stop/cut closure safe for
repeated calls, using a one-time guard such as sync.Once around the close(done)
path, while keeping the existing revoke-check and ticker goroutine behavior
unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c123b56d-0a2f-4c92-a1b1-fa3ea173469c

📥 Commits

Reviewing files that changed from the base of the PR and between e5379cd and 69d616b.

📒 Files selected for processing (32)

• cmd/silo/main.go
• docs/architecture/playback-paths-monitoring-kill-matrix.md
• docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md
• internal/api/handlers/admin_playback_control.go
• internal/api/handlers/downloads.go
• internal/api/handlers/nodes.go
• internal/api/handlers/playback.go
• internal/api/middleware/metrics.go
• internal/api/middleware/request_logger.go
• internal/api/router.go
• internal/jellycompat/auth.go
• internal/jellycompat/handlers_playback.go
• internal/jellycompat/image_proxy_tags.go
• internal/jellycompat/logging.go
• internal/jellycompat/router.go
• internal/jellycompat/server.go
• internal/jellycompat/streams.go
• internal/nodesessions/tracker.go
• internal/playback/session.go
• internal/playback/session_test.go
• internal/proxy/egress.go
• internal/proxy/server.go
• internal/streamenforcer/enforcer.go
• internal/streamenforcer/enforcer_test.go
• internal/streammonitor/monitor.go
• internal/streammonitor/monitor_test.go
• internal/streamrevoke/durable_postgres.go
• internal/streamrevoke/store.go
• internal/streamrevoke/store_test.go
• internal/streamtoken/token.go
• internal/transcodenode/server.go
• migrations/sql/20260705025758_stream_revocations.sql

[CoffeeKnyte]

Also addressed the two nitpicks from the review body:

• stop() not safe to call twice (streamrevoke.WatchAndCut) — the returned cut closure now guards close(done) with a sync.Once, so a double-call can no longer panic. (1232f24)
• Wrapping the writer disables sendfile zero-copy (proxy.sessionByteWriter) — added a ReadFrom that forwards to the underlying io.ReaderFrom to keep the sendfile fast path on direct-play/remux while still counting served bytes, with a non-recursive manual-copy fallback. Covered by tests for both paths. (045e5f6)

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md (1)

3-3: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Downgrade the completion banner until the config follow-up lands.

This says "IMPLEMENTED" and "all three phases shipped," but the same document later notes auth.stream_revocation_poll / auth.stream_revocation_ttl are still unwired. Please qualify the banner (for example, "implemented with remaining config follow-up") or move the remaining item out of the shipped section so the doc doesn't overstate completion.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md` at
line 3, The completion banner is overstating readiness because the plan still
has unwired config follow-up for auth.stream_revocation_poll and
auth.stream_revocation_ttl. Update the opening status text in this plan document
to qualify the implementation state (for example, “implemented with remaining
config follow-up”) or move the unresolved config item out of the shipped summary
so the document accurately reflects completion.

🧹 Nitpick comments (1)

internal/proxy/session_byte_writer_test.go (1)

51-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider covering the ≥1MiB flush branch.

Both tests use short payloads, so account()'s w.acc >= 1<<20 branch that calls tracker.AddBytes is never exercised here (and tracker is left nil in these tests, which only stays safe because that branch isn't hit). A follow-up test with a fake/stub tracker and a payload ≥1MiB would close this gap without weakening the current recursion/fast-path assertions.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/proxy/session_byte_writer_test.go` around lines 51 - 97, The
sessionByteWriter tests only cover small payloads, so the account() path that
triggers tracker.AddBytes when w.acc >= 1<<20 is never exercised. Add a
follow-up test around sessionByteWriter.account (or the ReadFrom flow that
reaches it) using a fake tracker and a payload at least 1MiB so the flush branch
is covered, while keeping the existing ReadFrom fast-path and fallback
no-recursion assertions intact.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md`:
- Line 3: The completion banner is overstating readiness because the plan still
has unwired config follow-up for auth.stream_revocation_poll and
auth.stream_revocation_ttl. Update the opening status text in this plan document
to qualify the implementation state (for example, “implemented with remaining
config follow-up”) or move the unresolved config item out of the shipped summary
so the document accurately reflects completion.

---

Nitpick comments:
In `@internal/proxy/session_byte_writer_test.go`:
- Around line 51-97: The sessionByteWriter tests only cover small payloads, so
the account() path that triggers tracker.AddBytes when w.acc >= 1<<20 is never
exercised. Add a follow-up test around sessionByteWriter.account (or the
ReadFrom flow that reaches it) using a fake tracker and a payload at least 1MiB
so the flush branch is covered, while keeping the existing ReadFrom fast-path
and fallback no-recursion assertions intact.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ba768ce-b95e-4048-9ed0-96ef5c3daf44

📥 Commits

Reviewing files that changed from the base of the PR and between 69d616b and 045e5f6.

📒 Files selected for processing (7)

• docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md
• internal/api/handlers/playback.go
• internal/api/router.go
• internal/proxy/server.go
• internal/proxy/session_byte_writer_test.go
• internal/streamrevoke/store.go
• internal/streamrevoke/store_test.go

🚧 Files skipped from review as they are similar to previous changes (5)

• internal/api/handlers/playback.go
• internal/streamrevoke/store_test.go
• internal/api/router.go
• internal/proxy/server.go
• internal/streamrevoke/store.go

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/architecture/playback-paths-monitoring-kill-matrix.md (1)

401-406: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Split the shipped note from the open follow-up and renumber the list.

This item repeats 4. and also mixes the already-shipped context.WithoutCancel hardening with the still-open durable-write self-heal gap. That makes the remaining work harder to scan. Please move the shipped note elsewhere and renumber the open items sequentially.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/architecture/playback-paths-monitoring-kill-matrix.md` around lines 401
- 406, The kill-matrix note mixes shipped behavior with the remaining open
follow-up and repeats the numbering, so split the
`Revoke`/`context.WithoutCancel` note from the durable-write self-heal item and
renumber the list sequentially. Update the relevant bullet in the monitoring
kill matrix so the `Revoke` hardening is documented as shipped separately, and
keep the open memory→durable re-mirror work as its own item under a unique
number to make the remaining follow-up easier to scan.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/architecture/playback-paths-monitoring-kill-matrix.md`:
- Around line 401-406: The kill-matrix note mixes shipped behavior with the
remaining open follow-up and repeats the numbering, so split the
`Revoke`/`context.WithoutCancel` note from the durable-write self-heal item and
renumber the list sequentially. Update the relevant bullet in the monitoring
kill matrix so the `Revoke` hardening is documented as shipped separately, and
keep the open memory→durable re-mirror work as its own item under a unique
number to make the remaining follow-up easier to scan.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b0dd814f-16cf-47c5-85cc-8f42bb3498e7

📥 Commits

Reviewing files that changed from the base of the PR and between 045e5f6 and 31be2d5.

📒 Files selected for processing (16)

• docs/architecture/playback-paths-monitoring-kill-matrix.md
• internal/api/handlers/admin_playback_control.go
• internal/api/handlers/downloads.go
• internal/api/handlers/nodes.go
• internal/api/router.go
• internal/jellycompat/streams.go
• internal/nodesessions/tracker.go
• internal/proxy/egress.go
• internal/proxy/server.go
• internal/proxy/session_byte_writer_test.go
• internal/streammonitor/monitor.go
• internal/streammonitor/monitor_test.go
• internal/streamrevoke/store.go
• internal/streamrevoke/store_test.go
• internal/streamtoken/token.go
• internal/transcodenode/server.go

🚧 Files skipped from review as they are similar to previous changes (7)

• internal/streammonitor/monitor_test.go
• internal/api/handlers/nodes.go
• internal/api/handlers/admin_playback_control.go
• internal/proxy/server.go
• internal/streamrevoke/store_test.go
• internal/nodesessions/tracker.go
• internal/transcodenode/server.go

[CoffeeKnyte]

Branch rebased onto latest main and reorganized into 3 commits

Heads-up on a force-push I just did: 31be2d52 → 43193e8c.

The branch had fallen 18 commits behind main, so I rebased it onto the current main tip (ea2d1762). That needed exactly one merge-conflict fix — a single hunk in cmd/silo/main.go where this branch's streamRevocation store init and main's new ipResolver var landed in the same place. They're independent additions, so both were kept (~18 lines). Everything else merged cleanly.

Since I was rebasing anyway, I re-split the old 9-commit history (3 feature commits + 6 incremental review-fix commits) into 3 clean, reviewable commits:

1. ce480577 — feat(playback): server-observed stream monitoring — the monitoring base layer (streammonitor, tracker serve-activity, client-identity fields, admin observation surfaces). Builds + tests pass in isolation.
2. aec40135 — feat(playback): stream kill switch + async over-cap enforcer — the enforcement layer (streamrevoke, streamenforcer, durable Postgres mirror, serve-path guards, migration).
3. 43193e8c — docs(playback): monitoring & kill-switch plan + as-built coverage matrix.

The straddling files (streamtoken/token.go, transcodenode/server.go) were hunk-split so the identity plumbing sits with monitoring and the revocation guards sit with the kill switch, keeping commit 1 buildable on its own.

I verified the final tree is byte-for-byte identical to a correct merge of the old branch tip with latest main (plus the two review-driven changes noted in the follow-up comment). go build ./..., go vet, gofmt, and all touched-package tests are green.

Then I added a new test and tidied the docs in response to the two latest CodeRabbit reviews — see the follow-up comment.

[CoffeeKnyte]

Addressed the two latest CodeRabbit reviews

Both sets of findings were posted as outside-diff-range notes in the review bodies (no inline threads), so acknowledging them here. All three are fixed and squashed into the appropriate commit:

• ✅ plan.md completion banner overstated readiness — review. Qualified the banner to "IMPLEMENTED (with a minor operator-config follow-up open)" and clarified that the unwired auth.stream_revocation_poll / auth.stream_revocation_ttl keys are operator ergonomics, not an unshipped phase. → 43193e8c

• ✅ session_byte_writer_test.go never exercised the ≥1 MiB flush branch — review. Added TestSessionByteWriterAccountFlushBranch: it drives a 1 MiB pour through a real Redis-less nodesessions.Tracker (which makes AddBytes a safe no-op), proving account()'s coarse-flush branch is taken, resets the accumulator, and is nil-safe against a real tracker — no Redis dependency needed. → aec40135

• ✅ coverage-matrix follow-up list had a duplicate 4. and mixed shipped/open items — review. Split the already-shipped context.WithoutCancel note into an "already-shipped hardening" line and renumbered the remaining open items sequentially (1–8). → 43193e8c

The two nitpicks CodeRabbit flagged as low-value were still worth doing — the test closes a real coverage gap and the doc fixes remove genuine inconsistencies.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/architecture/playback-paths-monitoring-kill-matrix.md`:
- Around line 181-208: Qualify the restart-survival wording in the kill matrix
so it does not imply unconditional durability. Update the table entries for the
integrated/Redis and multi-node/Redis cases, or add a footnote near the matrix,
to reflect the edge caveat described below it: boot-time durable warm can fail
open and leave the kill list empty until the first poll tick. Keep the existing
guarantees in the surrounding bullets consistent with the matrix text,
especially the restart and Redis-flush rows.

In `@docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md`:
- Line 15: The token-immutability statement is too broad because the plan later
adds an opaque Origin claim, so rephrase the text in the plan to say the
client-facing wire protocol stays unchanged rather than the token payload being
unchanged. Update the wording in the plan section that discusses the 24h stream
token, keeping the focus on sid/uid/pid revocation keys and preserving
consistency with the Origin claim addition.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e06e0610-63c8-419a-b5f9-c160a72c96d0

📥 Commits

Reviewing files that changed from the base of the PR and between 31be2d5 and 43193e8.

📒 Files selected for processing (33)

• cmd/silo/main.go
• docs/architecture/playback-paths-monitoring-kill-matrix.md
• docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md
• internal/api/handlers/admin_playback_control.go
• internal/api/handlers/downloads.go
• internal/api/handlers/nodes.go
• internal/api/handlers/playback.go
• internal/api/middleware/metrics.go
• internal/api/middleware/request_logger.go
• internal/api/router.go
• internal/jellycompat/auth.go
• internal/jellycompat/handlers_playback.go
• internal/jellycompat/image_proxy_tags.go
• internal/jellycompat/logging.go
• internal/jellycompat/router.go
• internal/jellycompat/server.go
• internal/jellycompat/streams.go
• internal/nodesessions/tracker.go
• internal/playback/session.go
• internal/playback/session_test.go
• internal/proxy/egress.go
• internal/proxy/server.go
• internal/proxy/session_byte_writer_test.go
• internal/streamenforcer/enforcer.go
• internal/streamenforcer/enforcer_test.go
• internal/streammonitor/monitor.go
• internal/streammonitor/monitor_test.go
• internal/streamrevoke/durable_postgres.go
• internal/streamrevoke/store.go
• internal/streamrevoke/store_test.go
• internal/streamtoken/token.go
• internal/transcodenode/server.go
• migrations/sql/20260705025758_stream_revocations.sql

💤 Files with no reviewable changes (22)

• internal/jellycompat/router.go
• migrations/sql/20260705025758_stream_revocations.sql
• internal/streamtoken/token.go
• internal/streamenforcer/enforcer_test.go
• internal/jellycompat/image_proxy_tags.go
• internal/playback/session.go
• internal/proxy/session_byte_writer_test.go
• internal/playback/session_test.go
• internal/streamrevoke/store_test.go
• internal/jellycompat/server.go
• internal/jellycompat/logging.go
• internal/proxy/server.go
• internal/streammonitor/monitor_test.go
• internal/jellycompat/handlers_playback.go
• internal/proxy/egress.go
• internal/streamrevoke/durable_postgres.go
• internal/streamenforcer/enforcer.go
• internal/streammonitor/monitor.go
• internal/jellycompat/streams.go
• internal/nodesessions/tracker.go
• internal/transcodenode/server.go
• internal/streamrevoke/store.go

🚧 Files skipped from review as they are similar to previous changes (9)

• internal/jellycompat/auth.go
• internal/api/middleware/metrics.go
• internal/api/middleware/request_logger.go
• internal/api/handlers/downloads.go
• internal/api/handlers/admin_playback_control.go
• cmd/silo/main.go
• internal/api/handlers/nodes.go
• internal/api/router.go
• internal/api/handlers/playback.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/architecture/playback-paths-monitoring-kill-matrix.md`:
- Line 4: The blockquote at the top of the document contains an empty quoted
line, which triggers markdownlint MD028. Update the opening quote text so the
blockquote is continuous with no blank line inside it, preserving the existing
wording in the document header.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d1887377-17cd-4552-a21b-41fcfb6f1b68

📥 Commits

Reviewing files that changed from the base of the PR and between 43193e8 and 8ff4577.

📒 Files selected for processing (2)

• docs/architecture/playback-paths-monitoring-kill-matrix.md
• docs/superpowers/plans/2026-07-04-stream-monitoring-and-kill-switch.md

[CoffeeKnyte]

Changed to draft momentarily to assess the architecture for now.

[CoffeeKnyte]

Rebased + hardening pass

Force-pushed: this branch is rebased onto current main, so the earlier commit SHAs changed and the inline review comments above now show as outdated. Every one of them was re-verified against the rebased code and replied to in place — none regressed.

Six new commits on top. Highlights:

Full byte visibility. Integrated deployments recorded bytes_served: 0 for every stream — bytes were only ever counted at the edge. There is now one shared metered writer (sendfile-preserving, Unwrap-able so the kill cut still reaches the socket) wired at every integrated pour, and LastServedAt is a distinct field advanced only by server-observed events, so a client progress report can no longer influence which of its own streams the over-cap enforcer trims.

The Audiobookshelf surface was entirely outside the design — three byte-serving routes, one of them unauthenticated (the public RSS feed). All now enforce revocation with their real credential-issue time; ABS bearer tokens are stateless JWTs that survive OnUserSessionsRevoked, so request-entry time would have meant a user kill never refused a later ABS request.

Downloads are visible, on a separate path. All six download-class pours (native ×2, jellycompat, ABS ×2, RSS feed) are tracked in a process-local in-memory registry and surface as a sibling transfers array on the admin sessions endpoint. streammonitor and streamenforcer are untouched: downloads stay off the live-stream path by construction, so one can never be counted against max_streams or trimmed as an over-cap stream. No schema change, no migration.

Downloads are killable today via user revocation — revoking a user cuts their in-flight download pours mid-transfer. Note this is a cutoff, not a standing block: it kills pours whose credential predates it and deliberately allows new ones. Killing a single download, and a standing per-user download block, are deferred; the registry already mints a unique per-pour id so that work only changes "" to that id at each WatchAndCut site.

Operator kill list. GET/POST/DELETE /api/v1/admin/streams/revocations. Store.List() previously existed but was reachable from nowhere, and because expiry is monotonic a mistaken kill was irreversible — Unrevoke is tombstone-guarded so a concurrent poll reconcile cannot resurrect it.

VERIFY-4 (buffer-ahead evasion) closed with a bounded 10m transcode grace measured from the server-observed clock, plus a 180s idle window for transcode records at the edge.

Still deferred, stated rather than implied: multi-replica enforcement (VERIFY-3), the transcode-node ghost sweep, a shared download quota / rolling volume budget, and phase 2 of download kill controls.

Verification: go build ./... passes; go test ./... shows only failures that reproduce on a clean checkout (two stale UserStore test doubles missing GetOnboardingState, plus two known flaky tests); lint unchanged from baseline; make verify-local-paths passes. make build and the frontend checks could not run (no pnpm on the build host) — no frontend changes are in scope.

[Quick104]

Automated review: architecture and claim validation

Automated-review disclosure: This review was produced by Codex in the T3 Code Codex harness using model gpt-5.6-sol with reasoning effort max. Independent specialist passes covered author-claim auditing, route coverage, distributed failure modes, security, repository Standards, and issue #305 Spec compliance. The reviewed head is 09de4f08ec9f1ffeadda396327d7612b8afed793.

Overall verdict

The direction is sound, but I do not think this head should merge as a complete resolution of #305.

The server should own stream observation and termination. A signed token can authorize access, but it cannot revoke itself, enforce a fleet-wide cap, or terminate an already-open response. Server-observed activity, an in-memory serve-path deny cache, asynchronous reconciliation, and durable revocation state are therefore appropriate primitives.

The problem is that this implementation materially overstates “every byte-serving path,” “never trusts client progress,” “stop any stream,” and “keep it dead.” Several failures occur on supported production topologies and ordinary Range/reconnect behavior, not just exotic configurations.

The branch is also no longer rebased onto current main: live base is a7601028d9d606f1de7f9ec6bef72c759de7c91a, actual merge-base is 08035f38067ea485b3938bec7a3bf48da212f46c, and the PR is currently 11 commits ahead / 8 behind.

Claims that are supported

• Cross-process/fleet admission and direct/remux termination were real gaps.
• streamrevoke.Store.IsRevoked is an in-memory-only lookup.
• Exact-session refusal is broadly wired across native, proxy, Jellycompat, and transcode paths.
• With healthy dependencies, PostgreSQL warm-up plus Redis re-arming makes ordinary default/admin kills restart- and Redis-flush-durable.
• Integrated native and compat HLS now hold server transport spans and meter bytes.
• The enforcer uses the same effective/group-merged limit as admission.
• Download transfers are structurally separate from the live-stream cap.
• The admin revocation endpoints are admin-protected and have strong input validation.
• Simply changing the existing global 24-hour token to five minutes would be unsafe for the current VOD playlist/client contract. That does not rule out renewable stream capabilities as a later design.

Material Spec/correctness findings

1. High — overlapping edge requests can hide a live stream. Proxy direct/remux requests independently Track and unconditionally Remove the same SID (internal/proxy/server.go:182-199,274-300). Tracker state is a set rather than a refcount (internal/nodesessions/tracker.go:80-84), so the first Range/GET to finish deletes the whole record and Redis key while another request continues. Later bytes are discarded (tracker.go:255-307). This directly defeats authoritative monitoring.

2. High — every ABS in-flight cutoff is a production no-op. All ABS routes pass through accessLog (internal/audiobooks/abs/handler.go:334-338), whose statusRecorder lacks Unwrap (access_log.go:84-121). http.ResponseController.SetWriteDeadline therefore cannot reach the socket. The cutoff tests call handlers directly and bypass that middleware (revocation_test.go:183-203).

3. High — over-cap kills do not stay dead. The enforcer writes a five-minute session revocation (internal/streamenforcer/enforcer.go:26-32,119-120), while the token remains valid for 24 hours (internal/playback/recipecard.go:180-186). Once refusal makes the stream disappear from monitoring, it is no longer re-revoked; the same token can reconnect after five minutes. This directly fails Protect against stream abuse: authoritative monitoring + kill switch for cap violations, admin terminate, and re-streaming #305's “same still-valid token is refused” and “stay dead” criteria.

4. High — integrated liveness still trusts client progress. UpdateProgress advances LastActivityAt; stale cleanup uses that field; and local monitoring falls back to it when LastServedAt is zero (internal/playback/session.go:732-746,1324-1395; internal/api/handlers/nodes.go:38-41). False progress can preserve a no-byte phantom session indefinitely.

5. High — user-cutoff credential time is unstable on compat/native fallback paths. Jellycompat repeatedly supplies fresh time.Now() values and native fallback uses request-entry time (internal/jellycompat/streams.go:92-119,155-160,469-473; internal/api/router.go:3536-3543). A later request from the same pre-cutoff login can therefore appear post-cutoff. There is also a Jellycompat lookup race that can sign an ownerless uid=0 edge token (internal/jellycompat/handlers_playback.go:355-399), which user revocation and the enforcer skip.

6. High — distributed unrevoke can resurrect. The unrevoke tombstone exists only in one process (internal/streamrevoke/store.go:455-489). A replica that misses pub/sub retains the kill; Redis reconciliation never removes locally-held entries absent from Redis (store.go:733-761); maintenance can then recreate the deleted PostgreSQL row and re-arm Redis (store.go:658-697).

7. High — PostgreSQL can indefinitely block the urgent Redis kill. Revoke strips caller cancellation, holds the global operation mutex, and performs the durable upsert before Redis/pub-sub, without a replacement deadline (internal/streamrevoke/store.go:312-365; internal/database/postgres.go:14-34). A stalled DB therefore prevents edge enforcement despite healthy Redis.

8. High — asynchronous transcode tracking can create a permanent ghost. Start/reconstruct launches an untracked go tracker.Track(...); if stop wins first, the delayed Track creates an explicit record after cleanup (internal/transcodenode/server.go:599-619,783-799,826-867).

9. High — protocol-v3 remote transcodes can count one stream twice. The node uses a generation-scoped transport ID while monitoring deduplicates exact IDs only (internal/api/handlers/playback_v3.go:761-769; internal/streammonitor/monitor.go:174-195). Once owner attribution is populated, the logical and transport records can both count and trigger a false cap kill.

10. High — transcode-node-without-proxy is supported but unmetered. TranscodeNode and ProxyNode are independently optional (internal/nodepool/planner.go:206-299). In the no-proxy topology, the API proxies segments without a SessionMeteredWriter or transport span (internal/api/handlers/playback.go:3556-3618,3885-3895), while the node assumes a fronting proxy measures bytes.

11. High policy gap — an actual ABS streaming route is cap-exempt. The authenticated bare file route is explicitly used for iOS streaming (internal/audiobooks/abs/file_handler.go:37-44) but is classified as download traffic (file_handler.go:59-63). One credential can open many streaming Range requests without consuming stream slots.

12. Medium-high — closing an RSS feed does not cut its current pour. Close only writes closed_at (internal/audiobooks/abs/rss_feeds_handler.go:114-140; internal/audiobooks/abs_rss_feed_store.go:103-107). The active watcher observes user/stream revocation, not feed closure (rss_feeds_handler.go:268-270). An indefinite feed URL can also become usable again after a temporary user cutoff expires unless the feed is closed/rotated or current entitlement is rechecked.

13. High — a longer old user revocation can suppress a newer security cutoff. The entire record associated with the later expiry wins (internal/streamrevoke/store.go:260-280; durable_postgres.go:51-65), including its older RevokedAt. Credentials issued between the old and new cutoffs remain allowed. For user cutoffs, RevokedAt and ExpiresAt must merge independently, or be replaced with an authorization generation.

14. High — multiple API enforcers are not merely redundant/idempotent. Integrated replicas remain mutually blind, and different replicas can select different victims from moving, non-atomic snapshots, over-killing below the allowance (cmd/silo/main.go:1750-1766; internal/streammonitor/monitor.go:301; internal/streamenforcer/enforcer.go:99-144).

15. Medium — monitoring is not uniformly asynchronous. Edge Track, and the first HLS Touch, synchronously write Redis before serving (internal/nodesessions/tracker.go:176-240). Native guards also parse/verify the signed token, so the added request-path work is more than a pure memory set lookup.

16. Medium — subtitles are not fully monitored or hard-killable in flight. Native, proxy, and Jellycompat subtitle routes perform entry-time refusal only, with no transport span, byte meter, or in-flight watcher (internal/api/router.go:2543-2544; internal/proxy/server.go:371-469; internal/jellycompat/streams.go:669-820). Large PGS tracks make this a real byte-serving path.

17. Medium — the motivating re-streaming heuristic does not exist. The PR's own matrix correctly concedes that one Silo stream re-broadcast to many external viewers and token-hoarding/fan-out remain undetected (docs/architecture/stream-abuse-matrix.md:54-58,99-105,382-383). Multiple upstream streams are caught only as an ordinary over-count.

Additional qualification: ordinary logout and individual device-session deletion do not invoke the user-stream revocation hook (internal/api/handlers/auth.go:261,419); only the bulk account/security-change path does. “Account-session revocation kills live streams” is therefore too broad.

Author-narrative corrections

• A stream token has a 24-hour absolute lifetime, not an unlimited lifetime. An already-open progressing GET may continue past expiry, which still justifies server-side cancellation.
• Base admin termination already had a server fallback that removed sessions and stopped transcodes; the real missing pieces were cutting active direct/remux sockets and making termination sticky against reconstruction.
• Integrated sessions were absent from the Redis node-session endpoint, but the web Admin Activity page already used a PostgreSQL-backed session endpoint.
• Integrated streams did not meaningfully report literal zero-byte counters; the per-session byte fields did not yet exist.
• Native managed downloads already had durable rows; what was missing was unified live in-flight visibility.
• Public ABS track playback already used an admitted native session.
• “Redis flush resurrected base kills” describes an intermediate design: base had no Redis kill list, and the retained token could reconstruct without requiring a flush or restart.

Standards findings

1. High: the mandatory AI disclosure names only “Claude,” omitting tool, exact model ID, involvement classification, and adversarial findings/resolution required by CONTRIBUTING.md:39 and docs/ai-contributions.md:6-18.
2. High: verification is paraphrased rather than raw output; required frontend checks and real local/runtime evidence were not supplied (docs/ai-contributions.md:20-34, CONTRIBUTING.md:30).
3. Medium: the new v1 revocation endpoints and transfers response lack capability advertisement, contrary to the repository's additive-v1 rule.
4. Medium: Session-to-monitor conversion lives in the HTTP handlers package but is consumed by the background enforcer; it belongs in the domain package that owns monitoring/session behavior.
5. Low smell: (sessionID, userID, startedAt) is a repeated data clump and should become a credential/revocation-subject type.

Recommended architecture

Keep the current primitives, but make the next version hybrid:

• Use atomic Redis admission leases/counters for the hard fleet-wide concurrency limit. Async monitoring should provide visibility and reconciliation, not be the primary admission correctness mechanism.
• Track active HTTP pours using per-SID refcounts or generations, with a cancellable connection registry.
• Publish integrated sessions from every replica and run a single elected/serialized enforcer.
• Store revocation and unrevoke as a versioned desired-state ledger with durable tombstones; project to Redis with revision-aware compare-and-set.
• Give external operations bounded deadlines, and do not let a stalled PostgreSQL write indefinitely delay urgent Redis propagation.
• Use a stable credential/login creation time or user authorization generation everywhere. Separate “terminate current streams” from a standing “block future streaming” policy.
• Make socket cancellation latched and test it through the real middleware chain with a real HTTP connection.
• Treat the ABS bare file route as playback when used for playback.
• Longer-term, consider short-lived renewable stream capabilities tied to SID. Short TTL alone is insufficient because it cannot terminate an already-open response.
• Treat re-stream detection as a separate heuristic/risk problem; concurrency alone cannot reveal one upstream connection relayed to many downstream viewers.

Independent validation

Passed at the pinned head:

• go test -race ./internal/streamrevoke ./internal/nodesessions ./internal/streamenforcer ./internal/streammonitor
• Focused proxy, transcode-node, transfer-registry, ABS, monitoring, and enforcer tests
• Targeted go vet on changed core packages
• changed-file gofmt check
• git diff --check
• make verify-local-paths

The broad suite is non-green, but the principal failures were reproduced on the exact base: stale UserStore test doubles missing GetOnboardingState, TestHandleReplanPlaybackV3SeekFailureRecoveryNeverChangesMediaVersion, and TestServeDirectPlayChangedEntityRejectsOldIfRange. They were therefore not attributed to this PR.

The central runtime claims still lack real-socket, N+1/no-progress, two-central-replica, missed-pub/sub, Redis-flush deletion-reconciliation, stalled-PostgreSQL, same-SID overlap, Track-vs-Stop, logical-vs-transport-ID, and no-proxy-transcode tests.

Bottom line: retain the server-owned monitoring and deny-list direction, but address the high-confidence lifecycle, revocation, and distributed-state defects before representing this as authoritative monitoring and a kill switch that keeps streams dead.

[CoffeeKnyte]

Thanks — this review set the whole work plan. Nearly all of it was confirmed by reading the code and is fixed; where I disagreed or deliberately didn't fix something, I've said so rather than quietly closing it.

Head is now d01b5de3 (was 09de4f08 when you reviewed). Work landed as six dependency-ordered batches.

Material Spec/correctness findings

1. Fixed — ecb4555e. Edge tracking is now lease/epoch refcounted, so a finishing Range request can't delete a record another request is still using.
2. Fixed — e35efdda + 7adedf64, and c24d8396 for the related latch. Unwrap() added; the replacement test drives the mounted router over a real socket (abs/revocation_test.go:32) instead of calling the handler directly. Worth flagging: the same Unwrap() that makes SetWriteDeadline work also let the rolling deadline writer re-arm the socket and erase the cut, so fixing this alone would have been inert — hence the separate latch commit.
3. Fixed — e5bf0155. Over-cap revocations now match the token's full reconstructable lifetime and are non-sliding (RevokeSessionForIfAbsent), so a repeat observation can't push a false positive's expiry forward forever.
4. Fixed — fc65dcaf. The LastActivityAt fallback is gone from the projection and from reaping. One deliberate exception: a paused session holding an open, ping-checked WebSocket stays exempt, because session_paused_grace_test.go encodes issue Playback freezes / won't resume on player state changes (resume after long pause, intro-skip, manual seek) #243 (reaping a paused transcode froze clients). An observed connection isn't a client-reported position, so this stays within the decision.
5. Fixed — e5bf0155. Credential time is now the token iat; jellycompat uses the compat-session CreatedAt, not the refreshed bridged token. The ownerless uid=0 race is closed too.
6. Fixed — e5bf0155. Durable tombstones (two nullable columns on stream_revocations, not a second table); a stale replica can no longer re-Upsert an explicitly lifted kill.
7. Fixed — e5bf0155. Redis and pub/sub now run before the durable mirror, and detached propagation uses bounded contexts, so a stalled PostgreSQL can't hold up the urgent edge kill.
8. Fixed — ecb4555e. The untracked go tracker.Track(...) is gone; ordering is explicit so a delayed Track can't resurrect a record after stop.
9. Fixed — ecb4555e. Monitoring now dedupes on canonical logical identity, so a logical and transport record can't both count.
10. Fixed — 7adedf64. The no-proxy path meters and holds a transport span (TestProxyToTranscodeNodeMetersOnlyMediaBodiesAndFlushesBeforeTransportEnd).
11. Won't fix as specified — deliberate. Decision A4: observe + make killable, keep cap-exempt. The route is now metered, transfer-rowed, refused on entry and cut in flight, but it doesn't consume a video stream slot. Making it count would also make ebook/comic/PDF reading count. Recorded as a settled decision, so it's reversible if you disagree.
12. Not fixed — deferred, open. Your "~40 lines" read was optimistic: the public feed pour passes an empty session key to both Refuse and WatchAndCut, so it needs a deterministic namespaced revocation id that can't collide with real session ids, threaded onto public feed requests, plus a decision on how close reports partial propagation failure. Left open rather than half-done.
13. Fixed — e5bf0155. RevokedAt and ExpiresAt now merge as independent monotonic dimensions, so a longer old kill can't suppress a newer cutoff.
14. Fixed for monitoring, not admission — 12f5d5f8. Integrated sessions are published to the shared silo:sessions: picture and a renewable Redis lease elects one evaluator per tick. Synchronous admission is still per-process, so cross-replica excess is trimmed within ~120s rather than refused at the door; I scored the matrix row that way instead of calling it clean. Same commit also closes the mirror race you'd flagged in the architecture section — see below.
15. Claim corrected, queue deferred. You were right; rather than rush it I fixed the documentation (first write per session is synchronous, later updates ride the refresh tick). A queue needs its whole lifecycle designed together — ordered keyed coalescing buffer, drain ordering against Remove/Release, tracker-owned contexts — and a naive fire-and-forget projection is exactly what caused finding 8.
16. Fixed — 7adedf64, with two stated bounds: proxy subtitle bytes are attributed only when a tracker record already exists, and compat subtitle extraction is buffered, so a cut stops delivery but not extraction already in progress.
17. Not built — filed as playback: re-stream detection (C16 fan-out) — the fingerprints are not consumable as-is #522, and your framing was too generous to it. Scoping it showed the premise was false: ClientIP is one value per session, not a set — integrated mode stamps it at session creation and no media request updates it, and the edge tracker overwrites the record per request. So no consumer of the existing snapshot can see fan-out at all; it needs per-request observation at media-serve time. Also worth stating plainly: an IP signal can only catch shared session-URL fan-out. Your C14 — one stream relayed to many downstream viewers — is invisible by construction, since we see exactly one address. It stays scored ❌/❌.

Logout qualification: correct, and unchanged. Per-login cuts need per-login identity in the stream credential (the authorization-generation model), not the iat model shipped. A user-wide cutoff would wrongly kill that user's other devices, so it's recorded as a new decision rather than implemented.

Author-narrative corrections

All accepted and folded into docs/architecture/stream-abuse-matrix.md rather than argued with. The 24h-not-unlimited token, the pre-existing admin-terminate fallback, the PostgreSQL-backed Admin Activity page, and the "Redis flush resurrected base kills" mischaracterisation are all corrected in the doc.

Standards findings

1. Fixed — PR body now has tool, exact model IDs (claude-opus-5, gpt-5.6-sol), involvement classification, and the adversarial findings/resolution including claims rejected on verification.
2. Fixed — raw command output, not paraphrase. Two gaps stated rather than papered over: pnpm is absent on this host (no frontend files changed), and this repo runs no go test job in CI at all — .github/workflows/ has only build/labeler/bot jobs. That's also why the internal/access/jellycompat compile break went unnoticed.
3. Fixed — 70064823, plus max_user_concurrent_transfers / transfer_monitoring_fail_closed on GET /downloads/capability.
4. Fixed — ecb4555e. The conversion moved to internal/streammonitor, so the enforcer and admin view share one definition.
5. Won't fix. Now that the credential model is token iat rather than the generation model, the clump is ~90 mechanical lines across the hot path for no behavioural gain. Reconsider if we ever adopt authorization generations.

Architecture recommendations

Adopted: per-SID refcounts/generations; publish integrated sessions from every replica; single elected enforcer; durable tombstones; bounded deadlines on external operations; latched socket cancellation tested through the real middleware chain.

Revision-aware CAS — adopted, and thank you for it. mirrorToRedis was an unconditional SET, so two replicas revoking the same key could lose the stronger kill, and edges learn kills only from Redis. It now merges server-side in Lua with the same monotonic semantics as applyLocal. Two traps worth recording: the comparison uses exact (unix_sec, nsec) pairs, because RFC3339 can't be compared lexicographically (Go omits trailing zeros, so …:00Z sorts after …:00.5Z) and millisecond truncation could retain an older cutoff; and the script merges before deciding to delete, since deleting on a lapsed incoming revocation could remove a live permanent kill written by another replica.

Not adopted: atomic Redis admission leases — admission remains per-process; and authorization generation, where the iat model was chosen instead. Both are recorded as settled decisions with their consequences, not as oversights.

Missing runtime tests

Most now exist and were verified non-vacuous by reverting each fix and confirming failure: real-socket via the mounted router, no-progress phantom, same-SID overlap, Track-vs-Stop, logical-vs-transport-ID, no-proxy transcode metering, missed-pub/sub, and stalled-PostgreSQL (TestBlockedDurableDoesNotDelayRedisAndIsBounded). Cross-replica revocation convergence is covered against a real Redis 8.6.2 — the repo's existing Redis doubles are hand-written ProcessHook fakes that can't execute Lua, so a fake would have proven nothing there. Those are gated on SILO_TEST_REDIS_ADDR and skip elsewhere, which given the no-CI point above means they're local-only until a Redis service is added.

Two of your three reproduced baseline failures still fail on main unchanged; internal/access/jellycompat still don't compile, so compat changes here are verified by reading only. Not attributed to this PR, but it needs its own fix.

Still 8 behind origin/main — not rebased yet, as you noted.

[CoffeeKnyte]

Closing in favour of #667, which lands the monitoring half of this work on its own.

Why the split. This branch built enforcement and monitoring together, and that was the
wrong order. Every threshold in the enforcement design — what delivery rate counts as a rip,
when a session is over-consuming, who gets cut — was chosen before anything had measured a
byte of real traffic. They were guesses with numbers attached.

#667 makes monitoring first-class and stops there: it observes only, decides nothing, and
repoints no existing admin read. It has now run for 18 hours on the live server, and the
value of doing it in this order showed up immediately — the parity projection found that the
view we currently trust counts a progress update as proof of playback, so sessions that
stopped receiving video hours ago were still being counted as active, holding transcode slots
(#666). That is exactly the kind of thing an enforcement rule built on the old numbers would
have acted on incorrectly.

Enforcement comes next, designed against the distributions #667 produces rather than against
assumptions. The design work here is not lost — it is preserved verbatim in the appendix of
the telemetry design doc (docs/design/2026-08-17-stream-telemetry-appendix.md, section E),
along with the prior-art trail from this branch and the reasoning recorded during review.

Nothing here is being discarded as wrong; it is being resequenced so the numbers come first.