diff --git a/app/eslint.config.js b/app/eslint.config.js index 2395dc1caa..4ea65fcfc8 100644 --- a/app/eslint.config.js +++ b/app/eslint.config.js @@ -228,6 +228,36 @@ export default [ }, }, + // Frontend config is centralized in src/utils/config.ts (AGENTS.md). A direct + // import.meta.env read elsewhere bypasses the derived values that file owns -- + // IS_DEV_LIKE exists precisely because import.meta.env.DEV is false under the + // E2E harness (vite build --mode development) -- and cannot be stubbed once the + // module graph has loaded, which is why the loopback OAuth test reads config + // instead. The rule was documented but unenforced, and had already drifted. + { + files: ['src/**/*.ts', 'src/**/*.tsx'], + ignores: [ + 'src/utils/config.ts', + 'src/test/**', + 'src/**/__tests__/**', + 'src/**/*.test.ts', + 'src/**/*.test.tsx', + ], + rules: { + 'no-restricted-syntax': [ + 'error', + { + // Pin the meta-property to `import.meta` -- `new.target` is a MetaProperty + // too -- and match both `import.meta.env` and `import.meta['env']`. + selector: + 'MemberExpression[object.type="MetaProperty"][object.meta.name="import"][object.property.name="meta"]:matches([computed=false][property.name="env"], [computed=true][property.value="env"])', + message: + 'Read frontend config from src/utils/config.ts (IS_DEV, IS_DEV_LIKE, ...) instead of import.meta.env directly; add the value there if it is missing.', + }, + ], + }, + }, + // React files configuration { files: ['src/**/*.jsx', 'src/**/*.tsx'], diff --git a/app/src/components/layout/TwoPanelLayout.tsx b/app/src/components/layout/TwoPanelLayout.tsx index 3bffa0ca3a..ef0bafbbcf 100644 --- a/app/src/components/layout/TwoPanelLayout.tsx +++ b/app/src/components/layout/TwoPanelLayout.tsx @@ -12,13 +12,14 @@ import { setSidebarWidth, toggleSidebar, } from '../../store/layoutSlice'; +import { IS_DEV } from '../../utils/config'; import { Button } from '../ui'; import { clampWidth, useResizableDivider } from './useResizableDivider'; const namespace = 'two-panel-layout'; function debug(message: string, payload?: Record) { - if (import.meta.env.DEV) { + if (IS_DEV) { console.debug(`[${namespace}] ${message}`, payload ?? {}); } } diff --git a/app/src/services/socketService.ts b/app/src/services/socketService.ts index 06673892f4..c3e9d16500 100644 --- a/app/src/services/socketService.ts +++ b/app/src/services/socketService.ts @@ -10,7 +10,7 @@ import { setBackend } from '../store/connectivitySlice'; import { resetForUser, setSocketIdForUser, setStatusForUser } from '../store/socketSlice'; import type { ChannelAuthMode, ChannelConnectionStatus, ChannelType } from '../types/channels'; import type { UserErrorScope } from '../types/userError'; -import { IS_DEV } from '../utils/config'; +import { IS_DEV, IS_TEST } from '../utils/config'; import { createSafeLogData, sanitizeError } from '../utils/sanitize'; import { getCoreRpcToken, getCoreRpcUrl } from './coreRpcClient'; import { createCoreSocket } from './coreSocket'; @@ -26,11 +26,11 @@ const socketWarn = debug('socket:warn'); const socketError = debug('socket:error'); // Enable socket logging in development by default — but never under test. -// `IS_DEV` is truthy in vitest, so without the MODE guard this force-enable +// `IS_DEV` is truthy in vitest, so without the `IS_TEST` guard this force-enable // floods every test file that imports this service (measured: 412 lines / // 46KB of `flow:approval_request` listener churn in a single run), inflating // runtime enough to push suites past the runner's foreground timeout. -if (IS_DEV && import.meta.env.MODE !== 'test') { +if (IS_DEV && !IS_TEST) { debug.enable('socket*'); } diff --git a/app/src/test/eslintCentralizedConfigRule.test.ts b/app/src/test/eslintCentralizedConfigRule.test.ts new file mode 100644 index 0000000000..b93dc1ba87 --- /dev/null +++ b/app/src/test/eslintCentralizedConfigRule.test.ts @@ -0,0 +1,70 @@ +import tsparser from '@typescript-eslint/parser'; +import { Linter } from 'eslint'; +import { describe, expect, it } from 'vitest'; + +// eslint.config.js is plain JS and ships no declarations; importing the real +// file is the point of this test, so the shape is asserted below at runtime. +// @ts-expect-error -- untyped JS module +import eslintConfig from '../../eslint.config.js'; + +/** + * Guards the `no-restricted-syntax` selector that keeps frontend config reads + * funnelled through `src/utils/config.ts`. The selector is easy to get subtly + * wrong in both directions -- `new.target` is a MetaProperty just like + * `import.meta`, and computed access stores the key on `property.value` rather + * than `property.name` -- so the exact boundary is pinned here. + * + * The selector and the ignore list are read out of the real `eslint.config.js` + * rather than restated, so these assertions cannot drift away from what ships. + * Linting runs through a bare `Linter` because the repo config is type-aware + * and `parserOptions.project` rejects the synthetic file used for the probes. + */ + +const configBlock = (eslintConfig as Linter.Config[]).find( + block => block.rules?.['no-restricted-syntax'] +); + +const [, restriction] = (configBlock?.rules?.['no-restricted-syntax'] ?? []) as [ + string, + { selector: string; message: string }, +]; + +const linter = new Linter(); + +function lint(code: string) { + const messages = linter.verify(code, { + languageOptions: { parser: tsparser, ecmaVersion: 'latest', sourceType: 'module' }, + rules: { 'no-restricted-syntax': ['error', restriction] }, + }); + // A parse failure would otherwise read as "the rule did not fire". + expect(messages.filter(message => message.fatal)).toEqual([]); + return messages; +} + +describe('centralized-frontend-config lint rule', () => { + it('is wired into the shipped config', () => { + expect(restriction?.selector).toBeTypeOf('string'); + expect(configBlock?.ignores).toContain('src/utils/config.ts'); + expect(configBlock?.ignores).toContain('src/test/**'); + }); + + it('rejects dotted import.meta.env access', () => { + expect(lint('export const a = import.meta.env.DEV;')).toHaveLength(1); + }); + + it('rejects computed import.meta["env"] access', () => { + expect(lint("export const a = import.meta['env'].DEV;")).toHaveLength(1); + }); + + it('allows other import.meta properties', () => { + expect(lint('export const a = import.meta.url;')).toHaveLength(0); + }); + + it('allows new.target.env, which is a different meta-property', () => { + expect(lint('export function G() { return new.target.env; }')).toHaveLength(0); + }); + + it('allows a plain object property named env', () => { + expect(lint('const o = { env: 1 }; export const a = o.env;')).toHaveLength(0); + }); +}); diff --git a/app/src/test/setup.ts b/app/src/test/setup.ts index 9f35c1b4bc..a9afbc1035 100644 --- a/app/src/test/setup.ts +++ b/app/src/test/setup.ts @@ -255,6 +255,7 @@ vi.mock('../utils/config', () => ({ CORE_RPC_TIMEOUT_MS: 30_000, IS_DEV: true, IS_DEV_LIKE: true, + IS_TEST: true, IS_PROD: false, E2E_DEFAULT_CORE_MODE: '', E2E_RESTART_APP_AS_RELOAD: false, diff --git a/app/src/utils/config.ts b/app/src/utils/config.ts index 1da0a7f16b..caf548de54 100644 --- a/app/src/utils/config.ts +++ b/app/src/utils/config.ts @@ -77,6 +77,13 @@ export const E2E_DEFAULT_CORE_MODE = */ export const IS_DEV_LIKE = IS_DEV || import.meta.env.MODE === 'development'; +/** + * True under vitest. Distinct from `IS_DEV`, which is *also* true there — so a + * dev-only default that must stay off in tests needs both, and reading `MODE` + * at the call site is exactly the direct access this module exists to absorb. + */ +export const IS_TEST = import.meta.env.MODE === 'test'; + /** Dev only: skip `.skip_onboarding` workspace check and ignore onboarded state so `/onboarding` always shows. Set `VITE_DEV_FORCE_ONBOARDING=true` in `.env.local`. */ export const DEV_FORCE_ONBOARDING = import.meta.env.DEV && import.meta.env.VITE_DEV_FORCE_ONBOARDING === 'true';