From 312d19c0d197754c73554bc8aef278984c9a2e96 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Fri, 10 Apr 2026 21:37:20 +0900 Subject: [PATCH] refactor: drop legacy env alias reads --- src/channels/discord.test.ts | 15 +----------- src/channels/discord.ts | 47 ------------------------------------ src/config.test.ts | 27 +++++++++++++++++++++ src/config/load-config.ts | 38 +++++++++++++++++++++++++---- 4 files changed, 61 insertions(+), 66 deletions(-) diff --git a/src/channels/discord.test.ts b/src/channels/discord.test.ts index 9350eb1..1171424 100644 --- a/src/channels/discord.test.ts +++ b/src/channels/discord.test.ts @@ -225,24 +225,11 @@ describe('DiscordChannel', () => { }); describe('channel registration', () => { - it('warns when only legacy owner token names are configured', () => { + it('warns when the canonical owner token is not configured', () => { const ownerFactory = registeredChannelFactories.get('discord'); - vi.mocked(getEnv).mockImplementation((key: string) => { - if (key === 'DISCORD_BOT_TOKEN') return 'legacy-owner-token'; - return undefined; - }); - expect(ownerFactory).toBeTypeOf('function'); expect(ownerFactory?.(createTestOpts() as any)).toBeNull(); - expect(logger.warn).toHaveBeenCalledWith( - { - role: 'owner', - canonicalKey: 'DISCORD_OWNER_BOT_TOKEN', - legacyKeys: ['DISCORD_BOT_TOKEN'], - }, - 'Discord: legacy bot token names detected; rename them to canonical role-based names', - ); expect(logger.warn).toHaveBeenCalledWith( 'Discord: DISCORD_OWNER_BOT_TOKEN not set', ); diff --git a/src/channels/discord.ts b/src/channels/discord.ts index ed3e71a..75bcaad 100644 --- a/src/channels/discord.ts +++ b/src/channels/discord.ts @@ -30,38 +30,6 @@ const DISCORD_ARBITER_CHANNEL = 'discord-arbiter'; const DISCORD_OWNER_TOKEN_KEY = 'DISCORD_OWNER_BOT_TOKEN'; const DISCORD_REVIEWER_TOKEN_KEY = 'DISCORD_REVIEWER_BOT_TOKEN'; const DISCORD_ARBITER_TOKEN_KEY = 'DISCORD_ARBITER_BOT_TOKEN'; -const DISCORD_OWNER_LEGACY_TOKEN_KEYS = [ - 'DISCORD_BOT_TOKEN', - 'DISCORD_CLAUDE_BOT_TOKEN', -]; -const DISCORD_REVIEWER_LEGACY_TOKEN_KEYS = [ - 'DISCORD_CODEX_BOT_TOKEN', - 'DISCORD_CODEX_MAIN_BOT_TOKEN', -]; -const DISCORD_ARBITER_LEGACY_TOKEN_KEYS = [ - 'DISCORD_REVIEW_BOT_TOKEN', - 'DISCORD_CODEX_REVIEW_BOT_TOKEN', -]; - -function warnIfLegacyDiscordTokenConfigured( - canonicalKey: string, - legacyKeys: string[], - role: 'owner' | 'reviewer' | 'arbiter', -): void { - if (getEnv(canonicalKey)) return; - - const configuredLegacyKeys = legacyKeys.filter((key) => !!getEnv(key)); - if (configuredLegacyKeys.length === 0) return; - - logger.warn( - { - role, - canonicalKey, - legacyKeys: configuredLegacyKeys, - }, - 'Discord: legacy bot token names detected; rename them to canonical role-based names', - ); -} /** * Download a Discord attachment to local disk. @@ -835,11 +803,6 @@ export class DiscordChannel implements Channel { } registerChannel(DISCORD_OWNER_CHANNEL, (opts: ChannelOpts) => { - warnIfLegacyDiscordTokenConfigured( - DISCORD_OWNER_TOKEN_KEY, - DISCORD_OWNER_LEGACY_TOKEN_KEYS, - 'owner', - ); const token = getEnv(DISCORD_OWNER_TOKEN_KEY) || ''; if (!token) { logger.warn('Discord: DISCORD_OWNER_BOT_TOKEN not set'); @@ -849,11 +812,6 @@ registerChannel(DISCORD_OWNER_CHANNEL, (opts: ChannelOpts) => { }); registerChannel(DISCORD_REVIEWER_CHANNEL, (opts: ChannelOpts) => { - warnIfLegacyDiscordTokenConfigured( - DISCORD_REVIEWER_TOKEN_KEY, - DISCORD_REVIEWER_LEGACY_TOKEN_KEYS, - 'reviewer', - ); const ownerToken = getEnv(DISCORD_OWNER_TOKEN_KEY) || ''; const token = getEnv(DISCORD_REVIEWER_TOKEN_KEY) || ''; if (!token) return null; @@ -874,11 +832,6 @@ registerChannel(DISCORD_REVIEWER_CHANNEL, (opts: ChannelOpts) => { }); registerChannel(DISCORD_ARBITER_CHANNEL, (opts: ChannelOpts) => { - warnIfLegacyDiscordTokenConfigured( - DISCORD_ARBITER_TOKEN_KEY, - DISCORD_ARBITER_LEGACY_TOKEN_KEYS, - 'arbiter', - ); const ownerToken = getEnv(DISCORD_OWNER_TOKEN_KEY) || ''; const reviewerToken = getEnv(DISCORD_REVIEWER_TOKEN_KEY) || ''; const token = getEnv(DISCORD_ARBITER_TOKEN_KEY) || ''; diff --git a/src/config.test.ts b/src/config.test.ts index 85ad9c8..47a6d56 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -23,6 +23,13 @@ describe('config/env loading', () => { delete process.env.CODEX_MODEL; delete process.env.STATUS_CHANNEL_ID; delete process.env.TZ; + delete process.env.DISCORD_BOT_TOKEN; + delete process.env.DISCORD_CLAUDE_BOT_TOKEN; + delete process.env.DISCORD_CODEX_BOT_TOKEN; + delete process.env.DISCORD_CODEX_MAIN_BOT_TOKEN; + delete process.env.DISCORD_REVIEW_BOT_TOKEN; + delete process.env.DISCORD_CODEX_REVIEW_BOT_TOKEN; + delete process.env.SESSION_COMMAND_USER_IDS; vi.resetModules(); }); @@ -94,4 +101,24 @@ describe('config/env loading', () => { expect(config.STATUS_CHANNEL_ID).toBe('status-room'); expect(config.TIMEZONE).toBe('UTC'); }); + + it('fails fast when a legacy Discord token alias is configured', async () => { + process.env.DISCORD_BOT_TOKEN = 'legacy-owner-token'; + + const { loadConfig } = await import('./config/load-config.js'); + + expect(() => loadConfig()).toThrow( + /Legacy env aliases are no longer supported; rename them to canonical keys \(DISCORD_BOT_TOKEN -> DISCORD_OWNER_BOT_TOKEN\)/, + ); + }); + + it('fails fast when SESSION_COMMAND_USER_IDS is configured', async () => { + process.env.SESSION_COMMAND_USER_IDS = 'alice,bob'; + + const { loadConfig } = await import('./config/load-config.js'); + + expect(() => loadConfig()).toThrow( + /Legacy env aliases are no longer supported; rename them to canonical keys \(SESSION_COMMAND_USER_IDS -> SESSION_COMMAND_ALLOWED_SENDERS\)/, + ); + }); }); diff --git a/src/config/load-config.ts b/src/config/load-config.ts index 36d07a6..5e3e9f0 100644 --- a/src/config/load-config.ts +++ b/src/config/load-config.ts @@ -7,6 +7,16 @@ import { getEnv } from '../env.js'; import type { AppConfig, RoleModelConfig } from './schema.js'; +const LEGACY_ENV_ALIAS_MAP = { + DISCORD_BOT_TOKEN: 'DISCORD_OWNER_BOT_TOKEN', + DISCORD_CLAUDE_BOT_TOKEN: 'DISCORD_OWNER_BOT_TOKEN', + DISCORD_CODEX_BOT_TOKEN: 'DISCORD_REVIEWER_BOT_TOKEN', + DISCORD_CODEX_MAIN_BOT_TOKEN: 'DISCORD_REVIEWER_BOT_TOKEN', + DISCORD_REVIEW_BOT_TOKEN: 'DISCORD_ARBITER_BOT_TOKEN', + DISCORD_CODEX_REVIEW_BOT_TOKEN: 'DISCORD_ARBITER_BOT_TOKEN', + SESSION_COMMAND_USER_IDS: 'SESSION_COMMAND_ALLOWED_SENDERS', +} as const; + function readText(key: string): string | undefined { return getEnv(key); } @@ -100,7 +110,29 @@ function normalizeServiceId( return serviceId === 'codex' ? codexMainServiceId : serviceId; } +function assertNoLegacyEnvAliasesConfigured(): void { + const configuredAliases = Object.entries(LEGACY_ENV_ALIAS_MAP).filter( + ([legacyKey]) => { + const value = getEnv(legacyKey); + return value != null && value !== ''; + }, + ); + + if (configuredAliases.length === 0) { + return; + } + + const aliasList = configuredAliases + .map(([legacyKey, canonicalKey]) => `${legacyKey} -> ${canonicalKey}`) + .join(', '); + throw new Error( + `Legacy env aliases are no longer supported; rename them to canonical keys (${aliasList})`, + ); +} + export function loadConfig(): AppConfig { + assertNoLegacyEnvAliasesConfigured(); + const assistantName = readText('ASSISTANT_NAME') ?? 'Andy'; const assistantSlug = assistantName.trim().toLowerCase(); @@ -217,11 +249,7 @@ export function loadConfig(): AppConfig { }, sessionCommands: { allowedSenders: new Set( - ( - readText('SESSION_COMMAND_ALLOWED_SENDERS') ?? - readText('SESSION_COMMAND_USER_IDS') ?? - '' - ) + (readText('SESSION_COMMAND_ALLOWED_SENDERS') ?? '') .split(',') .map((value) => value.trim()) .filter(Boolean),