fix(tag): require reasons only for sticky removals

packages/core/src/contracts/schemas/__tests__/tag-schemas.test.ts

@@ -0,0 +1,52 @@
+import { describe, expect, it } from 'vitest';
+import { removePlayerTagSchema, replacePlayerTagSchema } from '../tag.js';
+
+const removeBase = {
+  playerId: '11111111-1111-4111-8111-111111111111',
+  tagKey: 'vip' as const,
+  removalActor: 'manual' as const,
+  removalActorUserId: null,
+};
+
+const replaceBase = {
+  playerId: '11111111-1111-4111-8111-111111111111',
+  tagKey: 'level' as const,
+  assignReason: 'level changed',
+  assignActor: 'scheduled' as const,
+  assignActorUserId: null,
+};
+
+describe('player tag removal reason schemas', () => {
+  it.each([
+    ['empty', ''],
+    ['whitespace-only', '   '],
+    ['four non-whitespace characters', ' abcd '],
+  ])('rejects %s for removePlayerTag', (_label, removalReason) => {
+    expect(removePlayerTagSchema.safeParse({ ...removeBase, removalReason }).success).toBe(false);
+  });
+
+  it('allows a missing reason so the service can apply sticky-tag rules', () => {
+    expect(removePlayerTagSchema.safeParse(removeBase).success).toBe(true);
+  });
+
+  it.each([
+    ['missing', undefined],
+    ['empty', ''],
+    ['whitespace-only', '   '],
+    ['four non-whitespace characters', ' abcd '],
+  ])('rejects %s for replacePlayerTag', (_label, removalReason) => {
+    expect(replacePlayerTagSchema.safeParse({ ...replaceBase, removalReason }).success).toBe(false);
+  });
+
+  it('accepts exactly five non-whitespace characters and trims removePlayerTag output', () => {
+    expect(
+      removePlayerTagSchema.parse({ ...removeBase, removalReason: '  abcde  ' }).removalReason,
+    ).toBe('abcde');
+  });
+
+  it('accepts exactly five non-whitespace characters and trims replacePlayerTag output', () => {
+    expect(
+      replacePlayerTagSchema.parse({ ...replaceBase, removalReason: '  abcde  ' }).removalReason,
+    ).toBe('abcde');
+  });
+});

packages/core/src/contracts/schemas/tag.ts

@@ -71,7 +71,7 @@ export type AssignPlayerTagInput = z.infer<typeof assignPlayerTagSchema>;
 
 export const removePlayerTagSchema = playerTagSchema.pick({ playerId: true }).extend({
   tagKey: TagKeySchema,
-  removalReason: z.string().min(5),
+  removalReason: z.string().trim().min(5).optional(),
   removalActor: tagAssignRemoveSourceSchema,
   removalActorUserId: UuidSchema.nullable(),
 });
@@ -80,7 +80,7 @@ export type RemovePlayerTagInput = z.infer<typeof removePlayerTagSchema>;
 // Atomic same-key swap (the mutable `level` tag): one actor performs both halves,
 // so the assign actor fields double as the removal actor fields.
 export const replacePlayerTagSchema = assignPlayerTagSchema.extend({
-  removalReason: z.string().min(5),
+  removalReason: z.string().trim().min(5),
 });
 export type ReplacePlayerTagInput = z.infer<typeof replacePlayerTagSchema>;
 

packages/core/src/pam/tag/__tests__/tag.router.int.test.ts

@@ -56,6 +56,10 @@ async function seedTag(key: TagKey) {
   await db.drizzle.db.insert(tag).values({ key });
 }
 
+async function storedPlayerTags() {
+  return db.drizzle.db.select().from(playerTag);
+}
+
 async function storedRules() {
   return db.drizzle.db.select().from(tagRule);
 }
@@ -241,3 +245,107 @@ describe('tag router error mapping', () => {
     ).rejects.toMatchObject({ code: 'NOT_FOUND' });
   });
 });
+
+describe('tag router player-tag removal', () => {
+  it('normalizes the removal reason before persistence and audit emission', async () => {
+    await seedTag('high_roller');
+    const { router, events } = build(allowingGuard());
+    const assignInput = {
+      playerId: UID,
+      tagKey: 'high_roller' as TagKey,
+      assignReason: 'manual review',
+      assignActor: 'manual' as const,
+    };
+    await call(router.assignPlayerTag, assignInput, { context: AUTHED_CTX });
+    events.emit.mockClear();
+
+    const result = await call(
+      router.removePlayerTag,
+      {
+        playerId: UID,
+        tagKey: 'high_roller',
+        removalReason: '  cleared after review  ',
+        removalActor: 'manual',
+      },
+      { context: AUTHED_CTX },
+    );
+
+    expect(result.removalReason).toBe('cleared after review');
+    expect((await storedPlayerTags()).at(0)?.removalReason).toBe('cleared after review');
+    expect(events.emit).toHaveBeenCalledWith(
+      'tag.player.removed',
+      expect.objectContaining({ reason: 'cleared after review' }),
+    );
+  });
+
+  it('rejects a whitespace-only removal reason at the API boundary', async () => {
+    await seedTag('high_roller');
+    const { router } = build(allowingGuard());
+
+    await expect(
+      call(
+        router.removePlayerTag,
+        {
+          playerId: UID,
+          tagKey: 'high_roller',
+          removalReason: '   ',
+          removalActor: 'manual',
+        },
+        { context: AUTHED_CTX },
+      ),
+    ).rejects.toBeInstanceOf(ORPCError);
+    expect(await storedPlayerTags()).toHaveLength(0);
+  });
+
+  it('requires a reason when removing a sticky tag', async () => {
+    await db.drizzle.db.insert(tag).values({ key: 'vip', isSticky: true });
+    const { router } = build(allowingGuard());
+    await call(
+      router.assignPlayerTag,
+      {
+        playerId: UID,
+        tagKey: 'vip',
+        assignReason: 'manual review',
+        assignActor: 'manual',
+      },
+      { context: AUTHED_CTX },
+    );
+
+    await expect(
+      call(
+        router.removePlayerTag,
+        { playerId: UID, tagKey: 'vip', removalActor: 'manual' },
+        { context: AUTHED_CTX },
+      ),
+    ).rejects.toMatchObject({ code: 'BAD_REQUEST' });
+    expect((await storedPlayerTags()).at(0)?.removedAt).toBeNull();
+  });
+
+  it('allows removing a non-sticky tag without a reason', async () => {
+    await seedTag('high_roller');
+    const { router, events } = build(allowingGuard());
+    await call(
+      router.assignPlayerTag,
+      {
+        playerId: UID,
+        tagKey: 'high_roller',
+        assignReason: 'manual review',
+        assignActor: 'manual',
+      },
+      { context: AUTHED_CTX },
+    );
+    events.emit.mockClear();
+
+    await call(
+      router.removePlayerTag,
+      { playerId: UID, tagKey: 'high_roller', removalActor: 'manual' },
+      { context: AUTHED_CTX },
+    );
+
+    expect((await storedPlayerTags()).at(0)?.removalReason).toBe('manual tag removal');
+    expect(events.emit).toHaveBeenCalledWith(
+      'tag.player.removed',
+      expect.objectContaining({ reason: 'manual tag removal' }),
+    );
+  });
+});

packages/core/src/pam/tag/router/index.ts

@@ -8,6 +8,7 @@ import {
   TagAssignmentNotFoundError,
   TagKeyConflictError,
   TagInUseError,
+  TagRemovalReasonRequiredError,
 } from '../service/tag.service.js';
 import { TagRuleService, TagRuleNotFoundError } from '../service/tag-rule.service.js';
 
@@ -52,11 +53,16 @@ export function createTagRouter(tag: TagService, rule: TagRuleService, adminGuar
 
     removePlayerTag: os.removePlayerTag.handler(async ({ context, input }) => {
       await adminGuard.assert(context, 'tag', 'delete');
-      return mapErrors({ NOT_FOUND: [TagNotFoundError, TagAssignmentNotFoundError] }, () =>
-        tag.removePlayerTag(
-          { ...input, removalActorUserId: getUserId(context) },
-          context.clientMeta,
-        ),
+      return mapErrors(
+        {
+          BAD_REQUEST: TagRemovalReasonRequiredError,
+          NOT_FOUND: [TagNotFoundError, TagAssignmentNotFoundError],
+        },
+        () =>
+          tag.removePlayerTag(
+            { ...input, removalActorUserId: getUserId(context) },
+            context.clientMeta,
+          ),
       );
     }),
 

packages/core/src/pam/tag/service/tag.service.ts

@@ -65,6 +65,20 @@ export const TagInUseError = makeConflictError(
   'TagInUseError',
   'Tag is still referenced by a player tag or tag rule and cannot be deleted',
 );
+export const TagRemovalReasonRequiredError = makeConflictError(
+  'TagRemovalReasonRequiredError',
+  'A removal reason is required for sticky tags',
+);
+
+const DEFAULT_MANUAL_REMOVAL_REASON = 'manual tag removal';
+
+function resolveRemovalReason(reason: string | undefined, isSticky: boolean) {
+  const normalized = reason?.trim();
+  if (isSticky && !normalized) {
+    throw new TagRemovalReasonRequiredError();
+  }
+  return normalized ?? DEFAULT_MANUAL_REMOVAL_REASON;
+}
 
 // "First evidence wins per breach dimension, but a dimension that was never recorded gets
 // filled in the moment it's observed." Never overwrites an already-populated dimension with
@@ -342,6 +356,7 @@ export class TagService implements PlayerTags {
 
   private async _removePlayerTagOnTx(trx: DrizzleTx, args: RemovePlayerTagInput) {
     const foundTag = await this._findTagByKeyOrThrow(args.tagKey, trx);
+    const removalReason = resolveRemovalReason(args.removalReason, foundTag.isSticky);
     const active = findOneOrThrow(
       await trx
         .select()
@@ -360,7 +375,7 @@ export class TagService implements PlayerTags {
       .update(playerTag)
       .set({
         removedAt: new Date(),
-        removalReason: args.removalReason,
+        removalReason,
         removalActor: args.removalActor,
         removalActorUserId: args.removalActorUserId,
       })
@@ -374,6 +389,7 @@ export class TagService implements PlayerTags {
       const db = this.drizzle.db;
       const result = await db.transaction(async (trx) => {
         const foundTag = await this._findTagByKeyOrThrow(args.tagKey, trx);
+        const removalReason = resolveRemovalReason(args.removalReason, foundTag.isSticky);
         const active = findOneOrThrow(
           await trx
             .select()
@@ -392,7 +408,7 @@ export class TagService implements PlayerTags {
           .update(playerTag)
           .set({
             removedAt: new Date(),
-            removalReason: args.removalReason,
+            removalReason,
             removalActor: args.removalActor,
             removalActorUserId: args.removalActorUserId,
           })
@@ -403,7 +419,7 @@ export class TagService implements PlayerTags {
       void this.event.emit('tag.player.removed', {
         playerId: args.playerId,
         tagKey: args.tagKey,
-        reason: args.removalReason,
+        reason: result.removalReason ?? DEFAULT_MANUAL_REMOVAL_REASON,
         actorId: args.removalActorUserId ?? SYSTEM_ACTOR_ID,
         ip: meta?.ip ?? null,
         userAgent: meta?.userAgent ?? null,