code review fixes

This commit is contained in:
jaydenfyi 2026-01-25 10:41:07 +08:00
parent 1d12c9e91f
commit 5af8dddcc7
6 changed files with 40 additions and 20 deletions

View File

@ -79,10 +79,7 @@ export async function removeClientManager(accountId: string): Promise<void> {
} }
// Disconnect the client manager // Disconnect the client manager
await entry.manager.disconnect({ await entry.manager.disconnectAll();
username: accountId,
channel: accountId,
} as Parameters<typeof entry.manager.disconnect>[0]);
// Remove from registry // Remove from registry
registry.delete(accountId); registry.delete(accountId);

View File

@ -16,6 +16,7 @@ import { probeTwitch } from "./probe.js";
import { resolveTwitchTargets } from "./resolver.js"; import { resolveTwitchTargets } from "./resolver.js";
import { collectTwitchStatusIssues } from "./status.js"; import { collectTwitchStatusIssues } from "./status.js";
import { removeClientManager } from "./client-manager-registry.js"; import { removeClientManager } from "./client-manager-registry.js";
import { resolveTwitchToken } from "./token.js";
import type { import type {
ChannelAccountSnapshot, ChannelAccountSnapshot,
ChannelCapabilities, ChannelCapabilities,
@ -30,8 +31,13 @@ import type {
/** /**
* Check if an account is properly configured. * Check if an account is properly configured.
*/ */
function isConfigured(account: TwitchAccountConfig | null | undefined): boolean { function isConfigured(
return Boolean(account?.token && account?.username && account?.clientId); account: TwitchAccountConfig | null | undefined,
cfg: ClawdbotConfig,
accountId: string,
): boolean {
const tokenResolution = resolveTwitchToken(cfg, { accountId });
return Boolean(account?.username && account?.clientId && tokenResolution.token);
} }
/** /**
@ -105,7 +111,7 @@ export const twitchPlugin: ChannelPlugin<TwitchAccountConfig> = {
/** Check if an account is configured */ /** Check if an account is configured */
isConfigured: (_account: unknown, cfg: ClawdbotConfig): boolean => { isConfigured: (_account: unknown, cfg: ClawdbotConfig): boolean => {
const account = getAccountConfig(cfg, DEFAULT_ACCOUNT_ID); const account = getAccountConfig(cfg, DEFAULT_ACCOUNT_ID);
return isConfigured(account); return isConfigured(account, cfg, DEFAULT_ACCOUNT_ID);
}, },
/** Check if an account is enabled */ /** Check if an account is enabled */
@ -115,7 +121,7 @@ export const twitchPlugin: ChannelPlugin<TwitchAccountConfig> = {
describeAccount: (account: TwitchAccountConfig | undefined) => ({ describeAccount: (account: TwitchAccountConfig | undefined) => ({
accountId: DEFAULT_ACCOUNT_ID, accountId: DEFAULT_ACCOUNT_ID,
enabled: account?.enabled !== false, enabled: account?.enabled !== false,
configured: account ? isConfigured(account) : false, configured: account ? isConfigured(account, cfg, DEFAULT_ACCOUNT_ID) : false,
}), }),
}, },
@ -205,10 +211,13 @@ export const twitchPlugin: ChannelPlugin<TwitchAccountConfig> = {
runtime?: ChannelAccountSnapshot; runtime?: ChannelAccountSnapshot;
probe?: unknown; probe?: unknown;
}): ChannelAccountSnapshot => { }): ChannelAccountSnapshot => {
const accountMap = cfg.channels?.twitch?.accounts ?? {};
const resolvedAccountId =
Object.entries(accountMap).find(([, value]) => value === account)?.[0] ?? DEFAULT_ACCOUNT_ID;
return { return {
accountId: DEFAULT_ACCOUNT_ID, accountId: DEFAULT_ACCOUNT_ID,
enabled: account?.enabled !== false, enabled: account?.enabled !== false,
configured: isConfigured(account), configured: isConfigured(account, cfg, resolvedAccountId),
running: runtime?.running ?? false, running: runtime?.running ?? false,
lastStartAt: runtime?.lastStartAt ?? null, lastStartAt: runtime?.lastStartAt ?? null,
lastStopAt: runtime?.lastStopAt ?? null, lastStopAt: runtime?.lastStopAt ?? null,

View File

@ -1,6 +1,7 @@
import { StaticAuthProvider } from "@twurple/auth"; import { StaticAuthProvider } from "@twurple/auth";
import { ChatClient } from "@twurple/chat"; import { ChatClient } from "@twurple/chat";
import type { TwitchAccountConfig } from "./types.js"; import type { TwitchAccountConfig } from "./types.js";
import { normalizeToken } from "./utils/twitch.js";
/** /**
* Result of probing a Twitch account * Result of probing a Twitch account
@ -35,7 +36,7 @@ export async function probeTwitch(
}; };
} }
const rawToken = account.token.trim(); const rawToken = normalizeToken(account.token.trim());
let client: ChatClient | undefined; let client: ChatClient | undefined;

View File

@ -8,6 +8,7 @@
import { DEFAULT_ACCOUNT_ID, getAccountConfig } from "./config.js"; import { DEFAULT_ACCOUNT_ID, getAccountConfig } from "./config.js";
import { getClientManager as getRegistryClientManager } from "./client-manager-registry.js"; import { getClientManager as getRegistryClientManager } from "./client-manager-registry.js";
import type { ClawdbotConfig } from "clawdbot/plugin-sdk"; import type { ClawdbotConfig } from "clawdbot/plugin-sdk";
import { resolveTwitchToken } from "./token.js";
import { stripMarkdownForTwitch } from "./utils/markdown.js"; import { stripMarkdownForTwitch } from "./utils/markdown.js";
import { generateMessageId, isAccountConfigured, normalizeTwitchChannel } from "./utils/twitch.js"; import { generateMessageId, isAccountConfigured, normalizeTwitchChannel } from "./utils/twitch.js";
@ -66,11 +67,14 @@ export async function sendMessageTwitchInternal(
}; };
} }
if (!isAccountConfigured(account)) { const tokenResolution = resolveTwitchToken(cfg, { accountId });
if (!isAccountConfigured(account, tokenResolution.token)) {
return { return {
ok: false, ok: false,
messageId: generateMessageId(), messageId: generateMessageId(),
error: `Account ${accountId} is not properly configured. Required: username, token, clientId`, error:
`Account ${accountId} is not properly configured. ` +
"Required: username, clientId, and token (config or env for default account).",
}; };
} }

View File

@ -6,6 +6,7 @@
import { getAccountConfig } from "./config.js"; import { getAccountConfig } from "./config.js";
import type { ChannelAccountSnapshot, ChannelStatusIssue } from "./types.js"; import type { ChannelAccountSnapshot, ChannelStatusIssue } from "./types.js";
import { resolveTwitchToken } from "./token.js";
import { isAccountConfigured } from "./utils/twitch.js"; import { isAccountConfigured } from "./utils/twitch.js";
/** /**
@ -39,9 +40,10 @@ export function collectTwitchStatusIssues(
// Get full account config if available // Get full account config if available
let account: ReturnType<typeof getAccountConfig> | null = null; let account: ReturnType<typeof getAccountConfig> | null = null;
let cfg: Parameters<typeof resolveTwitchToken>[0] | undefined;
if (getCfg) { if (getCfg) {
try { try {
const cfg = getCfg() as { cfg = getCfg() as {
channels?: { twitch?: { accounts?: Record<string, unknown> } }; channels?: { twitch?: { accounts?: Record<string, unknown> } };
}; };
account = getAccountConfig(cfg, accountId); account = getAccountConfig(cfg, accountId);
@ -86,7 +88,10 @@ export function collectTwitchStatusIssues(
} }
// Checks that require account config // Checks that require account config
if (account && isAccountConfigured(account)) { const tokenResolution = cfg
? resolveTwitchToken(cfg as Parameters<typeof resolveTwitchToken>[0], { accountId })
: { token: "", source: "none" };
if (account && isAccountConfigured(account, tokenResolution.token)) {
// Check 4: Token format warning (normalized, but may indicate config issue) // Check 4: Token format warning (normalized, but may indicate config issue)
if (account.token?.startsWith("oauth:")) { if (account.token?.startsWith("oauth:")) {
issues.push({ issues.push({

View File

@ -65,10 +65,14 @@ export function normalizeToken(token: string): string {
* @param account - The Twitch account config to check * @param account - The Twitch account config to check
* @returns true if the account has required credentials * @returns true if the account has required credentials
*/ */
export function isAccountConfigured(account: { export function isAccountConfigured(
account: {
username?: string; username?: string;
token?: string; token?: string;
clientId?: string; clientId?: string;
}): boolean { },
return Boolean(account?.username && account?.token && account?.clientId); resolvedToken?: string | null,
): boolean {
const token = resolvedToken ?? account?.token;
return Boolean(account?.username && token && account?.clientId);
} }