From 69dd871404c6220f85524e8455e1b791ad1c2201 Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Wed, 4 Mar 2026 20:27:42 +0900 Subject: [PATCH] =?UTF-8?q?refactor:=20observability=20=E3=82=92=20logging?= =?UTF-8?q?=20=E3=81=AB=E5=86=8D=E7=B7=A8=E6=88=90=E3=81=97=E8=A8=AD?= =?UTF-8?q?=E5=AE=9A=E6=A7=8B=E9=80=A0=E3=82=92=E4=BD=93=E7=B3=BB=E5=8C=96?= =?UTF-8?q?=20(#466)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * takt: refactor-logging-config * fix: resolve merge conflicts * chore: trigger CI --------- Co-authored-by: github-actions[bot] Co-authored-by: Claude Sonnet 4.6 --- CLAUDE.md | 15 +- builtins/en/config.yaml | 8 +- builtins/ja/config.yaml | 6 +- docs/configuration.ja.md | 22 +- docs/configuration.md | 25 +- src/__tests__/config-env-overrides.test.ts | 138 +++++++++- .../config-modularity-boundary.test.ts | 10 + src/__tests__/config.test.ts | 76 ++++++ src/__tests__/globalConfig-defaults.test.ts | 241 +++++++++++++++++- src/__tests__/globalConfig.test.ts | 16 ++ .../globalConfigLegacyMigration.test.ts | 30 +++ src/__tests__/models-api-boundary.test.ts | 24 ++ src/__tests__/models.test.ts | 59 ++++- .../pieceExecution-ask-user-question.test.ts | 2 +- .../pieceExecution-debug-prompts.test.ts | 2 +- .../pieceExecution-session-loading.test.ts | 17 +- src/__tests__/providerEventLogger.test.ts | 38 ++- ...resolveConfigValue-no-defaultValue.test.ts | 20 ++ src/core/models/index.ts | 2 +- src/core/models/persisted-global-config.ts | 12 +- src/core/models/schemas.ts | 7 +- src/core/models/types.ts | 2 +- src/features/tasks/execute/pieceExecution.ts | 29 +-- .../tasks/execute/pieceExecutionUtils.ts | 20 ++ src/infra/config/env/config-env-overrides.ts | 31 ++- src/infra/config/global/globalConfigCore.ts | 175 +++---------- .../global/globalConfigLegacyMigration.ts | 99 +++++++ .../config/global/globalConfigSerializer.ts | 149 +++++++++++ src/infra/config/project/resolvedSettings.ts | 4 +- src/infra/config/resolveConfigValue.ts | 22 ++ src/shared/utils/providerEventLogger.ts | 14 +- 31 files changed, 1089 insertions(+), 226 deletions(-) create mode 100644 src/__tests__/globalConfigLegacyMigration.test.ts create mode 100644 src/__tests__/models-api-boundary.test.ts create mode 100644 src/features/tasks/execute/pieceExecutionUtils.ts create mode 100644 src/infra/config/global/globalConfigLegacyMigration.ts create mode 100644 src/infra/config/global/globalConfigSerializer.ts diff --git a/CLAUDE.md b/CLAUDE.md index 44aba48..82df6ce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -601,17 +601,24 @@ Implemented in `src/core/runtime/runtime-environment.ts`. ## Debugging -**Debug logging:** Set `debug_enabled: true` in `~/.takt/config.yaml` or create a `.takt/debug.yaml` file: +**Debug logging:** Set `logging.debug: true` in `~/.takt/config.yaml`: ```yaml -enabled: true +logging: + debug: true ``` -Debug logs are written to `.takt/logs/debug.log` (ndjson format). Log levels: `debug`, `info`, `warn`, `error`. +Debug logs are written to `.takt/runs/debug-{timestamp}/logs/` in NDJSON format. Log levels: `debug`, `info`, `warn`, `error`. -**Verbose mode:** Create `.takt/verbose` file (empty file) to enable verbose console output. This automatically enables debug logging and sets log level to `debug`. +**Verbose mode:** Set `verbose: true` in `~/.takt/config.yaml` or `TAKT_VERBOSE=true` to enable verbose console output. This enables `logging.debug`, `logging.trace`, and sets `logging.level` to `debug`. **Session logs:** All piece executions are logged to `.takt/logs/{sessionId}.jsonl`. Use `tail -f .takt/logs/{sessionId}.jsonl` to monitor in real-time. +**Environment variables:** + +- `TAKT_LOGGING_LEVEL=info` +- `TAKT_LOGGING_PROVIDER_EVENTS=true` +- `TAKT_VERBOSE=true` + **Testing with mocks:** Use `--provider mock` to test pieces without calling real AI APIs. Mock responses are deterministic and configurable via test fixtures. ## Testing Notes diff --git a/builtins/en/config.yaml b/builtins/en/config.yaml index 6210f0e..553d390 100644 --- a/builtins/en/config.yaml +++ b/builtins/en/config.yaml @@ -20,8 +20,12 @@ language: en # UI language: en | ja # piece_abort: true # run_complete: true # run_abort: true -# observability: -# provider_events: false # Persist provider stream events +# verbose: false # Shortcut: enable trace/debug and set logging.level=debug +# logging: +# level: info # Log level for console and file output +# trace: true # Generate human-readable execution trace report (trace.md) +# debug: false # Enable debug.log + prompts.jsonl +# provider_events: false # Persist provider stream events # Credentials (environment variables take priority) # anthropic_api_key: "sk-ant-..." # Claude API key diff --git a/builtins/ja/config.yaml b/builtins/ja/config.yaml index 6e0c99f..5a179bd 100644 --- a/builtins/ja/config.yaml +++ b/builtins/ja/config.yaml @@ -20,7 +20,11 @@ language: ja # 表示言語: ja | en # piece_abort: true # run_complete: true # run_abort: true -# observability: +# verbose: false # ショートカット: trace/debug有効化 + logging.level=debug +# logging: +# level: info # ログレベル: debug | info | warn | error +# trace: true # trace.md 実行レポート生成 +# debug: false # debug.log + prompts.jsonl を有効化 # provider_events: false # providerイベントログを記録 # 認証情報（環境変数優先） diff --git a/docs/configuration.ja.md b/docs/configuration.ja.md index dd53a9f..a64c20d 100644 --- a/docs/configuration.ja.md +++ b/docs/configuration.ja.md @@ -12,7 +12,8 @@ # ~/.takt/config.yaml language: en # UI 言語: 'en' または 'ja' default_piece: default # 新規プロジェクトのデフォルト piece -log_level: info # ログレベル: debug, info, warn, error +logging: + level: info # ログレベル: debug, info, warn, error provider: claude # デフォルト provider: claude, codex, opencode, cursor, または copilot model: sonnet # デフォルトモデル（省略可、provider にそのまま渡される） branch_name_strategy: romaji # ブランチ名生成方式: 'romaji'（高速）または 'ai'（低速） @@ -92,7 +93,7 @@ interactive_preview_movements: 3 # インタラクティブモードでの move |-----------|------|---------|------| | `language` | `"en"` \| `"ja"` | `"en"` | UI 言語 | | `default_piece` | string | `"default"` | 新規プロジェクトのデフォルト piece | -| `log_level` | `"debug"` \| `"info"` \| `"warn"` \| `"error"` | `"info"` | ログレベル | +| `logging.level` | `"debug"` \| `"info"` \| `"warn"` \| `"error"` | `"info"` | ログレベル | | `provider` | `"claude"` \| `"codex"` \| `"opencode"` \| `"cursor"` \| `"copilot"` | `"claude"` | デフォルト AI provider | | `model` | string | - | デフォルトモデル名（provider にそのまま渡される） | | `branch_name_strategy` | `"romaji"` \| `"ai"` | `"romaji"` | ブランチ名生成方式 | @@ -434,22 +435,27 @@ pipeline: ### デバッグログ -`~/.takt/config.yaml` で `debug_enabled: true` を設定するか、`.takt/debug.yaml` ファイルを作成してデバッグログを有効化できます。 +`~/.takt/config.yaml` で `logging.debug: true` を設定してデバッグログを有効化できます。 ```yaml -# .takt/debug.yaml -enabled: true +logging: + debug: true ``` -デバッグログは `.takt/logs/debug.log` に NDJSON 形式で出力されます。 +デバッグログは `.takt/runs/debug-{timestamp}/logs/debug.log` に NDJSON 形式で出力されます。 ### 詳細モード -空の `.takt/verbose` ファイルを作成すると、詳細なコンソール出力が有効になります。これにより、デバッグログも自動的に有効化されます。 +`verbose: true` を設定すると、詳細なコンソール出力が有効になります。これにより、デバッグログ・トレースも有効化され、ログレベルが `debug` になります。 -または、設定ファイルで `verbose: true` を設定することもできます。 +または、環境変数で `TAKT_VERBOSE=true` を指定して有効化できます。 ```yaml # ~/.takt/config.yaml または .takt/config.yaml verbose: true ``` + +```bash +# env +TAKT_VERBOSE=true +``` diff --git a/docs/configuration.md b/docs/configuration.md index b1e1164..00b6682 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -12,7 +12,8 @@ Configure TAKT defaults in `~/.takt/config.yaml`. This file is created automatic # ~/.takt/config.yaml language: en # UI language: 'en' or 'ja' default_piece: default # Default piece for new projects -log_level: info # Log level: debug, info, warn, error +logging: + level: info # Log level: debug, info, warn, error provider: claude # Default provider: claude, codex, opencode, cursor, or copilot model: sonnet # Default model (optional, passed to provider as-is) branch_name_strategy: romaji # Branch name generation: 'romaji' (fast) or 'ai' (slow) @@ -92,7 +93,7 @@ interactive_preview_movements: 3 # Movement previews in interactive mode (0-10, |-------|------|---------|-------------| | `language` | `"en"` \| `"ja"` | `"en"` | UI language | | `default_piece` | string | `"default"` | Default piece for new projects | -| `log_level` | `"debug"` \| `"info"` \| `"warn"` \| `"error"` | `"info"` | Log level | +| `logging.level` | `"debug"` \| `"info"` \| `"warn"` \| `"error"` | `"info"` | Log level | | `provider` | `"claude"` \| `"codex"` \| `"opencode"` \| `"cursor"` \| `"copilot"` | `"claude"` | Default AI provider | | `model` | string | - | Default model name (passed to provider as-is) | | `branch_name_strategy` | `"romaji"` \| `"ai"` | `"romaji"` | Branch name generation strategy | @@ -434,22 +435,28 @@ pipeline: ### Debug Logging -Enable debug logging by setting `debug_enabled: true` in `~/.takt/config.yaml` or by creating a `.takt/debug.yaml` file: +Enable debug logging by setting `logging.debug: true` in `~/.takt/config.yaml`: ```yaml -# .takt/debug.yaml -enabled: true +logging: + debug: true ``` -Debug logs are written to `.takt/logs/debug.log` in NDJSON format. +Debug logs are written to `.takt/runs/debug-{timestamp}/logs/debug.log` in NDJSON format. ### Verbose Mode -Create an empty `.takt/verbose` file to enable verbose console output. This automatically enables debug logging. - -Alternatively, set `verbose: true` in your config: +Set `verbose: true` in your config: ```yaml # ~/.takt/config.yaml or .takt/config.yaml verbose: true ``` + +You can also force verbose output via environment variable: + +```yaml +TAKT_VERBOSE=true +``` + +This also enables `logging.debug`, `logging.trace`, and sets `logging.level` to `debug`. diff --git a/src/__tests__/config-env-overrides.test.ts b/src/__tests__/config-env-overrides.test.ts index 53ccc38..74c4999 100644 --- a/src/__tests__/config-env-overrides.test.ts +++ b/src/__tests__/config-env-overrides.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { applyGlobalConfigEnvOverrides, applyProjectConfigEnvOverrides, @@ -52,7 +52,6 @@ describe('config env overrides', () => { }); it('should apply project env overrides from generated env names', () => { - process.env.TAKT_LOG_LEVEL = 'debug'; process.env.TAKT_MODEL = 'gpt-5'; process.env.TAKT_VERBOSE = 'true'; process.env.TAKT_CONCURRENCY = '3'; @@ -61,7 +60,6 @@ describe('config env overrides', () => { const raw: Record = {}; applyProjectConfigEnvOverrides(raw); - expect(raw.log_level).toBe('debug'); expect(raw.model).toBe('gpt-5'); expect(raw.verbose).toBe(true); expect(raw.concurrency).toBe(3); @@ -85,6 +83,140 @@ describe('config env overrides', () => { }); }); + it('should apply logging env overrides for global config', () => { + process.env.TAKT_LOGGING_LEVEL = 'debug'; + process.env.TAKT_LOGGING_TRACE = 'true'; + process.env.TAKT_LOGGING_DEBUG = 'true'; + process.env.TAKT_LOGGING_PROVIDER_EVENTS = 'true'; + + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + level: 'debug', + trace: true, + debug: true, + provider_events: true, + }); + }); + + it('should let logging leaf env vars override TAKT_LOGGING JSON', () => { + process.env.TAKT_LOGGING = '{"level":"info","trace":true,"debug":false}'; + process.env.TAKT_LOGGING_LEVEL = 'warn'; + process.env.TAKT_LOGGING_DEBUG = 'true'; + + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + level: 'warn', + trace: true, + debug: true, + }); + }); + + it('should map TAKT_LOGGING_LEVEL as global logging.level override', () => { + process.env.TAKT_LOGGING_LEVEL = 'warn'; + + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + level: 'warn', + }); + }); + + it('should apply logging JSON override for global config', () => { + process.env.TAKT_LOGGING = '{"level":"warn","debug":true}'; + + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + level: 'warn', + debug: true, + }); + }); + + it('should map TAKT_LOG_LEVEL to logging.level with deprecation warning', () => { + process.env.TAKT_LOG_LEVEL = 'warn'; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + level: 'warn', + }); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('TAKT_LOG_LEVEL'), + ); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should map TAKT_OBSERVABILITY_PROVIDER_EVENTS to logging.provider_events with deprecation warning', () => { + process.env.TAKT_OBSERVABILITY_PROVIDER_EVENTS = 'true'; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + provider_events: true, + }); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('TAKT_OBSERVABILITY_PROVIDER_EVENTS'), + ); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should prefer TAKT_LOGGING_* over legacy logging env vars', () => { + process.env.TAKT_LOGGING_LEVEL = 'info'; + process.env.TAKT_LOG_LEVEL = 'debug'; + process.env.TAKT_LOGGING_PROVIDER_EVENTS = 'false'; + process.env.TAKT_OBSERVABILITY_PROVIDER_EVENTS = 'true'; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + level: 'info', + provider_events: false, + }); + expect(warnSpy).not.toHaveBeenCalled(); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should prefer TAKT_LOGGING JSON over legacy logging env vars', () => { + process.env.TAKT_LOGGING = '{"level":"error","provider_events":false}'; + process.env.TAKT_LOG_LEVEL = 'debug'; + process.env.TAKT_OBSERVABILITY_PROVIDER_EVENTS = 'true'; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const raw: Record = {}; + applyGlobalConfigEnvOverrides(raw); + + expect(raw.logging).toEqual({ + level: 'error', + provider_events: false, + }); + expect(warnSpy).not.toHaveBeenCalled(); + } finally { + warnSpy.mockRestore(); + } + }); + it('should apply cursor API key override for global config', () => { process.env.TAKT_CURSOR_API_KEY = 'cursor-key-from-env'; process.env.TAKT_GEMINI_API_KEY = 'gemini-key-from-env'; diff --git a/src/__tests__/config-modularity-boundary.test.ts b/src/__tests__/config-modularity-boundary.test.ts index ebe66cd..79edbe1 100644 --- a/src/__tests__/config-modularity-boundary.test.ts +++ b/src/__tests__/config-modularity-boundary.test.ts @@ -12,8 +12,18 @@ describe('config module file-size boundary', () => { expect(lineCount).toBeLessThanOrEqual(300); }); + it('keeps globalConfig.ts as thin facade under 120 lines', () => { + const lineCount = getLineCount('../infra/config/global/globalConfig.ts'); + expect(lineCount).toBeLessThanOrEqual(120); + }); + it('keeps projectConfig.ts under 300 lines', () => { const lineCount = getLineCount('../infra/config/project/projectConfig.ts'); expect(lineCount).toBeLessThanOrEqual(300); }); + + it('keeps pieceExecution.ts under 300 lines', () => { + const lineCount = getLineCount('../features/tasks/execute/pieceExecution.ts'); + expect(lineCount).toBeLessThanOrEqual(300); + }); }); diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index 8ec5911..ef171ef 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -561,14 +561,20 @@ describe('isVerboseMode', () => { let testDir: string; let originalTaktConfigDir: string | undefined; let originalTaktVerbose: string | undefined; + let originalTaktLoggingDebug: string | undefined; + let originalTaktLoggingTrace: string | undefined; beforeEach(() => { testDir = join(tmpdir(), `takt-test-${randomUUID()}`); mkdirSync(testDir, { recursive: true }); originalTaktConfigDir = process.env.TAKT_CONFIG_DIR; originalTaktVerbose = process.env.TAKT_VERBOSE; + originalTaktLoggingDebug = process.env.TAKT_LOGGING_DEBUG; + originalTaktLoggingTrace = process.env.TAKT_LOGGING_TRACE; process.env.TAKT_CONFIG_DIR = join(testDir, 'global-takt'); delete process.env.TAKT_VERBOSE; + delete process.env.TAKT_LOGGING_DEBUG; + delete process.env.TAKT_LOGGING_TRACE; invalidateGlobalConfigCache(); }); @@ -583,6 +589,16 @@ describe('isVerboseMode', () => { } else { process.env.TAKT_VERBOSE = originalTaktVerbose; } + if (originalTaktLoggingDebug === undefined) { + delete process.env.TAKT_LOGGING_DEBUG; + } else { + process.env.TAKT_LOGGING_DEBUG = originalTaktLoggingDebug; + } + if (originalTaktLoggingTrace === undefined) { + delete process.env.TAKT_LOGGING_TRACE; + } else { + process.env.TAKT_LOGGING_TRACE = originalTaktLoggingTrace; + } if (existsSync(testDir)) { rmSync(testDir, { recursive: true, force: true }); @@ -629,6 +645,66 @@ describe('isVerboseMode', () => { expect(isVerboseMode(testDir)).toBe(false); }); + it('should return true when global logging.debug is enabled', () => { + const globalConfigDir = process.env.TAKT_CONFIG_DIR!; + mkdirSync(globalConfigDir, { recursive: true }); + writeFileSync( + join(globalConfigDir, 'config.yaml'), + [ + 'language: en', + 'logging:', + ' debug: true', + ].join('\n'), + 'utf-8', + ); + + expect(isVerboseMode(testDir)).toBe(true); + }); + + it('should return true when global logging.trace is enabled', () => { + const globalConfigDir = process.env.TAKT_CONFIG_DIR!; + mkdirSync(globalConfigDir, { recursive: true }); + writeFileSync( + join(globalConfigDir, 'config.yaml'), + [ + 'language: en', + 'logging:', + ' trace: true', + ].join('\n'), + 'utf-8', + ); + + expect(isVerboseMode(testDir)).toBe(true); + }); + + it('should return true when TAKT_LOGGING_DEBUG=true is set', () => { + process.env.TAKT_LOGGING_DEBUG = 'true'; + + expect(isVerboseMode(testDir)).toBe(true); + }); + + it('should return true when TAKT_LOGGING_TRACE=true is set', () => { + process.env.TAKT_LOGGING_TRACE = 'true'; + + expect(isVerboseMode(testDir)).toBe(true); + }); + + it('should return true when global logging.level is debug', () => { + const globalConfigDir = process.env.TAKT_CONFIG_DIR!; + mkdirSync(globalConfigDir, { recursive: true }); + writeFileSync( + join(globalConfigDir, 'config.yaml'), + [ + 'language: en', + 'logging:', + ' level: debug', + ].join('\n'), + 'utf-8', + ); + + expect(isVerboseMode(testDir)).toBe(true); + }); + it('should prioritize TAKT_VERBOSE over project and global config', () => { const projectConfigDir = getProjectConfigDir(testDir); mkdirSync(projectConfigDir, { recursive: true }); diff --git a/src/__tests__/globalConfig-defaults.test.ts b/src/__tests__/globalConfig-defaults.test.ts index 5ea1842..9bb3c8b 100644 --- a/src/__tests__/globalConfig-defaults.test.ts +++ b/src/__tests__/globalConfig-defaults.test.ts @@ -24,6 +24,7 @@ const { loadGlobalConfig, saveGlobalConfig, invalidateGlobalConfigCache, + loadGlobalMigratedProjectLocalFallback, } = await import('../infra/config/global/globalConfig.js'); const { getGlobalConfigPath } = await import('../infra/config/paths.js'); @@ -493,43 +494,271 @@ describe('loadGlobalConfig', () => { }); }); - it('should load observability.provider_events config from config.yaml', () => { + it('should load logging config from config.yaml', () => { const taktDir = join(testHomeDir, '.takt'); mkdirSync(taktDir, { recursive: true }); writeFileSync( getGlobalConfigPath(), [ 'language: en', - 'observability:', + 'logging:', ' provider_events: false', ].join('\n'), 'utf-8', ); const config = loadGlobalConfig(); - expect(config.observability).toEqual({ + expect(config.logging).toEqual({ providerEvents: false, }); }); - it('should save and reload observability.provider_events config', () => { + it('should load full logging config with all fields', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'logging:', + ' level: debug', + ' trace: true', + ' debug: true', + ' provider_events: true', + ].join('\n'), + 'utf-8', + ); + + const config = loadGlobalConfig(); + expect(config.logging).toEqual({ + level: 'debug', + trace: true, + debug: true, + providerEvents: true, + }); + }); + + it('should save and reload logging config', () => { const taktDir = join(testHomeDir, '.takt'); mkdirSync(taktDir, { recursive: true }); writeFileSync(getGlobalConfigPath(), 'language: en\n', 'utf-8'); const config = loadGlobalConfig(); - config.observability = { + config.logging = { + level: 'warn', + trace: false, + debug: true, providerEvents: false, }; saveGlobalConfig(config); invalidateGlobalConfigCache(); const reloaded = loadGlobalConfig(); - expect(reloaded.observability).toEqual({ + expect(reloaded.logging).toEqual({ + level: 'warn', + trace: false, + debug: true, providerEvents: false, }); }); + it('should save partial logging config (only provider_events)', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync(getGlobalConfigPath(), 'language: en\n', 'utf-8'); + + const config = loadGlobalConfig(); + config.logging = { + providerEvents: true, + }; + saveGlobalConfig(config); + invalidateGlobalConfigCache(); + + const reloaded = loadGlobalConfig(); + expect(reloaded.logging).toEqual({ + providerEvents: true, + }); + }); + + describe('deprecated migration: observability → logging', () => { + it('should migrate observability.provider_events to logging.providerEvents', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'observability:', + ' provider_events: true', + ].join('\n'), + 'utf-8', + ); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const config = loadGlobalConfig(); + expect(config.logging?.providerEvents).toBe(true); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('observability'), + ); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should not overwrite explicit logging.provider_events with observability value', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'logging:', + ' provider_events: false', + 'observability:', + ' provider_events: true', + ].join('\n'), + 'utf-8', + ); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const config = loadGlobalConfig(); + expect(config.logging?.providerEvents).toBe(false); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should emit deprecation warning when observability is present', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'observability:', + ' provider_events: false', + ].join('\n'), + 'utf-8', + ); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + loadGlobalConfig(); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Deprecated'), + ); + } finally { + warnSpy.mockRestore(); + } + }); + }); + + describe('deprecated migration: log_level → logging.level', () => { + it('should migrate log_level to logging.level', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'log_level: warn', + ].join('\n'), + 'utf-8', + ); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const config = loadGlobalConfig(); + expect(config.logging?.level).toBe('warn'); + expect(warnSpy).toHaveBeenCalled(); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should prefer logging.level over legacy log_level', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'logging:', + ' level: info', + 'log_level: warn', + ].join('\n'), + 'utf-8', + ); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const config = loadGlobalConfig(); + expect(config.logging?.level).toBe('info'); + } finally { + warnSpy.mockRestore(); + } + }); + }); + + describe('logging.level → logLevel fallback', () => { + it('should use logging.level as logLevel fallback when legacy log_level is absent', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'logging:', + ' level: warn', + ].join('\n'), + 'utf-8', + ); + + invalidateGlobalConfigCache(); + const fallback = loadGlobalMigratedProjectLocalFallback(); + expect(fallback.logLevel).toBe('warn'); + }); + + it('should prefer logging.level over legacy log_level', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'log_level: debug', + 'logging:', + ' level: warn', + ].join('\n'), + 'utf-8', + ); + + invalidateGlobalConfigCache(); + const fallback = loadGlobalMigratedProjectLocalFallback(); + expect(fallback.logLevel).toBe('warn'); + }); + + it('should fall back to legacy log_level when logging.level is absent', () => { + const taktDir = join(testHomeDir, '.takt'); + mkdirSync(taktDir, { recursive: true }); + writeFileSync( + getGlobalConfigPath(), + [ + 'language: en', + 'log_level: debug', + ].join('\n'), + 'utf-8', + ); + + invalidateGlobalConfigCache(); + const fallback = loadGlobalMigratedProjectLocalFallback(); + expect(fallback.logLevel).toBe('debug'); + }); + }); + it('should save and reload notification_sound_events config', () => { const taktDir = join(testHomeDir, '.takt'); mkdirSync(taktDir, { recursive: true }); diff --git a/src/__tests__/globalConfig.test.ts b/src/__tests__/globalConfig.test.ts index 35f25f2..c4107f8 100644 --- a/src/__tests__/globalConfig.test.ts +++ b/src/__tests__/globalConfig.test.ts @@ -118,4 +118,20 @@ piece_overrides: expect(reloaded.pieceOverrides?.qualityGates).toEqual(['Test 1', 'Test 2']); }); }); + + describe('security hardening', () => { + it('should reject forbidden keys that can cause prototype pollution', () => { + const configContent = ` +logging: + level: info + __proto__: + polluted: true +`; + writeFileSync(testConfigPath, configContent, 'utf-8'); + + const manager = GlobalConfigManager.getInstance(); + expect(() => manager.load()).toThrow(/forbidden key "__proto__"/i); + expect(({} as Record)['polluted']).toBeUndefined(); + }); + }); }); diff --git a/src/__tests__/globalConfigLegacyMigration.test.ts b/src/__tests__/globalConfigLegacyMigration.test.ts new file mode 100644 index 0000000..41c593a --- /dev/null +++ b/src/__tests__/globalConfigLegacyMigration.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it, vi } from 'vitest'; +import { migrateDeprecatedGlobalConfigKeys } from '../infra/config/global/globalConfigLegacyMigration.js'; + +describe('migrateDeprecatedGlobalConfigKeys', () => { + it('should return migrated config without mutating input object', () => { + const rawConfig: Record = { + log_level: 'warn', + observability: { + provider_events: true, + }, + }; + + const originalSnapshot = JSON.parse(JSON.stringify(rawConfig)) as Record; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const migrated = migrateDeprecatedGlobalConfigKeys(rawConfig); + expect(migrated.migratedLogLevel).toBe('warn'); + expect(migrated.migratedConfig).toEqual({ + logging: { + level: 'warn', + provider_events: true, + }, + }); + expect(rawConfig).toEqual(originalSnapshot); + } finally { + warnSpy.mockRestore(); + } + }); +}); diff --git a/src/__tests__/models-api-boundary.test.ts b/src/__tests__/models-api-boundary.test.ts new file mode 100644 index 0000000..b2ed64d --- /dev/null +++ b/src/__tests__/models-api-boundary.test.ts @@ -0,0 +1,24 @@ +import { readFileSync } from 'node:fs'; +import { describe, expect, it } from 'vitest'; + +function readModule(path: string): string { + return readFileSync(new URL(path, import.meta.url), 'utf-8'); +} + +describe('core/models public type-name boundary', () => { + it('should expose LoggingConfig from index barrel', () => { + const source = readModule('../core/models/index.ts'); + expect(source).toMatch(/\bLoggingConfig\b/); + }); + + it('should not expose legacy ObservabilityConfig from index barrel', () => { + const source = readModule('../core/models/index.ts'); + expect(source).not.toMatch(/\bObservabilityConfig\b/); + }); + + it('should expose LoggingConfig exactly once in index barrel exports', () => { + const source = readModule('../core/models/index.ts'); + const matches = source.match(/\bLoggingConfig\b/g) ?? []; + expect(matches).toHaveLength(1); + }); +}); diff --git a/src/__tests__/models.test.ts b/src/__tests__/models.test.ts index cc72dcf..e139087 100644 --- a/src/__tests__/models.test.ts +++ b/src/__tests__/models.test.ts @@ -578,18 +578,69 @@ describe('GlobalConfigSchema', () => { const result = GlobalConfigSchema.parse(config); expect(result.provider).toBe('claude'); - expect(result.observability).toBeUndefined(); + expect(result.logging).toBeUndefined(); }); - it('should accept valid config', () => { + it('should accept valid logging config', () => { + const config = { + logging: { + provider_events: false, + }, + }; + + const result = GlobalConfigSchema.parse(config); + expect(result.logging?.provider_events).toBe(false); + }); + + it('should accept full logging config with all fields', () => { + const config = { + logging: { + level: 'debug', + trace: true, + debug: true, + provider_events: true, + }, + }; + + const result = GlobalConfigSchema.parse(config); + expect(result.logging?.level).toBe('debug'); + expect(result.logging?.trace).toBe(true); + expect(result.logging?.debug).toBe(true); + expect(result.logging?.provider_events).toBe(true); + }); + + it('should accept partial logging config', () => { + const config = { + logging: { + level: 'warn', + }, + }; + + const result = GlobalConfigSchema.parse(config); + expect(result.logging?.level).toBe('warn'); + expect(result.logging?.trace).toBeUndefined(); + expect(result.logging?.debug).toBeUndefined(); + expect(result.logging?.provider_events).toBeUndefined(); + }); + + it('should reject invalid logging level', () => { + const config = { + logging: { + level: 'verbose', + }, + }; + + expect(() => GlobalConfigSchema.parse(config)).toThrow(); + }); + + it('should reject observability key (strict schema rejects unknown keys)', () => { const config = { observability: { provider_events: false, }, }; - const result = GlobalConfigSchema.parse(config); - expect(result.observability?.provider_events).toBe(false); + expect(() => GlobalConfigSchema.parse(config)).toThrow(); }); it('should parse global provider object block', () => { diff --git a/src/__tests__/pieceExecution-ask-user-question.test.ts b/src/__tests__/pieceExecution-ask-user-question.test.ts index e321c0f..d372745 100644 --- a/src/__tests__/pieceExecution-ask-user-question.test.ts +++ b/src/__tests__/pieceExecution-ask-user-question.test.ts @@ -87,7 +87,7 @@ vi.mock('../infra/config/index.js', () => ({ runtime: undefined, preventSleep: false, model: undefined, - observability: undefined, + logging: undefined, }), saveSessionState: vi.fn(), ensureDir: vi.fn(), diff --git a/src/__tests__/pieceExecution-debug-prompts.test.ts b/src/__tests__/pieceExecution-debug-prompts.test.ts index 123cf68..4b14c7b 100644 --- a/src/__tests__/pieceExecution-debug-prompts.test.ts +++ b/src/__tests__/pieceExecution-debug-prompts.test.ts @@ -102,7 +102,7 @@ vi.mock('../infra/config/index.js', () => ({ runtime: undefined, preventSleep: false, model: undefined, - observability: undefined, + logging: undefined, }), saveSessionState: vi.fn(), ensureDir: vi.fn(), diff --git a/src/__tests__/pieceExecution-session-loading.test.ts b/src/__tests__/pieceExecution-session-loading.test.ts index 5664378..9cb6c3e 100644 --- a/src/__tests__/pieceExecution-session-loading.test.ts +++ b/src/__tests__/pieceExecution-session-loading.test.ts @@ -84,7 +84,7 @@ vi.mock('../infra/config/index.js', () => ({ runtime: undefined, preventSleep: false, model: undefined, - observability: undefined, + logging: undefined, }), saveSessionState: vi.fn(), ensureDir: vi.fn(), @@ -165,7 +165,7 @@ const defaultResolvedConfigValues = { runtime: undefined, preventSleep: false, model: undefined, - observability: undefined, + logging: undefined, analytics: undefined, }; @@ -274,6 +274,19 @@ describe('executePiece session loading', () => { expect(mockInfo).toHaveBeenCalledWith('Model: (default)'); }); + it('should resolve logging config from piece config values', async () => { + await executePiece(makeConfig(), 'task', '/tmp/project', { + projectCwd: '/tmp/project', + }); + + const calls = vi.mocked(resolvePieceConfigValues).mock.calls; + expect(calls).toHaveLength(1); + const keys = calls[0]?.[1]; + expect(Array.isArray(keys)).toBe(true); + expect(keys).toContain('logging'); + expect(keys).not.toContain('observability'); + }); + it('should log configured model from global/project settings when movement model is unresolved', async () => { vi.mocked(resolvePieceConfigValues).mockReturnValue({ ...defaultResolvedConfigValues, diff --git a/src/__tests__/providerEventLogger.test.ts b/src/__tests__/providerEventLogger.test.ts index d2ac74a..89a0e9a 100644 --- a/src/__tests__/providerEventLogger.test.ts +++ b/src/__tests__/providerEventLogger.test.ts @@ -23,15 +23,22 @@ describe('providerEventLogger', () => { it('should disable provider events by default', () => { expect(isProviderEventsEnabled()).toBe(false); expect(isProviderEventsEnabled({})).toBe(false); - expect(isProviderEventsEnabled({ observability: {} })).toBe(false); + expect(isProviderEventsEnabled({ logging: {} })).toBe(false); }); it('should enable provider events only when explicitly true', () => { - expect(isProviderEventsEnabled({ observability: { providerEvents: true } })).toBe(true); + expect(isProviderEventsEnabled({ logging: { providerEvents: true } })).toBe(true); }); it('should disable provider events only when explicitly false', () => { - expect(isProviderEventsEnabled({ observability: { providerEvents: false } })).toBe(false); + expect(isProviderEventsEnabled({ logging: { providerEvents: false } })).toBe(false); + }); + + it('should not enable provider events from legacy observability key', () => { + const legacyOnlyConfig = { + observability: { providerEvents: true }, + } as unknown as Parameters[0]; + expect(isProviderEventsEnabled(legacyOnlyConfig)).toBe(false); }); it('should write normalized JSONL records when enabled', () => { @@ -155,6 +162,31 @@ describe('providerEventLogger', () => { expect(parsed.data.text).toContain('...[truncated]'); }); + it('should report file write failures to stderr only once', () => { + const logger = createProviderEventLogger({ + logsDir: join(tempDir, 'missing', 'nested'), + sessionId: 'session-err', + runId: 'run-err', + provider: 'claude', + movement: 'plan', + enabled: true, + }); + + const original = vi.fn(); + const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + try { + const wrapped = logger.wrapCallback(original); + wrapped({ type: 'text', data: { text: 'first' } }); + wrapped({ type: 'text', data: { text: 'second' } }); + + expect(original).toHaveBeenCalledTimes(2); + expect(stderrSpy).toHaveBeenCalledTimes(1); + expect(stderrSpy.mock.calls[0]?.[0]).toContain('Failed to write provider event log'); + } finally { + stderrSpy.mockRestore(); + } + }); + it('should write init event records with typed data objects', () => { const logger = createProviderEventLogger({ logsDir: tempDir, diff --git a/src/__tests__/resolveConfigValue-no-defaultValue.test.ts b/src/__tests__/resolveConfigValue-no-defaultValue.test.ts index 7c4cf93..276efa8 100644 --- a/src/__tests__/resolveConfigValue-no-defaultValue.test.ts +++ b/src/__tests__/resolveConfigValue-no-defaultValue.test.ts @@ -92,6 +92,26 @@ describe('RESOLUTION_REGISTRY defaultValue removal', () => { }); }); + describe('logLevel migration', () => { + it('should resolve logLevel from global logging.level after migration', () => { + writeFileSync( + globalConfigPath, + [ + 'language: en', + 'logging:', + ' level: warn', + ].join('\n'), + 'utf-8', + ); + invalidateGlobalConfigCache(); + + expect(resolveConfigValueWithSource(projectDir, 'logLevel')).toEqual({ + value: 'warn', + source: 'global', + }); + }); + }); + describe('project-local priority for migrated keys', () => { it.each([ { diff --git a/src/core/models/index.ts b/src/core/models/index.ts index fd9682c..aca3c11 100644 --- a/src/core/models/index.ts +++ b/src/core/models/index.ts @@ -27,7 +27,7 @@ export type { PieceConfig, PieceState, CustomAgentConfig, - ObservabilityConfig, + LoggingConfig, Language, PipelineConfig, ProjectConfig, diff --git a/src/core/models/persisted-global-config.ts b/src/core/models/persisted-global-config.ts index b9c7098..5d242e4 100644 --- a/src/core/models/persisted-global-config.ts +++ b/src/core/models/persisted-global-config.ts @@ -35,8 +35,14 @@ export interface CustomAgentConfig { claudeSkill?: string; } -/** Observability configuration for runtime event logs */ -export interface ObservabilityConfig { +/** Logging configuration for runtime output */ +export interface LoggingConfig { + /** Log level for global output behavior */ + level?: 'debug' | 'info' | 'warn' | 'error'; + /** Enable trace logging */ + trace?: boolean; + /** Enable debug logging */ + debug?: boolean; /** Enable provider stream event logging (default: false when undefined) */ providerEvents?: boolean; } @@ -93,7 +99,7 @@ export interface PersistedGlobalConfig { provider?: 'claude' | 'codex' | 'opencode' | 'cursor' | 'copilot' | 'mock'; model?: string; /** @globalOnly */ - observability?: ObservabilityConfig; + logging?: LoggingConfig; analytics?: AnalyticsConfig; /** @globalOnly */ /** Directory for shared clones (worktree_dir in config). If empty, uses ../{clone-name} relative to project */ diff --git a/src/core/models/schemas.ts b/src/core/models/schemas.ts index 75cbb1c..916f105 100644 --- a/src/core/models/schemas.ts +++ b/src/core/models/schemas.ts @@ -460,7 +460,10 @@ export const CustomAgentConfigSchema = z.object({ { message: 'Agent must have prompt_file, prompt, claude_agent, or claude_skill' } ); -export const ObservabilityConfigSchema = z.object({ +export const LoggingConfigSchema = z.object({ + level: z.enum(['debug', 'info', 'warn', 'error']).optional(), + trace: z.boolean().optional(), + debug: z.boolean().optional(), provider_events: z.boolean().optional(), }); @@ -500,7 +503,7 @@ export const GlobalConfigSchema = z.object({ language: LanguageSchema.optional().default(DEFAULT_LANGUAGE), provider: ProviderReferenceSchema.optional().default('claude'), model: z.string().optional(), - observability: ObservabilityConfigSchema.optional(), + logging: LoggingConfigSchema.optional(), analytics: AnalyticsConfigSchema.optional(), /** Directory for shared clones (worktree_dir in config). If empty, uses ../{clone-name} relative to project */ worktree_dir: z.string().optional(), diff --git a/src/core/models/types.ts b/src/core/models/types.ts index 1b98b79..00ec737 100644 --- a/src/core/models/types.ts +++ b/src/core/models/types.ts @@ -63,7 +63,7 @@ export type { export type { PersonaProviderEntry, CustomAgentConfig, - ObservabilityConfig, + LoggingConfig, Language, PipelineConfig, ProjectConfig, diff --git a/src/features/tasks/execute/pieceExecution.ts b/src/features/tasks/execute/pieceExecution.ts index ea6cdbb..a678488 100644 --- a/src/features/tasks/execute/pieceExecution.ts +++ b/src/features/tasks/execute/pieceExecution.ts @@ -1,7 +1,3 @@ -/** - * Piece execution logic - */ - import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { PieceEngine, createDenyAskUserQuestionHandler } from '../../../core/piece/index.js'; @@ -28,33 +24,12 @@ import { AnalyticsEmitter } from './analyticsEmitter.js'; import { createOutputFns, createPrefixedStreamHandler } from './outputFns.js'; import { RunMetaManager } from './runMeta.js'; import { createIterationLimitHandler, createUserInputHandler } from './iterationLimitHandler.js'; +import { assertTaskPrefixPair, truncate, formatElapsedTime } from './pieceExecutionUtils.js'; export type { PieceExecutionResult, PieceExecutionOptions }; const log = createLogger('piece'); -function assertTaskPrefixPair( - taskPrefix: string | undefined, - taskColorIndex: number | undefined, -): void { - if ((taskPrefix != null) !== (taskColorIndex != null)) { - throw new Error('taskPrefix and taskColorIndex must be provided together'); - } -} - -function truncate(str: string, maxLength: number): string { - return str.length <= maxLength ? str : str.slice(0, maxLength) + '...'; -} - -function formatElapsedTime(startTime: string, endTime: string): string { - const elapsedSec = (new Date(endTime).getTime() - new Date(startTime).getTime()) / 1000; - if (elapsedSec < 60) return `${elapsedSec.toFixed(1)}s`; - return `${Math.floor(elapsedSec / 60)}m ${Math.floor(elapsedSec % 60)}s`; -} - -/** - * Execute a piece and handle all events - */ export async function executePiece( pieceConfig: PieceConfig, task: string, @@ -100,7 +75,7 @@ export async function executePiece( const isWorktree = cwd !== projectCwd; const globalConfig = resolvePieceConfigValues( projectCwd, - ['notificationSound', 'notificationSoundEvents', 'provider', 'runtime', 'preventSleep', 'model', 'observability', 'analytics'], + ['notificationSound', 'notificationSoundEvents', 'provider', 'runtime', 'preventSleep', 'model', 'logging', 'analytics'], ); const shouldNotify = globalConfig.notificationSound !== false; const nse = globalConfig.notificationSoundEvents; diff --git a/src/features/tasks/execute/pieceExecutionUtils.ts b/src/features/tasks/execute/pieceExecutionUtils.ts new file mode 100644 index 0000000..860d5ba --- /dev/null +++ b/src/features/tasks/execute/pieceExecutionUtils.ts @@ -0,0 +1,20 @@ +export function assertTaskPrefixPair( + taskPrefix: string | undefined, + taskColorIndex: number | undefined, +): void { + if ((taskPrefix != null) !== (taskColorIndex != null)) { + throw new Error('taskPrefix and taskColorIndex must be provided together'); + } +} + +export function truncate(value: string, maxLength: number): string { + return value.length <= maxLength ? value : value.slice(0, maxLength) + '...'; +} + +export function formatElapsedTime(startTime: string, endTime: string): string { + const elapsedSec = (new Date(endTime).getTime() - new Date(startTime).getTime()) / 1000; + if (elapsedSec < 60) { + return `${elapsedSec.toFixed(1)}s`; + } + return `${Math.floor(elapsedSec / 60)}m ${Math.floor(elapsedSec % 60)}s`; +} diff --git a/src/infra/config/env/config-env-overrides.ts b/src/infra/config/env/config-env-overrides.ts index edfc8af..2857b12 100644 --- a/src/infra/config/env/config-env-overrides.ts +++ b/src/infra/config/env/config-env-overrides.ts @@ -75,12 +75,38 @@ function applyEnvOverrides(target: Record, specs: readonly EnvS } } +function applyLegacyGlobalLoggingEnvOverrides(target: Record): void { + const nextLogging = process.env.TAKT_LOGGING; + const nextLoggingLevel = process.env.TAKT_LOGGING_LEVEL; + const legacyLogLevel = process.env.TAKT_LOG_LEVEL; + if (legacyLogLevel !== undefined && nextLoggingLevel === undefined && nextLogging === undefined) { + console.warn('Deprecated: "TAKT_LOG_LEVEL" is deprecated. Use "TAKT_LOGGING_LEVEL" instead.'); + setNested(target, 'logging.level', parseEnvValue('TAKT_LOG_LEVEL', legacyLogLevel, 'string')); + } + + const nextLoggingProviderEvents = process.env.TAKT_LOGGING_PROVIDER_EVENTS; + const legacyProviderEvents = process.env.TAKT_OBSERVABILITY_PROVIDER_EVENTS; + if (legacyProviderEvents !== undefined && nextLoggingProviderEvents === undefined && nextLogging === undefined) { + console.warn( + 'Deprecated: "TAKT_OBSERVABILITY_PROVIDER_EVENTS" is deprecated. Use "TAKT_LOGGING_PROVIDER_EVENTS" instead.', + ); + setNested( + target, + 'logging.provider_events', + parseEnvValue('TAKT_OBSERVABILITY_PROVIDER_EVENTS', legacyProviderEvents, 'boolean'), + ); + } +} + const GLOBAL_ENV_SPECS: readonly EnvSpec[] = [ { path: 'language', type: 'string' }, { path: 'provider', type: 'string' }, { path: 'model', type: 'string' }, - { path: 'observability', type: 'json' }, - { path: 'observability.provider_events', type: 'boolean' }, + { path: 'logging', type: 'json' }, + { path: 'logging.level', type: 'string' }, + { path: 'logging.trace', type: 'boolean' }, + { path: 'logging.debug', type: 'boolean' }, + { path: 'logging.provider_events', type: 'boolean' }, { path: 'analytics', type: 'json' }, { path: 'analytics.enabled', type: 'boolean' }, { path: 'analytics.events_path', type: 'string' }, @@ -155,6 +181,7 @@ const PROJECT_ENV_SPECS: readonly EnvSpec[] = [ export function applyGlobalConfigEnvOverrides(target: Record): void { applyEnvOverrides(target, GLOBAL_ENV_SPECS); + applyLegacyGlobalLoggingEnvOverrides(target); } export function applyProjectConfigEnvOverrides(target: Record): void { diff --git a/src/infra/config/global/globalConfigCore.ts b/src/infra/config/global/globalConfigCore.ts index b66744e..d530823 100644 --- a/src/infra/config/global/globalConfigCore.ts +++ b/src/infra/config/global/globalConfigCore.ts @@ -8,11 +8,7 @@ import { } from '../providerReference.js'; import { normalizeProviderProfiles, - denormalizeProviderProfiles, normalizePieceOverrides, - denormalizePieceOverrides, - denormalizeProviderOptions, - normalizeRuntime, } from '../configNormalizers.js'; import { getGlobalConfigPath } from '../paths.js'; import { applyGlobalConfigEnvOverrides } from '../env/config-env-overrides.js'; @@ -23,7 +19,20 @@ import { removeMigratedProjectLocalKeys, type GlobalMigratedProjectLocalFallback, } from './globalMigratedProjectLocalFallback.js'; +import { + sanitizeConfigValue, + migrateDeprecatedGlobalConfigKeys, +} from './globalConfigLegacyMigration.js'; +import { serializeGlobalConfig } from './globalConfigSerializer.js'; export { validateCliPath } from './cliPathValidator.js'; + +function getRecord(value: unknown): Record | undefined { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return undefined; + } + return value as Record; +} + type ProviderType = NonNullable; type RawProviderReference = ConfigProviderReference; export class GlobalConfigManager { @@ -59,15 +68,25 @@ export class GlobalConfigManager { const content = readFileSync(configPath, 'utf-8'); const parsedRaw = parseYaml(content); if (parsedRaw && typeof parsedRaw === 'object' && !Array.isArray(parsedRaw)) { - Object.assign(rawConfig, parsedRaw as Record); + const sanitizedParsedRaw = getRecord(sanitizeConfigValue(parsedRaw, 'config')); + if (!sanitizedParsedRaw) { + throw new Error('Configuration error: ~/.takt/config.yaml must be a YAML object.'); + } + for (const [key, value] of Object.entries(sanitizedParsedRaw)) { + rawConfig[key] = value; + } } else if (parsedRaw != null) { throw new Error('Configuration error: ~/.takt/config.yaml must be a YAML object.'); } } applyGlobalConfigEnvOverrides(rawConfig); - const migratedProjectLocalFallback = extractMigratedProjectLocalFallback(rawConfig); - const schemaInput = { ...rawConfig }; + const { migratedConfig, migratedLogLevel } = migrateDeprecatedGlobalConfigKeys(rawConfig); + const migratedProjectLocalFallback = extractMigratedProjectLocalFallback({ + ...migratedConfig, + ...(migratedLogLevel !== undefined ? { log_level: migratedLogLevel } : {}), + }); + const schemaInput = { ...migratedConfig }; removeMigratedProjectLocalKeys(schemaInput); const parsed = GlobalConfigSchema.parse(schemaInput); @@ -80,8 +99,11 @@ export class GlobalConfigManager { language: parsed.language, provider: normalizedProvider.provider, model: normalizedProvider.model, - observability: parsed.observability ? { - providerEvents: parsed.observability.provider_events, + logging: parsed.logging ? { + level: parsed.logging.level, + trace: parsed.logging.trace, + debug: parsed.logging.debug, + providerEvents: parsed.logging.provider_events, } : undefined, analytics: parsed.analytics ? { enabled: parsed.analytics.enabled, @@ -110,7 +132,9 @@ export class GlobalConfigManager { pieceCategoriesFile: parsed.piece_categories_file, providerOptions: normalizedProvider.providerOptions, providerProfiles: normalizeProviderProfiles(parsed.provider_profiles as Record }> | undefined), - runtime: normalizeRuntime(parsed.runtime), + runtime: parsed.runtime?.prepare && parsed.runtime.prepare.length > 0 + ? { prepare: [...new Set(parsed.runtime.prepare)] } + : undefined, preventSleep: parsed.prevent_sleep, notificationSound: parsed.notification_sound, notificationSoundEvents: parsed.notification_sound_events ? { @@ -140,136 +164,7 @@ export class GlobalConfigManager { save(config: PersistedGlobalConfig): void { const configPath = getGlobalConfigPath(); - const raw: Record = { - language: config.language, - provider: config.provider, - }; - if (config.model) { - raw.model = config.model; - } - if (config.observability && config.observability.providerEvents !== undefined) { - raw.observability = { - provider_events: config.observability.providerEvents, - }; - } - if (config.analytics) { - const analyticsRaw: Record = {}; - if (config.analytics.enabled !== undefined) analyticsRaw.enabled = config.analytics.enabled; - if (config.analytics.eventsPath) analyticsRaw.events_path = config.analytics.eventsPath; - if (config.analytics.retentionDays !== undefined) analyticsRaw.retention_days = config.analytics.retentionDays; - if (Object.keys(analyticsRaw).length > 0) { - raw.analytics = analyticsRaw; - } - } - if (config.worktreeDir) { - raw.worktree_dir = config.worktreeDir; - } - if (config.autoPr !== undefined) { - raw.auto_pr = config.autoPr; - } - if (config.draftPr !== undefined) { - raw.draft_pr = config.draftPr; - } - if (config.disabledBuiltins && config.disabledBuiltins.length > 0) { - raw.disabled_builtins = config.disabledBuiltins; - } - if (config.enableBuiltinPieces !== undefined) { - raw.enable_builtin_pieces = config.enableBuiltinPieces; - } - if (config.anthropicApiKey) { - raw.anthropic_api_key = config.anthropicApiKey; - } - if (config.openaiApiKey) { - raw.openai_api_key = config.openaiApiKey; - } - if (config.geminiApiKey) { - raw.gemini_api_key = config.geminiApiKey; - } - if (config.googleApiKey) { - raw.google_api_key = config.googleApiKey; - } - if (config.groqApiKey) { - raw.groq_api_key = config.groqApiKey; - } - if (config.openrouterApiKey) { - raw.openrouter_api_key = config.openrouterApiKey; - } - if (config.codexCliPath) { - raw.codex_cli_path = config.codexCliPath; - } - if (config.claudeCliPath) { - raw.claude_cli_path = config.claudeCliPath; - } - if (config.cursorCliPath) { - raw.cursor_cli_path = config.cursorCliPath; - } - if (config.copilotCliPath) { - raw.copilot_cli_path = config.copilotCliPath; - } - if (config.copilotGithubToken) { - raw.copilot_github_token = config.copilotGithubToken; - } - if (config.opencodeApiKey) { - raw.opencode_api_key = config.opencodeApiKey; - } - if (config.cursorApiKey) { - raw.cursor_api_key = config.cursorApiKey; - } - if (config.bookmarksFile) { - raw.bookmarks_file = config.bookmarksFile; - } - if (config.pieceCategoriesFile) { - raw.piece_categories_file = config.pieceCategoriesFile; - } - const rawProviderOptions = denormalizeProviderOptions(config.providerOptions); - if (rawProviderOptions) { - raw.provider_options = rawProviderOptions; - } - const rawProviderProfiles = denormalizeProviderProfiles(config.providerProfiles); - if (rawProviderProfiles && Object.keys(rawProviderProfiles).length > 0) { - raw.provider_profiles = rawProviderProfiles; - } - const normalizedRuntime = normalizeRuntime(config.runtime); - if (normalizedRuntime) { - raw.runtime = normalizedRuntime; - } - if (config.preventSleep !== undefined) { - raw.prevent_sleep = config.preventSleep; - } - if (config.notificationSound !== undefined) { - raw.notification_sound = config.notificationSound; - } - if (config.notificationSoundEvents) { - const eventRaw: Record = {}; - if (config.notificationSoundEvents.iterationLimit !== undefined) { - eventRaw.iteration_limit = config.notificationSoundEvents.iterationLimit; - } - if (config.notificationSoundEvents.pieceComplete !== undefined) { - eventRaw.piece_complete = config.notificationSoundEvents.pieceComplete; - } - if (config.notificationSoundEvents.pieceAbort !== undefined) { - eventRaw.piece_abort = config.notificationSoundEvents.pieceAbort; - } - if (config.notificationSoundEvents.runComplete !== undefined) { - eventRaw.run_complete = config.notificationSoundEvents.runComplete; - } - if (config.notificationSoundEvents.runAbort !== undefined) { - eventRaw.run_abort = config.notificationSoundEvents.runAbort; - } - if (Object.keys(eventRaw).length > 0) { - raw.notification_sound_events = eventRaw; - } - } - if (config.autoFetch) { - raw.auto_fetch = config.autoFetch; - } - if (config.baseBranch) { - raw.base_branch = config.baseBranch; - } - const denormalizedPieceOverrides = denormalizePieceOverrides(config.pieceOverrides); - if (denormalizedPieceOverrides) { - raw.piece_overrides = denormalizedPieceOverrides; - } + const raw = serializeGlobalConfig(config); writeFileSync(configPath, stringifyYaml(raw), 'utf-8'); this.invalidateCache(); invalidateAllResolvedConfigCache(); diff --git a/src/infra/config/global/globalConfigLegacyMigration.ts b/src/infra/config/global/globalConfigLegacyMigration.ts new file mode 100644 index 0000000..192d100 --- /dev/null +++ b/src/infra/config/global/globalConfigLegacyMigration.ts @@ -0,0 +1,99 @@ +function getRecord(value: unknown): Record | undefined { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return undefined; + } + return value as Record; +} + +const FORBIDDEN_CONFIG_KEYS = new Set(['__proto__', 'prototype', 'constructor']); + +export function sanitizeConfigValue(value: unknown, path: string): unknown { + if (Array.isArray(value)) { + return value.map((item, index) => sanitizeConfigValue(item, `${path}[${index}]`)); + } + + const record = getRecord(value); + if (!record) { + return value; + } + + const sanitized: Record = {}; + for (const [key, nestedValue] of Object.entries(record)) { + if (FORBIDDEN_CONFIG_KEYS.has(key)) { + throw new Error(`Configuration error: forbidden key "${key}" at "${path}".`); + } + sanitized[key] = sanitizeConfigValue(nestedValue, `${path}.${key}`); + } + return sanitized; +} + +type LegacyGlobalConfigMigrationResult = { + migratedConfig: Record; + migratedLogLevel?: string; +}; + +export function migrateDeprecatedGlobalConfigKeys(rawConfig: Record): LegacyGlobalConfigMigrationResult { + const migratedConfig: Record = { ...rawConfig }; + const hasLegacyLogLevel = Object.prototype.hasOwnProperty.call(rawConfig, 'log_level'); + const legacyLogLevel = rawConfig.log_level; + const hasLegacyObservability = Object.prototype.hasOwnProperty.call(rawConfig, 'observability'); + const observability = getRecord(rawConfig.observability); + const initialLogging = getRecord(rawConfig.logging); + let migratedLogging = initialLogging ? { ...initialLogging } : undefined; + + if (hasLegacyObservability) { + console.warn('Deprecated: "observability" is deprecated. Use "logging" instead.'); + if (observability) { + const observabilityProviderEvents = observability.provider_events; + if (observabilityProviderEvents !== undefined) { + const hasExplicitProviderEvents = migratedLogging + ? Object.prototype.hasOwnProperty.call(migratedLogging, 'provider_events') + : false; + if (!hasExplicitProviderEvents) { + migratedLogging = { + ...(migratedLogging ?? {}), + provider_events: observabilityProviderEvents, + }; + } + } + } + } + + if (hasLegacyLogLevel) { + console.warn('Deprecated: "log_level" is deprecated. Use "logging.level" instead.'); + } + + const resolvedLoggingLevel = migratedLogging?.level; + const migratedLogLevel = typeof resolvedLoggingLevel === 'string' + ? resolvedLoggingLevel + : hasLegacyLogLevel && typeof legacyLogLevel === 'string' + ? legacyLogLevel + : undefined; + + if (migratedLogLevel !== undefined) { + const hasExplicitLevel = migratedLogging + ? Object.prototype.hasOwnProperty.call(migratedLogging, 'level') + : false; + if (!hasExplicitLevel) { + migratedLogging = { + ...(migratedLogging ?? {}), + level: migratedLogLevel, + }; + } + } + if (migratedLogging) { + migratedConfig.logging = migratedLogging; + } + + if (hasLegacyObservability) { + delete migratedConfig.observability; + } + if (hasLegacyLogLevel) { + delete migratedConfig.log_level; + } + + return { + migratedConfig, + migratedLogLevel, + }; +} diff --git a/src/infra/config/global/globalConfigSerializer.ts b/src/infra/config/global/globalConfigSerializer.ts new file mode 100644 index 0000000..218bf08 --- /dev/null +++ b/src/infra/config/global/globalConfigSerializer.ts @@ -0,0 +1,149 @@ +import type { PersistedGlobalConfig } from '../../../core/models/persisted-global-config.js'; +import { + denormalizeProviderProfiles, + denormalizePieceOverrides, + denormalizeProviderOptions, +} from '../configNormalizers.js'; + +export function serializeGlobalConfig(config: PersistedGlobalConfig): Record { + const raw: Record = { + language: config.language, + provider: config.provider, + }; + if (config.model) { + raw.model = config.model; + } + if (config.logging && ( + config.logging.level !== undefined + || config.logging.trace !== undefined + || config.logging.debug !== undefined + || config.logging.providerEvents !== undefined + )) { + raw.logging = { + ...(config.logging.level !== undefined ? { level: config.logging.level } : {}), + ...(config.logging.trace !== undefined ? { trace: config.logging.trace } : {}), + ...(config.logging.debug !== undefined ? { debug: config.logging.debug } : {}), + ...(config.logging.providerEvents !== undefined ? { provider_events: config.logging.providerEvents } : {}), + }; + } + if (config.analytics) { + const analyticsRaw: Record = {}; + if (config.analytics.enabled !== undefined) analyticsRaw.enabled = config.analytics.enabled; + if (config.analytics.eventsPath) analyticsRaw.events_path = config.analytics.eventsPath; + if (config.analytics.retentionDays !== undefined) analyticsRaw.retention_days = config.analytics.retentionDays; + if (Object.keys(analyticsRaw).length > 0) { + raw.analytics = analyticsRaw; + } + } + if (config.worktreeDir) { + raw.worktree_dir = config.worktreeDir; + } + if (config.autoPr !== undefined) { + raw.auto_pr = config.autoPr; + } + if (config.draftPr !== undefined) { + raw.draft_pr = config.draftPr; + } + if (config.disabledBuiltins && config.disabledBuiltins.length > 0) { + raw.disabled_builtins = config.disabledBuiltins; + } + if (config.enableBuiltinPieces !== undefined) { + raw.enable_builtin_pieces = config.enableBuiltinPieces; + } + if (config.anthropicApiKey) { + raw.anthropic_api_key = config.anthropicApiKey; + } + if (config.openaiApiKey) { + raw.openai_api_key = config.openaiApiKey; + } + if (config.geminiApiKey) { + raw.gemini_api_key = config.geminiApiKey; + } + if (config.googleApiKey) { + raw.google_api_key = config.googleApiKey; + } + if (config.groqApiKey) { + raw.groq_api_key = config.groqApiKey; + } + if (config.openrouterApiKey) { + raw.openrouter_api_key = config.openrouterApiKey; + } + if (config.codexCliPath) { + raw.codex_cli_path = config.codexCliPath; + } + if (config.claudeCliPath) { + raw.claude_cli_path = config.claudeCliPath; + } + if (config.cursorCliPath) { + raw.cursor_cli_path = config.cursorCliPath; + } + if (config.copilotCliPath) { + raw.copilot_cli_path = config.copilotCliPath; + } + if (config.copilotGithubToken) { + raw.copilot_github_token = config.copilotGithubToken; + } + if (config.opencodeApiKey) { + raw.opencode_api_key = config.opencodeApiKey; + } + if (config.cursorApiKey) { + raw.cursor_api_key = config.cursorApiKey; + } + if (config.bookmarksFile) { + raw.bookmarks_file = config.bookmarksFile; + } + if (config.pieceCategoriesFile) { + raw.piece_categories_file = config.pieceCategoriesFile; + } + const rawProviderOptions = denormalizeProviderOptions(config.providerOptions); + if (rawProviderOptions) { + raw.provider_options = rawProviderOptions; + } + const rawProviderProfiles = denormalizeProviderProfiles(config.providerProfiles); + if (rawProviderProfiles && Object.keys(rawProviderProfiles).length > 0) { + raw.provider_profiles = rawProviderProfiles; + } + if (config.runtime?.prepare && config.runtime.prepare.length > 0) { + raw.runtime = { + prepare: [...new Set(config.runtime.prepare)], + }; + } + if (config.preventSleep !== undefined) { + raw.prevent_sleep = config.preventSleep; + } + if (config.notificationSound !== undefined) { + raw.notification_sound = config.notificationSound; + } + if (config.notificationSoundEvents) { + const eventRaw: Record = {}; + if (config.notificationSoundEvents.iterationLimit !== undefined) { + eventRaw.iteration_limit = config.notificationSoundEvents.iterationLimit; + } + if (config.notificationSoundEvents.pieceComplete !== undefined) { + eventRaw.piece_complete = config.notificationSoundEvents.pieceComplete; + } + if (config.notificationSoundEvents.pieceAbort !== undefined) { + eventRaw.piece_abort = config.notificationSoundEvents.pieceAbort; + } + if (config.notificationSoundEvents.runComplete !== undefined) { + eventRaw.run_complete = config.notificationSoundEvents.runComplete; + } + if (config.notificationSoundEvents.runAbort !== undefined) { + eventRaw.run_abort = config.notificationSoundEvents.runAbort; + } + if (Object.keys(eventRaw).length > 0) { + raw.notification_sound_events = eventRaw; + } + } + if (config.autoFetch) { + raw.auto_fetch = config.autoFetch; + } + if (config.baseBranch) { + raw.base_branch = config.baseBranch; + } + const denormalizedPieceOverrides = denormalizePieceOverrides(config.pieceOverrides); + if (denormalizedPieceOverrides) { + raw.piece_overrides = denormalizedPieceOverrides; + } + return raw; +} diff --git a/src/infra/config/project/resolvedSettings.ts b/src/infra/config/project/resolvedSettings.ts index b777524..f50343e 100644 --- a/src/infra/config/project/resolvedSettings.ts +++ b/src/infra/config/project/resolvedSettings.ts @@ -1,5 +1,5 @@ -import { resolveConfigValue } from '../resolveConfigValue.js'; +import { isVerboseShortcutEnabled } from '../resolveConfigValue.js'; export function isVerboseMode(projectDir: string): boolean { - return resolveConfigValue(projectDir, 'verbose'); + return isVerboseShortcutEnabled(projectDir); } diff --git a/src/infra/config/resolveConfigValue.ts b/src/infra/config/resolveConfigValue.ts index 6a31114..9d36606 100644 --- a/src/infra/config/resolveConfigValue.ts +++ b/src/infra/config/resolveConfigValue.ts @@ -75,6 +75,7 @@ const MIGRATED_PROJECT_LOCAL_CONFIG_KEY_SET = new Set( ); const RESOLUTION_REGISTRY: Partial<{ [K in ConfigParameterKey]: ResolutionRule }> = { + logLevel: { layers: ['local', 'global'] }, provider: { layers: ['local', 'piece', 'global'], pieceValue: (pieceContext) => pieceContext?.provider, @@ -134,6 +135,10 @@ function getGlobalLayerValue( globalMigratedProjectLocalFallback: GlobalMigratedProjectLocalFallback, key: K, ): LoadedConfig[K] | undefined { + if (key === 'logLevel' && global.logging?.level !== undefined) { + return global.logging.level as LoadedConfig[K]; + } + if (isMigratedProjectLocalConfigKey(key)) { return globalMigratedProjectLocalFallback[key] as LoadedConfig[K] | undefined; } @@ -243,3 +248,20 @@ export function resolveConfigValues( } return result; } + +export function isVerboseShortcutEnabled( + projectDir: string, + options?: ResolveConfigOptions, +): boolean { + const verbose = resolveConfigValue(projectDir, 'verbose', options); + if (verbose === true) { + return true; + } + + const logging = resolveConfigValue(projectDir, 'logging', options); + if (logging?.debug === true || logging?.trace === true) { + return true; + } + + return resolveConfigValue(projectDir, 'logLevel', options) === 'debug'; +} diff --git a/src/shared/utils/providerEventLogger.ts b/src/shared/utils/providerEventLogger.ts index 6c51953..c921b45 100644 --- a/src/shared/utils/providerEventLogger.ts +++ b/src/shared/utils/providerEventLogger.ts @@ -94,13 +94,19 @@ export function createProviderEventLogger(config: ProviderEventLoggerConfig): Pr const filepath = join(config.logsDir, `${config.sessionId}-provider-events.jsonl`); let movement = config.movement; let provider = config.provider; + let hasReportedWriteFailure = false; const write = (event: StreamEvent): void => { try { const record = buildLogRecord(event, provider, movement, config.runId); appendFileSync(filepath, JSON.stringify(record) + '\n', 'utf-8'); - } catch { - // Silently fail - observability logging should not interrupt main flow. + } catch (error) { + if (hasReportedWriteFailure) { + return; + } + hasReportedWriteFailure = true; + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`[takt] Failed to write provider event log: ${message}\n`); } }; @@ -129,9 +135,9 @@ export function createProviderEventLogger(config: ProviderEventLoggerConfig): Pr } export function isProviderEventsEnabled(config?: { - observability?: { + logging?: { providerEvents?: boolean; }; }): boolean { - return config?.observability?.providerEvents === true; + return config?.logging?.providerEvents === true; }