[api] fix ban duration display

CLAUDE.md

@@ -269,6 +269,15 @@ inconsistency here without checking the client first.
   every non-avatar drop with `AvatarItemType: null` for this reason; `boxAvatarItemType` applies
   it on every surface, so don't coalesce it with `?? 0` anywhere a box is built. This holds for
   every worker that mints a box, not only `econ`.
+- A ban's `Duration` (`api`: `/api/PlayerReporting/v1/moderationBlockDetails`, and the
+  `ModerationKick` frame `www`'s staff ban sends) is the seconds LEFT as of the response — the
+  client counts it down from receipt. It is NOT the ban's full span paired with
+  `TimeoutStartedAt` (start + duration): served that way, the screen read correctly only in
+  the kick frame, sent at the instant the span and the remainder coincide, and every later
+  sign-in showed the whole ban still to run. `TimeoutStartedAt` stays null for a ban; where
+  other implementations set it at all, it names the start of a voice-chat timeout. Other
+  implementations with timed bans keep their set-time private and serve the remainder on
+  every read; `banned_at` here is the staff panel's audit trail, not the client's.
 - Accessibility is sent as the `RoomAccessibility` enum NAME on
   `rooms` `PUT /rooms/:id/subrooms/:sid/accessibility` (`accessibility=Private`), not the
   ordinal the room-level `/rooms/:id/accessibility` takes. The enum has five members

apps/api/src/openapi.ts

@@ -1370,8 +1370,9 @@ export const VoteToKickReason = z.object({
  * `GET|POST /api/PlayerReporting/v1/moderationBlockDetails` — the caller's block. With an
  * account-wide ban in force (a `report` row with `banned` set) it describes that ban:
  * `IsBan` true, the report's `ReportCategory`, a fixed `Message` of "Rule violation", and
- * its span as `TimeoutStartedAt` (the report's `created_at`) plus `Duration` (seconds to
- * `ban_expires`; int32 max for a permanent ban). Otherwise it is the "not blocked" answer, mirroring the reference server's stub `ReturnModerationBlockDetails()`:
+ * `Duration` as the seconds LEFT as of the request (`ban_expires` minus now; int32 max for
+ * a permanent ban) — the client counts it down from receipt, so it is never the full span.
+ * `TimeoutStartedAt` stays null for a ban. Otherwise it is the "not blocked" answer, mirroring the reference server's stub `ReturnModerationBlockDetails()`:
  * `ReportCategory` is `Unknown` (-1) rather than 0, which is a real category, and
  * `Message` is null — the client distinguishes "no message" from a blank one, so we send
  * null where the reference sends an empty string. `IsVoiceModAutoban`/`TimeoutStartedAt`
@@ -1391,7 +1392,7 @@ export const ModerationBlockDetails = z.object({
 	Duration: z
 		.int()
 		.describe(
-			'Length of the block in seconds from `TimeoutStartedAt`; 2147483647 (int32 max) for a permanent ban; 0 when not blocked'
+			'Seconds LEFT on the block as of this request (the client counts it down from receipt); 2147483647 (int32 max) for a permanent ban; 0 when not blocked'
 		),
 	GameSessionId: z.int(),
 	IsHostKick: z.boolean().describe('Always false — no host kick is ever recorded here'),
@@ -1411,7 +1412,7 @@ export const ModerationBlockDetails = z.object({
 		.string()
 		.nullable()
 		.describe(
-			'When the block began — the ban’s report `created_at` (ISO-8601 UTC); `Duration` runs from it. Null when not blocked'
+			'Always null — the client does not pair it with `Duration` for a ban (it names the start of a voice-chat timeout, which this server never hands out)'
 		),
 	AssociatedAccountUsername: z.string().nullable().describe('Always null'),
 	ShowCreatorCodeOfConduct: z.boolean().describe('Always false'),

apps/api/src/reports-db.ts

@@ -125,9 +125,10 @@ export interface ReportRow {
 	banned_by_player_id: number | null
 	/**
 	 * ISO-8601 UTC instant the ban was HANDED DOWN — distinct from `created_at`, which is
-	 * when the report was filed, and the two can be months apart. This is what the client's
-	 * block screen counts the ban from (see `banBlockDetails`); rows written before
-	 * 0020_report_ban_audit.sql carry NULL and fall back to `created_at`.
+	 * when the report was filed, and the two can be months apart. The staff panel's audit
+	 * trail and its ordering (`getBansInForce`); the client's block screen does NOT count
+	 * from it (see `banBlockDetails`). Rows written before 0020_report_ban_audit.sql carry
+	 * NULL and fall back to `created_at` where a date is needed.
 	 */
 	banned_at: string | null
 	/**
@@ -271,8 +272,8 @@ export async function isPlayerBanned(
  * report itself intact.
  *
  * `bannedBy` is the acting moderator, and is recorded alongside `banned_at` — the instant
- * the ban was handed down, which is NOT the report's `created_at` and is what the client's
- * block screen counts from. A lift clears all four columns together: an unbanned row must
+ * the ban was handed down, which is NOT the report's `created_at`; the staff panel shows
+ * and sorts by it. A lift clears all four columns together: an unbanned row must
  * not keep an audit trail saying a ban runs from somewhere, and `banned = 0` with a
  * `banned_at` still set would read as a ban to anything checking the timestamp.
  *
@@ -507,9 +508,9 @@ export async function getBansInForce(
 // ---- Block details ----------------------------------------------------------
 
 /**
- * `Duration` on a permanent ban. The client's field is a 32-bit int of seconds that PAIRS
- * with `TimeoutStartedAt` — start + duration is the end of the block — so a ban with no
- * end gets the largest value the field holds, 68 years past its start.
+ * `Duration` on a permanent ban. The client's field is a 32-bit int of seconds LEFT, so a
+ * ban with no end gets the largest value the field holds — 68 years, which other
+ * implementations also send and the client's screen renders as permanent.
  */
 export const PERMANENT_BAN_DURATION = 2_147_483_647
 
@@ -547,18 +548,24 @@ export const NOT_BLOCKED = {
  * (the screen at sign-in) and `www`'s live `ModerationKick` frame (the screen when the ban is
  * handed down mid-session). Built once here so both screens describe the same ban.
  *
- * `Duration` and `TimeoutStartedAt` are a PAIR in the client: the block runs from the
- * start for the duration. The start is `banned_at`, the instant the ban was handed down
- * (see 0020_report_ban_audit.sql), and the duration is the seconds from there to
- * `ban_expires`, so the two sum to the expiry; or `PERMANENT_BAN_DURATION` when there is
- * none.
+ * `Duration` is the seconds LEFT on the ban as of `now` — the time from the request to
+ * `ban_expires`, rounded up so a ban with a second to run never reads 0 — or
+ * `PERMANENT_BAN_DURATION` when there is no expiry. The client counts it down from the
+ * moment it receives it, with no reference to when the ban began: other implementations
+ * with timed bans keep the instant they set the block PRIVATE and serve `Duration` minus
+ * the elapsed time on every read.
+ * This once sent the FULL span from `banned_at` instead, on the theory that the client
+ * pairs it with `TimeoutStartedAt` (start + duration = end). It does not: the screen read
+ * correctly only at the instant the ban was handed down — when the full span IS the
+ * remainder, so the mid-session `ModerationKick` looked right — and every later sign-in
+ * showed the whole ban still to run.
  *
- * It falls back to the report's `created_at` for a row banned before that column existed.
- * The two can be months apart, and using `created_at` as the start — which is what this
- * did before there was anything else to use — misreports both halves of the pair: a 7-day
- * ban applied to a 30-day-old report told the player their block began a month ago and
- * ended three weeks ago. Every pre-migration row still reads exactly as it used to, which
- * is the point of the fallback rather than a coalesce to now.
+ * `TimeoutStartedAt` stays null. Other implementations don't send it for a ban; where it is
+ * set at all it names the start of a voice-chat timeout, which is what the name is about.
+ * `banned_at` is the audit trail and the staff panel's, not the client's.
+ *
+ * `now` is a parameter so the caller can pass the same instant it resolved the ban with
+ * (`getActiveBan`), and a ban that was in force at the read can't come out as 0 left.
  *
  * The category is the one the report was
  * filed under, so the client's ban screen names the reason. `Message` is a fixed "Rule
@@ -568,19 +575,17 @@ export const NOT_BLOCKED = {
  * reported them. Everything else keeps its `NOT_BLOCKED` value: the other block kinds and
  * screen dressings, none of which this server hands out.
  */
-export function banBlockDetails(ban: ReportRow) {
-	const startedAtIso = ban.banned_at ?? ban.created_at
-	const startedAt = Date.parse(startedAtIso)
+export function banBlockDetails(ban: ReportRow, now: Date = new Date()) {
 	const duration =
 		ban.ban_expires === null
 			? PERMANENT_BAN_DURATION
-			: Math.max(1, Math.ceil((Date.parse(ban.ban_expires) - startedAt) / 1000))
+			: Math.max(1, Math.ceil((Date.parse(ban.ban_expires) - now.getTime()) / 1000))
 	return {
 		...NOT_BLOCKED,
 		ReportCategory: ban.report_category,
 		Duration: duration,
 		IsBan: true,
 		Message: 'Rule violation',
-		TimeoutStartedAt: startedAtIso,
+		TimeoutStartedAt: null,
 	}
 }

apps/api/src/routes/moderation.ts

@@ -368,12 +368,11 @@ export const moderationRoutes = new Hono<App>({ strict: false })
 				'matchmake refuses on (login still issues a token, so the client can reach this ' +
 				'screen) — so a caller with one in force gets ' +
 				'`IsBan: true`, the `ReportCategory` the report was filed under, the fixed ' +
-				'`Message` “Rule violation”, and the block’s span as the pair the client reads ' +
-				'them as: `TimeoutStartedAt` is when the ban was handed down (`banned_at`, ' +
-				'falling back to the report’s `created_at` for a ban set before that column ' +
-				'existed) and `Duration` the ' +
-				'seconds from there to `ban_expires` (2147483647, the int32 max, for a permanent ' +
-				'ban). ' +
+				'`Message` “Rule violation”, and `Duration` as the seconds LEFT on the ban as of ' +
+				'this request — the client counts it down from when it receives it, so it is ' +
+				'`ban_expires` minus now, rounded up, never the ban’s full span (2147483647, the ' +
+				'int32 max, for a permanent ban). `TimeoutStartedAt` stays null: the client does ' +
+				'not pair it with `Duration` for a ban, and no reference server sends it. ' +
 				'`PlayerIdReporter` stays null: it names a kicking host, and the reporter is not ' +
 				'shown to the player they reported. Only the caller’s own account is consulted, ' +
 				'not the ban-evasion arms.\n\n' +
@@ -396,8 +395,11 @@ export const moderationRoutes = new Hono<App>({ strict: false })
 		async (c) => {
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
-			const ban = await getActiveBan(c.env.DB, id)
-			return c.json(ban ? banBlockDetails(ban) : NOT_BLOCKED)
+			// One instant for both: the ban resolved as in force at `now` is described as of
+			// `now`, so it can't come out with 0 left.
+			const now = new Date()
+			const ban = await getActiveBan(c.env.DB, id, now)
+			return c.json(ban ? banBlockDetails(ban, now) : NOT_BLOCKED)
 		}
 	)
 	// The reasons the client offers when a player starts a vote-to-kick. Order matters —

apps/api/src/test/integration/api.test.ts

@@ -6245,17 +6245,16 @@ describe('player reports', () => {
 			expect(res.status).toBe(401)
 		})
 
-		// Duration and TimeoutStartedAt are a pair in the client — the block runs from the
-		// start for the duration. The start is `banned_at`, the instant the ban was handed
-		// down (see 0020_report_ban_audit.sql), NOT the report's created_at: the two can be
-		// months apart. A permanent ban runs for the largest span the int holds.
+		// `Duration` is the seconds LEFT — the client counts it down from receipt. A permanent
+		// ban runs for the largest span the int holds, and `TimeoutStartedAt` stays null: the
+		// client does not pair it with `Duration` for a ban.
 		test('describes a permanent ban', async () => {
 			await submit(
 				{ PlayerIdReported: '221', ReportCategory: '102', Details: 'slurs' },
 				await bearer()
 			)
 			const [row] = await getReportsAgainst(env.DB, 221)
-			const banned = await banFromReport(env.DB, row!.id)
+			await banFromReport(env.DB, row!.id)
 
 			expect(await details('POST', '221')).toEqual({
 				...NOT_BLOCKED,
@@ -6266,21 +6265,15 @@ describe('player reports', () => {
 				// the reporter is not shown to the player they reported (PlayerIdReporter
 				// stays null).
 				Message: 'Rule violation',
-				TimeoutStartedAt: banned!.banned_at,
 			})
 		})
 
-		// A timed ban's Duration is the seconds from the start to the expiry, so the pair
-		// sums to `ban_expires` — not the seconds left as of the request.
-		test('describes a timed ban as its banned_at plus the span to expiry', async () => {
+		// A fresh timed ban: the seconds left are (within the write's latency) the whole ban.
+		test('describes a timed ban as the seconds left as of the request', async () => {
 			await submit({ PlayerIdReported: '222', ReportCategory: '103' }, await bearer())
 			const [row] = await getReportsAgainst(env.DB, 222)
-			// The expiry is set relative to NOW, which is what a moderator handing down a
-			// one-hour ban means — and now is `banned_at`, not the report's created_at.
 			const banExpires = new Date(Date.now() + 3600 * 1000)
-			const banned = await banFromReport(env.DB, row!.id, {
-				banExpires: banExpires.toISOString(),
-			})
+			await banFromReport(env.DB, row!.id, { banExpires: banExpires.toISOString() })
 
 			// `Duration` is asserted as a range below, so it is left out of the object match
 			// — NOT_BLOCKED carries a 0 for it, which would win over the real value.
@@ -6291,59 +6284,59 @@ describe('player reports', () => {
 				ReportCategory: 103,
 				IsBan: true,
 				Message: 'Rule violation',
-				TimeoutStartedAt: banned!.banned_at,
+				TimeoutStartedAt: null,
 			})
-			// Within a second of the hour: `banned_at` is stamped by the write, so the span
-			// is the hour minus however long the write took.
+			// Within a second of the hour: however long the write took has already elapsed.
 			expect(body.Duration).toBeGreaterThan(3595)
 			expect(body.Duration).toBeLessThanOrEqual(3600)
 		})
 
-		// The whole point of `banned_at`: a ban applied to an OLD report used to be
-		// described as having started when the report was filed, so a 7-day ban on a
-		// month-old report told the player their block began a month ago — and the duration,
-		// measured from there, came out as already served. Both halves of the pair now
-		// start from the ban.
-		test('counts a ban from when it was handed down, not from when the report was filed', async () => {
+		// The case the bug lived in: a player signing in AFTER the ban was handed down. The
+		// client counts `Duration` down from the moment it receives it, so it must be what is
+		// left NOW — not the ban's full span from `banned_at`, which read as the whole ban
+		// still to run at every sign-in (and only looked right in the mid-session kick frame,
+		// sent at the instant the span and the remainder are the same).
+		test('counts down: a ban handed down a day ago has a day less to run', async () => {
 			await submit({ PlayerIdReported: '224', ReportCategory: '102' }, await bearer())
 			const [row] = await getReportsAgainst(env.DB, 224)
-			// Backdate the report a month, as if it had sat in the queue.
-			const filed = new Date(Date.now() - 30 * 86_400_000).toISOString()
-			await env.DB.prepare('UPDATE report SET created_at = ?2 WHERE id = ?1')
-				.bind(row!.id, filed)
+			// A seven-day ban handed down yesterday: six days left.
+			const banned = await banFromReport(env.DB, row!.id, {
+				banExpires: new Date(Date.now() + 6 * 86_400_000).toISOString(),
+			})
+			await env.DB.prepare('UPDATE report SET banned_at = ?2 WHERE id = ?1')
+				.bind(row!.id, new Date(Date.now() - 86_400_000).toISOString())
 				.run()
 
-			const banned = await banFromReport(env.DB, row!.id, {
-				banExpires: new Date(Date.now() + 7 * 86_400_000).toISOString(),
-			})
 			const body = (await details('GET', '224')) as Record<string, unknown>
-			expect(body.TimeoutStartedAt).toBe(banned!.banned_at)
-			expect(body.TimeoutStartedAt).not.toBe(filed)
-			// Seven days, not the negative-then-clamped span the report's date would give.
-			expect(body.Duration).toBeGreaterThan(7 * 86_400 - 5)
-			expect(body.Duration).toBeLessThanOrEqual(7 * 86_400)
+			expect(body.IsBan).toBe(true)
+			expect(body.Duration).toBeGreaterThan(6 * 86_400 - 5)
+			expect(body.Duration).toBeLessThanOrEqual(6 * 86_400)
+			// The start is not on the wire at all: null, not `banned_at`.
+			expect(body.TimeoutStartedAt).toBeNull()
+			expect(banned!.banned_at).not.toBeNull()
 		})
 
-		// A row banned BEFORE 0020 added the column carries no `banned_at`, and must keep
-		// reading exactly as it used to rather than falling back to "now" — which would
-		// silently restart every standing ban the moment this shipped.
-		test('falls back to created_at for a ban with no recorded banned_at', async () => {
+		// A row banned BEFORE 0020 added `banned_at` reads the same as any other: the
+		// remainder needs only `ban_expires`.
+		test('reads a ban with no recorded banned_at the same way', async () => {
 			await submit({ PlayerIdReported: '225', ReportCategory: '101' }, await bearer())
 			const [row] = await getReportsAgainst(env.DB, 225)
 			await env.DB.prepare(
 				`UPDATE report SET banned = 1, ban_expires = ?2, banned_at = NULL WHERE id = ?1`
 			)
-				.bind(row!.id, new Date(Date.parse(row!.created_at) + 3600 * 1000).toISOString())
+				.bind(row!.id, new Date(Date.now() + 3600 * 1000).toISOString())
 				.run()
 
-			expect(await details('GET', '225')).toEqual({
-				...NOT_BLOCKED,
+			const { Duration: _duration, ...unblocked } = NOT_BLOCKED
+			const body = (await details('GET', '225')) as Record<string, unknown>
+			expect(body).toMatchObject({
+				...unblocked,
 				ReportCategory: 101,
-				Duration: 3600,
 				IsBan: true,
 				Message: 'Rule violation',
-				TimeoutStartedAt: row!.created_at,
 			})
+			expect(body.Duration).toBeGreaterThan(3595)
+			expect(body.Duration).toBeLessThanOrEqual(3600)
 		})
 
 		// A ban that has served its time is not a block, even though the row still says

apps/www/src/staff.ts

@@ -216,10 +216,11 @@ function banExpiry(
  * ROOM ban must never send it (`rooms` sends `IsBan: false` and is enforced by refusing the
  * room), because the client cannot tell the two apart and shows a game-wide ban screen.
  *
- * Its contents are {@link banBlockDetails} — the same `ReportCategory`, `Message` and the
- * `Duration`/`TimeoutStartedAt` pair `moderationBlockDetails` answers for this ban — so the
- * screen a player sees mid-session is the screen they see when they next sign in, not a
- * generic "banned" with no length.
+ * Its contents are {@link banBlockDetails} — the same `ReportCategory`, `Message` and
+ * `Duration` `moderationBlockDetails` answers for this ban — so the screen a player sees
+ * mid-session is the screen they see when they next sign in, not a generic "banned" with no
+ * length. `Duration` is the seconds LEFT, and this frame goes out the instant the ban is
+ * handed down, so here that is the ban's whole span; every later sign-in gets less.
  *
  * Sent to anyone ONLINE, not only to a player standing in an instance: someone in a menu is
  * as banned as someone in a room. `GameSessionId` is their instance, or 0 when they are in

apps/www/src/test/integration/api.test.ts

@@ -1280,14 +1280,16 @@ it('throws a banned player out of the instance they are standing in', async () =
 	expect(frames[0].data.GameSessionId).toBe(77001)
 
 	// The SAME ban the sign-in screen describes, not a generic one: its category, message and
-	// the Duration/TimeoutStartedAt pair, three days from when it was handed down.
+	// `Duration` — the seconds LEFT, which at the instant the ban is handed down is the whole
+	// three days. `TimeoutStartedAt` is not paired with it by the client and stays null.
 	const banned = await getReportById(env.DB, report.id)
 	expect(frames[0].data).toMatchObject({
 		ReportCategory: banned!.report_category,
 		Message: 'Rule violation',
-		TimeoutStartedAt: banned!.banned_at,
-		Duration: 3 * 86_400,
+		TimeoutStartedAt: null,
 	})
+	expect(frames[0].data.Duration).toBeGreaterThan(3 * 86_400 - 5)
+	expect(frames[0].data.Duration).toBeLessThanOrEqual(3 * 86_400)
 
 	// And they are out of that instance — moved into their own DORM, not deleted: `match`
 	// lets a banned player matchmake there and nowhere else, so it is where they read the