fix(admin): name the characters the audit details column left as ids

src/app/admin/audit/page.tsx

@@ -682,6 +682,7 @@ export default async function AdminAuditPage({
                           roleNames,
                           tierLabels,
                           new Map(Object.entries(r.detailAccountNames)),
+                          new Map(Object.entries(r.detailCharacterNames)),
                         )}
                       />
                     ) : (

src/app/admin/audit/summarize.ts

@@ -27,6 +27,7 @@ type Render = (
   roleNames: ReadonlyMap<string, string>,
   labels: Record<string, string>,
   accountNames: ReadonlyMap<string, string>,
+  characterNames: ReadonlyMap<string, string>,
 ) => string;
 
 /**
@@ -101,7 +102,9 @@ function scalar(key: string): Part {
   return part([key], (d) => (d[key] === undefined ? "" : fmt(d[key])));
 }
 
-/** A payload value behind a fixed word, e.g. `character 90000001`. */
+/** A payload value behind a fixed word, e.g. `detected by token-health`. For a
+ * value that is a character id, use `characterRef` — it renders exactly this on
+ * the miss path but resolves a name when there is one. */
 function labelled(word: string, key: string): Part {
   return part([key], (d) => (d[key] === undefined ? "" : `${word} ${fmt(d[key])}`));
 }
@@ -164,6 +167,27 @@ function accountRef(word: string, key: string): Part {
   });
 }
 
+/** A character id resolved to its name, e.g. `main Probe Kid`. Unlike
+ * `accountRef`'s uuid or `roleRef`'s snowflake, a miss degrades to the raw id
+ * via `fmt`, not to a shortened form: a 9-10 digit character id is already
+ * legible, so truncating it would destroy information for no gain, and
+ * rendering it whole keeps the miss path byte-identical to what this column
+ * showed before names were resolved -- an unresolvable row is no worse than
+ * it is today. `characterNames` is keyed by the `details` field name, the
+ * same convention `accountNames` uses -- see `resolveAuditIdentities`'s
+ * `detailCharacterNames`. The miss path is deliberately `labelled`'s exactly
+ * -- render on any defined value, nothing only when the key is absent -- so a
+ * row this cannot resolve reads exactly as it did before, whatever shape the
+ * unenforced jsonb column turns out to hold. */
+function characterRef(word: string, key: string): Part {
+  return part([key], (d, _roleNames, _labels, _accountNames, characterNames) => {
+    const raw = d[key];
+    if (raw === undefined) return "";
+    const name = characterNames.get(key);
+    return `${word} ${name ?? fmt(raw)}`;
+  });
+}
+
 /** A single Discord role id, resolved to its configured tier name where one
  * exists. The id in `roleId` (role_strip_failed, role_sync_failed) is always
  * one of `discord.roleIds`' values -- the same set `roleNames` is built from
@@ -263,7 +287,7 @@ const PARTS: Record<string, readonly Part[]> = {
   "tier.approved": [tierTransition("from", "to"), flag("locked", "locked")],
   "account.merged": [
     shortRef("absorbed", "sourceAccountId"),
-    labelled("character", "characterId"),
+    characterRef("character", "characterId"),
   ],
   // The only action in the repo whose payload exceeds FALLBACK_KEYS, and the
   // key the fallback dropped was the price — the reason the row exists.
@@ -293,17 +317,22 @@ const PARTS: Record<string, readonly Part[]> = {
   // noteChange's doc.
   "payout.notes_changed": [noteChange("had", "has")],
   "status.changed": [transition("from", "to"), flag("self", "self-service")],
-  "admin.bootstrap_granted": [labelled("character", "characterId")],
-  "account.created": [labelled("main", "mainCharacterId")],
-  "account.main_changed": [labelled("main →", "mainCharacterId")],
+  "admin.bootstrap_granted": [characterRef("character", "characterId")],
+  "account.created": [characterRef("main", "mainCharacterId")],
+  "account.main_changed": [characterRef("main →", "mainCharacterId")],
   // Same shape as the member-driven row above; the two are separate actions
   // only so the log can say who drove it (services/accounts.ts).
-  "admin.main_changed": [labelled("main →", "mainCharacterId")],
+  "admin.main_changed": [characterRef("main →", "mainCharacterId")],
   "character.reclaimed": [accountRef("from", "fromAccount")],
   "character.unlinked": [scalar("name"), flag("wasMain", "was main")],
   "token.invalidated": [scalar("reason")],
   "token.verify_failed": [scalar("error")],
-  "token.subject_mismatch": [labelled("subject", "subjectCharacterId")],
+  // The subject is by construction a different character from the row's
+  // target (jobs/token-health.ts:61 -- the mismatch is the whole point). It
+  // names whichever character the token's EVE subject belongs to, which this
+  // app need never have held a row for, so a raw id here is expected rather
+  // than a sign the lookup is broken.
+  "token.subject_mismatch": [characterRef("subject", "subjectCharacterId")],
   // Two writers, two payload shapes. token-health computes a shortfall against
   // config and sends `missingScopes`; the location job sends the single scope
   // whose read ESI refused. Each renders nothing for the other's keys, so the
@@ -316,10 +345,10 @@ const PARTS: Record<string, readonly Part[]> = {
   "tier.unlocked": [tierLabelled("was", "tier")],
   "status.note_changed": [noteChange("had", "has")],
   "character.owner_mismatch": [labelled("detected by", "detectedBy")],
-  "access_list.holder_designated": [labelled("character", "characterId")],
+  "access_list.holder_designated": [characterRef("character", "characterId")],
   "access_list.holder_replaced": [
-    labelled("character", "characterId"),
-    labelled("was", "previousCharacterId"),
+    characterRef("character", "characterId"),
+    characterRef("was", "previousCharacterId"),
   ],
   "access_list.watch_added": [accessListRef("name", "accessListId")],
   "access_list.watch_removed": [accessListRef("name", "accessListId")],
@@ -379,15 +408,18 @@ const FALLBACK_KEYS = 3;
  * tier value to this deployment's configured label. `accountNames` maps a
  * `details` field name (not a uuid) to the account it resolved to -- see
  * `resolveAuditIdentities`'s `detailAccountNames` and `accountRef` above.
- * All three passed in rather than imported so this module stays a pure
- * function of its arguments and needs no env to test.
+ * `characterNames` is the same idea for character ids -- see
+ * `detailCharacterNames` and `characterRef` above. All four passed in rather
+ * than imported so this module stays a pure function of its arguments and
+ * needs no env to test.
  */
 export function summarizeDetails(
   action: string,
   details: unknown,
   roleNames: ReadonlyMap<string, string> = new Map(),
   labels: Record<string, string> = {},
   accountNames: ReadonlyMap<string, string> = new Map(),
+  characterNames: ReadonlyMap<string, string> = new Map(),
 ): string {
   const d = (details && typeof details === "object" ? details : {}) as Record<
     string,
@@ -397,7 +429,7 @@ export function summarizeDetails(
     const parts = PARTS[action];
     if (parts) {
       const rendered = parts
-        .map((p) => p(d, roleNames, labels, accountNames))
+        .map((p) => p(d, roleNames, labels, accountNames, characterNames))
         .filter(Boolean);
       const declared = new Set(parts.flatMap((p) => p.keys));
       const hidden = Object.keys(d).filter((k) => !declared.has(k)).length;

src/services/audit.ts

@@ -84,6 +84,13 @@ export type ResolvedAuditRow = typeof auditLog.$inferSelect & {
    * without knowing which action wrote it. Empty for every row that carries
    * no such field — see `DETAIL_ACCOUNT_KEYS`. */
   detailAccountNames: Record<string, string>;
+  /** Character ids embedded in `details`, resolved to that character's name —
+   * see `DETAIL_CHARACTER_KEYS`. Keyed by the `details` field name rather than
+   * by the id: `access_list.holder_replaced` carries two character ids on the
+   * same row (`characterId` and `previousCharacterId`), and an id-keyed map
+   * could not tell a caller which field either name came from — nor hold both
+   * when a re-designation of the sitting holder makes the two ids equal. */
+  detailCharacterNames: Record<string, string>;
 };
 
 const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
@@ -218,6 +225,64 @@ const DETAIL_ACCOUNT_KEYS: Readonly<Record<string, readonly string[]>> = {
   "character.reclaimed": ["fromAccount"],
 };
 
+/**
+ * `details` keys that hold a character id, per action — same idea as
+ * `DETAIL_ACCOUNT_KEYS`, one join fewer. Resolution is best-effort in both
+ * directions: a character row is HARD-deleted the moment it is unlinked or
+ * reclaimed (accounts.ts, both the reclaim and unlink paths), and
+ * `token.subject_mismatch` below can name a character this app never held a
+ * row for at all. Either way the renderer falls back to the raw id, same as
+ * any other unresolved detail.
+ *
+ * `access_list.watch_added`/`watch_removed`'s `accessListId` is deliberately
+ * NOT listed here — it is an EVE access-list id, not a character id, and is
+ * exactly the false-positive a bare key-name heuristic would produce. The
+ * `NAMESPACE_TARGET_KIND` comment above flags the same namespace but reaches
+ * the opposite conclusion, and deliberately: there the collision is tolerated
+ * because a ~6-digit list id will not match a ~9-10-digit character id in
+ * practice, and nothing user-facing reads that resolution anyway. Here the
+ * result is rendered, so the key is excluded outright rather than left to a
+ * size argument.
+ *
+ * `token.subject_mismatch`'s `subjectCharacterId` is, by construction, a
+ * DIFFERENT character from the row's own target (src/jobs/token-health.ts:61 —
+ * the mismatch is the whole point): it names whichever character the token's
+ * EVE subject actually belongs to, which need never have been linked here.
+ * A raw id on those rows is expected, not a sign the lookup is broken.
+ */
+const DETAIL_CHARACTER_KEYS: Readonly<Record<string, readonly string[]>> = {
+  "account.created": ["mainCharacterId"],
+  "account.main_changed": ["mainCharacterId"],
+  "admin.main_changed": ["mainCharacterId"],
+  "account.merged": ["characterId"],
+  "admin.bootstrap_granted": ["characterId"],
+  "access_list.holder_designated": ["characterId"],
+  "access_list.holder_replaced": ["characterId", "previousCharacterId"],
+  "token.subject_mismatch": ["subjectCharacterId"],
+};
+
+/**
+ * `jsonb` gives back a genuine number for anything written as one, but the
+ * column's shape is unenforced by the type system — a hand-inserted or
+ * legacy row could hold the same id as a digit string instead. Accept either
+ * and reject everything else, rather than assuming today's writer shape is
+ * the only one that will ever be read.
+ *
+ * The string branch round-trips through `String` rather than trusting
+ * `Number`, because this path renders a NAME: `"0090000001"` and a 17-digit
+ * string both coerce to some other character's id, and the summary would then
+ * confidently name a character the payload never referred to. Failing the
+ * round-trip degrades to the raw-id render, which is honest.
+ */
+function detailCharacterId(raw: unknown): number | null {
+  if (typeof raw === "number") return Number.isInteger(raw) ? raw : null;
+  if (typeof raw === "string" && DIGITS_RE.test(raw)) {
+    const id = Number(raw);
+    return String(id) === raw ? id : null;
+  }
+  return null;
+}
+
 /**
  * Resolves actor/target ids to human (main character) names in a fixed,
  * small number of batched queries, independent of row count:
@@ -229,8 +294,8 @@ const DETAIL_ACCOUNT_KEYS: Readonly<Record<string, readonly string[]>> = {
  *   2b. names of deleted payout operations, recovered from the `payout.deleted`
  *      audit row's own details — skipped entirely if step 1 resolved every
  *      payout target already
- *   3. every character name needed (target characters + all main characters
- *      collected above), in one shot
+ *   3. every character name needed (target characters + character ids read out
+ *      of `details` + all main characters collected above), in one shot
  * Anything that doesn't resolve is left as `null`/`"unresolved"`; the raw
  * `actor`/`target` strings on the row are always preserved unchanged.
  */
@@ -244,6 +309,7 @@ export async function resolveAuditIdentities(
   const targetCharacterIds = new Set<number>();
   const targetDiscordIds = new Set<string>();
   const targetPayoutIds = new Set<string>();
+  const detailCharacterIds = new Set<number>();
 
   for (const r of rows) {
     if (r.actor !== "system" && UUID_RE.test(r.actor)) accountIds.add(r.actor);
@@ -264,6 +330,14 @@ export async function resolveAuditIdentities(
       const raw = r.details?.[key];
       if (typeof raw === "string" && UUID_RE.test(raw)) accountIds.add(raw);
     }
+    // Same idea, one join fewer: a character id living inside `details`.
+    // Collected separately only because the name lookup needs to know which
+    // ids came from here; unioned into `characterIds` at step 3 so it shares
+    // that one query rather than paying for a new one.
+    for (const key of DETAIL_CHARACTER_KEYS[r.action] ?? []) {
+      const id = detailCharacterId(r.details?.[key]);
+      if (id !== null) detailCharacterIds.add(id);
+    }
   }
 
   const [directAccounts, links, payoutOperations] = await Promise.all([
@@ -332,7 +406,7 @@ export async function resolveAuditIdentities(
     }
   }
 
-  const characterIds = new Set<number>(targetCharacterIds);
+  const characterIds = new Set<number>([...targetCharacterIds, ...detailCharacterIds]);
   for (const a of accountById.values()) {
     if (a.mainCharacterId !== null) characterIds.add(a.mainCharacterId);
   }
@@ -410,7 +484,22 @@ export async function resolveAuditIdentities(
       if (name !== null) detailAccountNames[key] = name;
     }
 
-    return { ...r, actorName, actorKind, targetName, targetKind, detailAccountNames };
+    const detailCharacterNames: Record<string, string> = {};
+    for (const key of DETAIL_CHARACTER_KEYS[r.action] ?? []) {
+      const id = detailCharacterId(r.details?.[key]);
+      const name = id !== null ? (nameByCharacterId.get(id) ?? null) : null;
+      if (name !== null) detailCharacterNames[key] = name;
+    }
+
+    return {
+      ...r,
+      actorName,
+      actorKind,
+      targetName,
+      targetKind,
+      detailAccountNames,
+      detailCharacterNames,
+    };
   });
 }
 

tests/audit-resolve.test.ts

@@ -251,6 +251,111 @@ describe("resolveAuditIdentities / queryAuditLog resolution", () => {
     expect(row.detailAccountNames).toEqual({});
   });
 
+  it("resolves a detail character id (account.main_changed's mainCharacterId)", async () => {
+    const acc = await seedAccount(ctx.db);
+    await seedCharacter(ctx.db, cfg, {
+      id: 90008,
+      accountId: acc.id,
+      name: "New Main",
+      main: true,
+    });
+    await logAudit(ctx.db, {
+      actor: acc.id,
+      action: "account.main_changed",
+      target: acc.id,
+      details: { mainCharacterId: 90008 },
+    });
+    const [row] = await queryAuditLog(ctx.db);
+    expect(row.detailCharacterNames).toEqual({ mainCharacterId: "New Main" });
+  });
+
+  it("resolves both characterId and previousCharacterId on a holder_replaced row to distinct names", async () => {
+    const acc1 = await seedAccount(ctx.db);
+    const acc2 = await seedAccount(ctx.db);
+    await seedCharacter(ctx.db, cfg, {
+      id: 90009,
+      accountId: acc1.id,
+      name: "New Holder",
+    });
+    await seedCharacter(ctx.db, cfg, {
+      id: 90010,
+      accountId: acc2.id,
+      name: "Old Holder",
+    });
+    await logAudit(ctx.db, {
+      actor: "system",
+      action: "access_list.holder_replaced",
+      target: "some-list",
+      details: { characterId: 90009, previousCharacterId: 90010 },
+    });
+    const [row] = await queryAuditLog(ctx.db);
+    expect(row.detailCharacterNames).toEqual({
+      characterId: "New Holder",
+      previousCharacterId: "Old Holder",
+    });
+  });
+
+  it("leaves detailCharacterNames empty when the detail character id doesn't exist", async () => {
+    await logAudit(ctx.db, {
+      actor: "system",
+      action: "account.main_changed",
+      target: "all",
+      details: { mainCharacterId: 424242 },
+    });
+    const [row] = await queryAuditLog(ctx.db);
+    expect(row.detailCharacterNames).toEqual({});
+  });
+
+  it("leaves detailCharacterNames empty for actions that don't declare a character-id detail key", async () => {
+    await logAudit(ctx.db, {
+      actor: "system",
+      action: "tier.changed",
+      target: "all",
+      details: { from: "member", to: "alumni" },
+    });
+    const [row] = await queryAuditLog(ctx.db);
+    expect(row.detailCharacterNames).toEqual({});
+  });
+
+  it("resolves a character id the payload carried as a digit string", async () => {
+    const acc = await seedAccount(ctx.db);
+    await seedCharacter(ctx.db, cfg, {
+      id: 90011,
+      accountId: acc.id,
+      name: "String Main",
+      main: true,
+    });
+    await logAudit(ctx.db, {
+      actor: acc.id,
+      action: "account.main_changed",
+      target: acc.id,
+      details: { mainCharacterId: "90011" },
+    });
+    const [row] = await queryAuditLog(ctx.db);
+    expect(row.detailCharacterNames).toEqual({ mainCharacterId: "String Main" });
+  });
+
+  it("refuses a digit string that is not this id's own decimal form", async () => {
+    const acc = await seedAccount(ctx.db);
+    await seedCharacter(ctx.db, cfg, {
+      id: 90012,
+      accountId: acc.id,
+      name: "Zero Padded",
+      main: true,
+    });
+    // `Number("0090012")` is 90012, so a naive coercion would render the
+    // summary as "main → Zero Padded" for a payload that never held that id.
+    // Naming the wrong character is worse than showing the raw value.
+    await logAudit(ctx.db, {
+      actor: acc.id,
+      action: "account.main_changed",
+      target: acc.id,
+      details: { mainCharacterId: "0090012" },
+    });
+    const [row] = await queryAuditLog(ctx.db);
+    expect(row.detailCharacterNames).toEqual({});
+  });
+
   it("resolves a full page of 200+ rows with a small, constant number of queries (no N+1)", async () => {
     const accounts = await Promise.all(
       Array.from({ length: 20 }, () => seedAccount(ctx.db)),