fix: fence profile updates and check TeamSpeak permission failures

src/bot/profile.test.ts

@@ -1,5 +1,7 @@
 import { describe, it, expect, beforeEach, vi } from "vitest";
+import { Client, generateIdentity } from "@honeybbq/teamspeak-client";
 import { BotProfileManager } from "./profile.js";
+import { TS6HttpQuery } from "../ts-protocol/http-query.js";
 import type { TS3Client } from "../ts-protocol/client.js";
 import type { QueuedSong } from "../audio/queue.js";
 
@@ -14,6 +16,9 @@ function makeMockTs(): TS3Client & {
     get clearCalls() { return clears; },
     getHost: () => "127.0.0.1",
     getHttpQuery: () => null,
+    getClientId: () => 17,
+    getChannelId: () => 5n,
+    execCommand: vi.fn().mockResolvedValue(undefined),
     fileTransferInitUpload: vi.fn().mockResolvedValue({}),
     uploadFileData: vi.fn().mockImplementation(async (_h: any, _i: any, stream: any) => {
       const chunks: Buffer[] = [];
@@ -213,7 +218,7 @@ describe("BotProfileManager channel description follows the bot (#159)", () => {
     ts.cid = 5n;
     (ts as any).getChannelId = () => ts.cid;
     channelEdits = () =>
-      (ts.sendCommandNoWait as any).mock.calls
+      (ts.execCommand as any).mock.calls
         .map((c: any[]) => c[0] as string)
         .filter((cmd: string) => cmd.startsWith("channeledit"));
   });
@@ -264,3 +269,174 @@ describe("BotProfileManager channel description follows the bot (#159)", () => {
     expect(channelEdits()).toHaveLength(1);
   });
 });
+
+function deferred<T>() {
+  let resolve!: (value: T) => void;
+  let reject!: (error: Error) => void;
+  const promise = new Promise<T>((res, rej) => { resolve = res; reject = rej; });
+  return { promise, resolve, reject };
+}
+
+function makeHttpProfile(partial: Partial<typeof cfgOff> = {}) {
+  const ts = makeMockTs() as any;
+  const state = { clid: 17, cid: 5n };
+  const descriptions = new Map<number, string>();
+  const http = new TS6HttpQuery({ host: "127.0.0.1", port: 10080 });
+  const request = vi.spyOn(http, "request").mockImplementation(async (_method, path, body) => {
+    if (path.includes("clientlist")) {
+      return { status: 200, body: { body: [{ clid: String(state.clid), cid: String(state.cid) }], status: { code: 0, message: "ok" } } };
+    }
+    if (path.includes("channeledit")) descriptions.set(Number(body!.cid), String(body!.channel_description));
+    return { status: 200, body: { status: { code: 0, message: "ok" } } };
+  });
+  ts.getHttpQuery = () => http;
+  ts.getClientId = () => state.clid;
+  ts.getChannelId = () => state.cid;
+  const logger: any = { child: () => logger, info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() };
+  const pm = new BotProfileManager(ts, logger, { ...cfgOff, ...partial }, "Bot");
+  return { pm, ts, state, http, request, descriptions, logger };
+}
+
+describe("BotProfileManager checked TS6 profile lifecycle", () => {
+  it("clears the old channel through HTTP Query when moved, even without full-client edit permission", async () => {
+    const { pm, ts, state, descriptions } = makeHttpProfile({ channelDescEnabled: true });
+    ts.execCommand.mockRejectedValue(new Error("insufficient client permissions"));
+    await pm.onSongChange(fakeSong);
+    expect(descriptions.get(5)).toContain("X - Y");
+    state.cid = 9n;
+    await pm.onChannelMoved(9n);
+    expect(descriptions.get(5)).toBe("");
+    expect(descriptions.get(9)).toContain("X - Y");
+    expect(ts.sendCommandNoWait).not.toHaveBeenCalled();
+    expect(ts.execCommand).not.toHaveBeenCalled();
+  });
+
+  it("discards an old client-list reply after reconnect", async () => {
+    const { pm, state, request } = makeHttpProfile({ channelDescEnabled: true });
+    state.cid = 0n;
+    const lookup = deferred<any>();
+    request.mockImplementationOnce(() => lookup.promise);
+    const update = pm.onSongChange(fakeSong);
+    await flush();
+    state.clid = 21;
+    state.cid = 9n;
+    pm.onConnect();
+    lookup.resolve({ status: 200, body: { body: [{ clid: "17", cid: "5" }], status: { code: 0, message: "ok" } } });
+    await update;
+    expect(request.mock.calls.filter((call) => call[1].includes("channeledit"))).toEqual([]);
+    await pm.onSongChange(null);
+    expect(request).toHaveBeenLastCalledWith("POST", "/1/channeledit?sid=1", { cid: 9, channel_description: "" });
+  });
+
+  it("does not restore the previous remembered channel when a write completes after reconnect", async () => {
+    const { pm, state, request } = makeHttpProfile({ channelDescEnabled: true });
+    const write = deferred<any>();
+    request.mockImplementationOnce(() => write.promise);
+    const update = pm.onSongChange(fakeSong);
+    await flush();
+    state.clid = 21;
+    state.cid = 9n;
+    pm.onConnect();
+    write.resolve({ status: 200, body: { status: { code: 0, message: "ok" } } });
+    await update;
+    await pm.onSongChange(null);
+    expect(request).toHaveBeenLastCalledWith("POST", "/1/channeledit?sid=1", { cid: 9, channel_description: "" });
+  });
+
+  it("discards a pending channel lookup when playback stops", async () => {
+    const { pm, ts, request, descriptions } = makeHttpProfile({ channelDescEnabled: true });
+    ts.getChannelId = () => 0n;
+    const lookup = deferred<any>();
+    request.mockImplementationOnce(() => lookup.promise);
+    const update = pm.onSongChange(fakeSong);
+    await flush();
+    await pm.onSongChange(null);
+    lookup.resolve({ status: 200, body: { body: [{ clid: "17", cid: "5" }], status: { code: 0, message: "ok" } } });
+    await update;
+    expect(descriptions.get(5)).toBe("");
+  });
+
+  it("discards a pending channel lookup when the bot is moved", async () => {
+    const { pm, ts, request, descriptions } = makeHttpProfile({ channelDescEnabled: true });
+    ts.getChannelId = () => 0n;
+    const lookup = deferred<any>();
+    request.mockImplementationOnce(() => lookup.promise);
+    const update = pm.onSongChange(fakeSong);
+    await flush();
+    await pm.onChannelMoved(9n);
+    lookup.resolve({ status: 200, body: { body: [{ clid: "17", cid: "5" }], status: { code: 0, message: "ok" } } });
+    await update;
+    expect(descriptions.has(5)).toBe(false);
+    expect(descriptions.get(9)).toContain("X - Y");
+  });
+
+  it("does not disable the new connection after an old write returns a permission failure", async () => {
+    const { pm, state, request } = makeHttpProfile({ channelDescEnabled: true });
+    const write = deferred<any>();
+    request.mockImplementationOnce(() => write.promise);
+    const update = pm.onSongChange(fakeSong);
+    await flush();
+    state.clid = 21;
+    state.cid = 9n;
+    pm.onConnect();
+    write.resolve({ status: 403, body: { status: { code: 2568, message: "insufficient client permissions" } } });
+    await update;
+    await pm.onSongChange(fakeSong);
+    expect(request).toHaveBeenLastCalledWith("POST", "/1/channeledit?sid=1", { cid: 9, channel_description: "♪ 正在播放: X - Y\n专辑: Z\n平台: netease" });
+  });
+
+  it("resolves an unknown channel and sends raw newlines with one targeted description update", async () => {
+    const { pm, ts, state, request } = makeHttpProfile({ channelDescEnabled: true, descriptionEnabled: true });
+    ts.getChannelId = () => 0n;
+    state.cid = 5n;
+    await pm.onSongChange(fakeSong);
+    expect(request.mock.calls.filter((call) => call[1].includes("clientedit"))).toEqual([
+      ["POST", "/1/clientedit?sid=1", { clid: 17, client_description: "X - Y [Z]" }],
+    ]);
+    expect(request).toHaveBeenLastCalledWith("POST", "/1/channeledit?sid=1", { cid: 5, channel_description: "♪ 正在播放: X - Y\n专辑: Z\n平台: netease" });
+    expect(ts.execCommand).not.toHaveBeenCalled();
+  });
+
+  it("does not resolve or write a disconnected client", async () => {
+    const { pm, state, request } = makeHttpProfile({ channelDescEnabled: true, descriptionEnabled: true });
+    state.clid = 0;
+    state.cid = 0n;
+    await pm.onSongChange(fakeSong);
+    expect(request).not.toHaveBeenCalled();
+  });
+
+  it("reports HTTP lookup permission errors once and retries after reconnect", async () => {
+    const { pm, ts, request } = makeHttpProfile({ channelDescEnabled: true });
+    ts.getChannelId = () => 0n;
+    request.mockResolvedValue({ status: 403, body: { status: { code: 2568, message: "insufficient client permissions" } } });
+    await pm.onSongChange(fakeSong);
+    await pm.onSongChange(fakeSong);
+    expect(request).toHaveBeenCalledTimes(1);
+    pm.onConnect();
+    await pm.onSongChange(fakeSong);
+    expect(request).toHaveBeenCalledTimes(2);
+  });
+
+  it("checks self clientupdate permission responses and retries only after reconnect", async () => {
+    const { pm, ts, request, logger } = makeHttpProfile({ nicknameEnabled: true, awayStatusEnabled: true });
+    const client: any = new Client(generateIdentity(0), "127.0.0.1:9987", "Bot");
+    const commands: string[] = [];
+    client.handler.sendPacket = vi.fn((_type, data: Buffer) => {
+      const command = data.toString();
+      commands.push(command);
+      const returnCode = command.match(/return_code=(\d+)/)?.[1];
+      client.handler.onPacket({ typeFlagged: 2, data: Buffer.from(`error id=2568 msg=insufficient\\sclient\\spermissions${returnCode ? ` return_code=${returnCode}` : ""}`) });
+    });
+    ts.sendCommandNoWait.mockImplementation((command: string) => client.sendCommandNoWait(command));
+    ts.execCommand.mockImplementation((command: string) => client.execCommand(command));
+    await pm.onSongChange(null);
+    await pm.onSongChange(null);
+    expect(commands).toHaveLength(1);
+    expect(commands[0]).toMatch(/^clientupdate client_nickname=Bot client_away=1 client_away_message=等待播放 return_code=\d+$/);
+    expect(request).not.toHaveBeenCalled();
+    expect(logger.info.mock.calls.some((call: any[]) => call[1] === "Client properties updated (nickname + away)")).toBe(false);
+    pm.onConnect();
+    await pm.onSongChange(null);
+    expect(commands).toHaveLength(2);
+  });
+});

src/bot/profile.ts

@@ -13,6 +13,13 @@ const AVATAR_MAX_BYTES = 200 * 1024;
 /** Timeout for file-transfer operations (upload / delete). */
 const FILE_TRANSFER_TIMEOUT_MS = 6000;
 
+interface ProfileUpdateContext {
+  generation: number;
+  channelGeneration: number;
+  clientId: number;
+  httpQuery: ReturnType<TS3Client["getHttpQuery"]>;
+}
+
 /**
  * Manages the bot's TeamSpeak presence (avatar, description, nickname,
  * away status, channel description, now-playing messages).
@@ -57,6 +64,8 @@ export class BotProfileManager {
    * the generation changed, a newer update has superseded them.
    */
   private generation = 0;
+  /** Channel moves supersede channel writes without cancelling avatar work. */
+  private channelGeneration = 0;
 
   constructor(
     tsClient: TS3Client,
@@ -119,20 +128,25 @@ export class BotProfileManager {
    */
   async onSongChange(song: QueuedSong | null): Promise<void> {
     const gen = ++this.generation;
+    this.channelGeneration++;
     this.currentSong = song;
+    const context = this.createUpdateContext();
 
     // 1. Avatar first — file transfer uses its own response tracker and
     //    must run before sendCommandNoWait calls whose orphaned responses
     //    could confuse the command matcher.
     await this.updateAvatar(song?.coverUrl ?? null, gen);
-    if (this.generation !== gen) return; // superseded
+    if (!this.isCurrentUpdate(context)) return;
 
-    // 2. Combined clientupdate (nickname + away in one fire-and-forget)
-    await this.updateClientProperties(song);
+    // 2. Checked clientupdate sent by the visible client itself.
+    await this.updateClientProperties(song, context);
+    if (!this.isCurrentUpdate(context)) return;
     // 3. Description (clientedit on TS3, httpQuery on TS6)
-    await this.updateDescription(song);
-    // 4. Channel description (fire-and-forget channeledit)
-    await this.updateChannelDescription(song);
+    await this.updateDescription(song, context);
+    if (!this.isCurrentUpdate(context)) return;
+    // 4. Checked channel description update.
+    await this.updateChannelDescription(song, context);
+    if (!this.isCurrentUpdate(context)) return;
     // 5. Now-playing chat message
     if (song) await this.sendNowPlayingMessage(song);
   }
@@ -140,6 +154,7 @@ export class BotProfileManager {
   /** Reset permission-denied flags and bump generation on new connection. */
   onConnect(): void {
     this.generation++;
+    this.channelGeneration++;
     this.currentSong = null;
     // Channel ids are per-server; never carry one across a (re)connect.
     this.channelDescCid = null;
@@ -168,19 +183,20 @@ export class BotProfileManager {
     if (!this.config.channelDescEnabled || this.permDenied.channelDesc) return;
     const oldChannelId = this.channelDescCid;
     if (oldChannelId === newChannelId) return;
+    this.channelGeneration++;
+    const context = this.createUpdateContext();
+    const song = this.currentSong;
     try {
       if (oldChannelId !== null) {
-        await this.tsClient.sendCommandNoWait(
-          `channeledit cid=${oldChannelId} channel_description=`,
-        );
+        if (!await this.writeChannelDescription(oldChannelId, "", context)) return;
         this.channelDescCid = null;
       }
     } catch (err) {
-      this.handleFeatureError("channelDesc", err);
+      if (this.isCurrentChannelUpdate(context)) this.handleFeatureError("channelDesc", err);
       return;
     }
-    if (this.currentSong) {
-      await this.updateChannelDescription(this.currentSong, newChannelId);
+    if (song) {
+      await this.updateChannelDescription(song, context, newChannelId);
     }
   }
 
@@ -286,18 +302,19 @@ export class BotProfileManager {
     }
   }
 
-  private async updateDescription(song: QueuedSong | null): Promise<void> {
+  private async updateDescription(song: QueuedSong | null, context: ProfileUpdateContext): Promise<void> {
     if (!this.config.descriptionEnabled || this.permDenied.description) return;
+    if (!this.isCurrentUpdate(context)) return;
 
     try {
       const text = song
         ? `${song.name} - ${song.artist} [${song.album}]`
         : "";
 
-      const clid = this.tsClient.getClientId();
+      const clid = context.clientId;
       if (clid <= 0) return;
 
-      const httpQuery = this.tsClient.getHttpQuery();
+      const httpQuery = context.httpQuery;
 
       if (httpQuery) {
         // IMPORTANT:
@@ -306,6 +323,7 @@ export class BotProfileManager {
         const result = await httpQuery.clientEdit(clid, {
           client_description: text,
         });
+        if (!this.isCurrentUpdate(context)) return;
 
         this.logger.info(
           { status: result.status, clid },
@@ -318,27 +336,22 @@ export class BotProfileManager {
           ),
           5000,
         );
+        if (!this.isCurrentUpdate(context)) return;
 
         this.logger.info({ clid }, "Description updated");
       }
     } catch (err) {
-      this.handleFeatureError("description", err);
+      if (this.isCurrentUpdate(context)) this.handleFeatureError("description", err);
     }
   }
 
   /**
    * Build and send a single `clientupdate` command that sets nickname
-   * and away status together, avoiding multiple round-trips that can
-   * cause command-queue timeouts on the TS3 protocol.
-   *
-   * Values are collected as raw strings/numbers. The TS6 HTTP path
-   * forwards them as JSON (the server expects real spaces, not `\s`);
-   * the TS3 wire path escapes them on the fly. Previously the code
-   * escaped upfront and then split the escaped string to build the
-   * JSON body, so TS6 received literal backslashes and silently
-   * rejected the update.
+   * and away status together. The full client sends this command on both
+   * TS3 and TS6, with a return code so permission failures are observable.
    */
-  private async updateClientProperties(song: QueuedSong | null): Promise<void> {
+  private async updateClientProperties(song: QueuedSong | null, context: ProfileUpdateContext): Promise<void> {
+    if (!this.isCurrentUpdate(context) || context.clientId <= 0) return;
     const rawProps: Record<string, string | number> = {};
 
     // --- Nickname ---
@@ -374,20 +387,23 @@ export class BotProfileManager {
           : `${key}=${value}`,
       );
 
-      await this.tsClient.sendCommandNoWait(
-        `clientupdate ${parts.join(" ")}`,
+      await this.withTimeout(
+        this.tsClient.execCommand(`clientupdate ${parts.join(" ")}`),
+        5000,
       );
+      if (!this.isCurrentUpdate(context)) return;
 
       this.logger.info(
         {
-          clid: this.tsClient.getClientId(),
+          clid: context.clientId,
           props: Object.keys(rawProps),
         },
         "Client properties updated (nickname + away)",
       );
     } catch (err) {
-      this.handleFeatureError("nickname", err);
-      this.handleFeatureError("awayStatus", err);
+      if (!this.isCurrentUpdate(context)) return;
+      if (rawProps.client_nickname !== undefined) this.handleFeatureError("nickname", err);
+      if (rawProps.client_away !== undefined) this.handleFeatureError("awayStatus", err);
     }
   }
 
@@ -438,28 +454,35 @@ export class BotProfileManager {
 
   private async updateChannelDescription(
     song: QueuedSong | null,
+    context: ProfileUpdateContext,
     targetChannelId?: bigint,
   ): Promise<void> {
     if (!this.config.channelDescEnabled || this.permDenied.channelDesc) return;
+    if (!this.isCurrentChannelUpdate(context) || context.clientId <= 0) return;
 
     try {
-      let channelId = targetChannelId ?? this.tsClient.getChannelId();
+      // A stop already knows which channel to clear if a write succeeded.
+      // Avoid a needless client-list lookup that could prevent that cleanup.
+      let channelId = !song && this.channelDescCid !== null
+        ? this.channelDescCid
+        : targetChannelId ?? this.tsClient.getChannelId();
 
       // TS6 full-client may report channelID() as 0 even after the
       // visible music client has already joined a channel.
       // Fall back to HTTP Query and resolve our real clid -> cid.
       if (channelId === 0n) {
-        const httpQuery = this.tsClient.getHttpQuery();
-        const clid = this.tsClient.getClientId();
+        const httpQuery = context.httpQuery;
+        const clid = context.clientId;
 
         if (httpQuery && clid > 0) {
           const result = await httpQuery.clientList();
+          if (!this.isCurrentChannelUpdate(context)) return;
 
           const payload = result.body as {
             body?: Array<Record<string, string>>;
           };
 
-          const me = payload.body?.find(
+          const me = payload?.body?.find(
             (client) => Number(client.clid) === clid,
           );
 
@@ -478,43 +501,12 @@ export class BotProfileManager {
       }
 
       if (!song) {
-        const target = this.channelDescCid ?? channelId;
-
-        if (target === 0n) return;
-
-        const httpQuery = this.tsClient.getHttpQuery();
-
-        if (httpQuery) {
-          const result = await httpQuery.channelEdit(Number(target), {
-            channel_description: "",
-          });
-
-          this.logger.info(
-            {
-              status: result.status,
-              cid: target.toString(),
-            },
-            "Channel description cleared",
-          );
-        } else {
-          await this.withTimeout(
-            this.tsClient.execCommand(
-              `channeledit cid=${target} channel_description=`,
-            ),
-            5000,
-          );
-
-          this.logger.info(
-            { cid: target.toString() },
-            "Channel description cleared",
-          );
-        }
-
-        this.channelDescCid = null;
+        if (channelId <= 0n) return;
+        if (await this.writeChannelDescription(channelId, "", context)) this.channelDescCid = null;
         return;
       }
 
-      if (channelId === 0n) return;
+      if (channelId <= 0n) return;
 
       const lines = [
         `♪ 正在播放: ${song.name} - ${song.artist}`,
@@ -525,40 +517,41 @@ export class BotProfileManager {
       // HTTP Query uses a normal JSON string, so use real newlines here.
       const desc = lines.join("\n");
 
-      const httpQuery = this.tsClient.getHttpQuery();
-
-      if (httpQuery) {
-        const result = await httpQuery.channelEdit(Number(channelId), {
-          channel_description: desc,
-        });
-
-        this.logger.info(
-          {
-            status: result.status,
-            cid: channelId.toString(),
-          },
-          "Channel description updated",
-        );
-      } else {
-        await this.withTimeout(
-          this.tsClient.execCommand(
-            `channeledit cid=${channelId} channel_description=${escapeTS3(desc)}`,
-          ),
-          5000,
-        );
-
-        this.logger.info(
-          { cid: channelId.toString() },
-          "Channel description updated",
-        );
-      }
-
-      this.channelDescCid = channelId;
+      if (await this.writeChannelDescription(channelId, desc, context)) this.channelDescCid = channelId;
     } catch (err) {
-      this.handleFeatureError("channelDesc", err);
+      if (this.isCurrentChannelUpdate(context)) this.handleFeatureError("channelDesc", err);
     }
   }
 
+  /** Both move cleanup and ordinary writes use the same checked transport. */
+  private async writeChannelDescription(
+    channelId: bigint,
+    description: string,
+    context: ProfileUpdateContext,
+  ): Promise<boolean> {
+    if (!this.isCurrentChannelUpdate(context)) return false;
+    let status: number | undefined;
+    if (context.httpQuery) {
+      const result = await context.httpQuery.channelEdit(Number(channelId), {
+        channel_description: description,
+      });
+      status = result.status;
+    } else {
+      await this.withTimeout(
+        this.tsClient.execCommand(
+          `channeledit cid=${channelId} channel_description=${escapeTS3(description)}`,
+        ),
+        5000,
+      );
+    }
+    if (!this.isCurrentChannelUpdate(context)) return false;
+    this.logger.info(
+      { status, cid: channelId.toString() },
+      description ? "Channel description updated" : "Channel description cleared",
+    );
+    return true;
+  }
+
   private async sendNowPlayingMessage(song: QueuedSong): Promise<void> {
     if (!this.config.nowPlayingMsgEnabled || this.permDenied.nowPlayingMsg) return;
     try {
@@ -571,6 +564,25 @@ export class BotProfileManager {
 
   // --- Helpers ---
 
+  private createUpdateContext(): ProfileUpdateContext {
+    return {
+      generation: this.generation,
+      channelGeneration: this.channelGeneration,
+      clientId: this.tsClient.getClientId(),
+      httpQuery: this.tsClient.getHttpQuery(),
+    };
+  }
+
+  private isCurrentUpdate(context: ProfileUpdateContext): boolean {
+    return context.generation === this.generation &&
+      context.clientId === this.tsClient.getClientId() &&
+      context.httpQuery === this.tsClient.getHttpQuery();
+  }
+
+  private isCurrentChannelUpdate(context: ProfileUpdateContext): boolean {
+    return this.isCurrentUpdate(context) && context.channelGeneration === this.channelGeneration;
+  }
+
   /**
    * Append CDN resize parameters to get a thumbnail suitable for TS3 avatars.
    * NetEase and QQ Music CDNs support URL-based image resizing.

src/ts-protocol/http-query.ts

@@ -167,7 +167,12 @@ export class TS6HttpQuery {
 
   /** List clients on a virtual server */
   async clientList(sid = 1): Promise<HttpQueryResult> {
-    return this.request("GET", `/1/clientlist?sid=${sid}`);
+    const path = `/1/clientlist?sid=${sid}`;
+    const result = await this.request("GET", path);
+    if (result.status < 200 || result.status >= 300) {
+      throw new HttpQueryError(path, result.status, result.body);
+    }
+    return result;
   }
 
   /** List channels on a virtual server */