[#773] add-kiro-provider#776
Open
nrslib wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughTAKT に Kiro CLI をプロバイダとして統合する包括的な実装です。プロバイダ型定義から設定管理、実行基盤、クライアント実装、Provider 登録、ユーティリティ、ドキュメント、テストまで全体をカバーしています。 ChangesKiro CLI プロバイダ統合
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/kiro-client.test.ts`:
- Around line 64-67: The test currently deletes process.env.KIRO_API_KEY in the
beforeEach hook causing cross-test side effects; modify the setup to capture the
original value (e.g., const originalKiroApiKey = process.env.KIRO_API_KEY)
before deleting it in beforeEach and add an afterEach hook that restores
process.env.KIRO_API_KEY = originalKiroApiKey (or deletes it if it was
undefined). Update or add the afterEach alongside the existing
beforeEach/vi.clearAllMocks() to ensure tests restore the environment variable
after each run.
In `@src/infra/kiro/process.ts`:
- Around line 146-153: The current handling in the stdoutBytes (and similarly
stderrBytes) overflow branch kills the child with SIGTERM and immediately calls
rejectOnce, which can leave a lingering CLI if it ignores SIGTERM; change this
to send SIGTERM via child.kill('SIGTERM'), then start a short fallback timer
(e.g., 1–2s) that will call child.kill('SIGKILL') if the process is still alive,
and only then call rejectOnce(createExecError(...)). Ensure the same pattern is
applied to both the stdoutBytes and stderrBytes branches, referencing
stdoutBytes, stderrBytes, KIRO_MAX_BUFFER_BYTES, child.kill, and
rejectOnce/createExecError so the code first attempts graceful termination and
then force kills before rejecting to avoid process residuals.
- Around line 39-50: The buildEnv function currently sets env.KIRO_API_KEY only
when the incoming kiroApiKey argument is provided, so a parent-process
KIRO_API_KEY in process.env is not propagated; update buildEnv (and where
KIRO_ENV_ALLOWLIST is considered) to set env.KIRO_API_KEY from kiroApiKey if
provided, otherwise fall back to process.env.KIRO_API_KEY when present (i.e., if
kiroApiKey === undefined and process.env.KIRO_API_KEY !== undefined then assign
env.KIRO_API_KEY = process.env.KIRO_API_KEY) so child processes receive the
parent auth token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aaac966a-303c-4aff-9b2b-f306fe9c9e21
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (47)
README.mdbuiltins/en/config.yamlbuiltins/ja/config.yamldocs/README.ja.mddocs/ci-cd.ja.mddocs/ci-cd.mddocs/cli-reference.ja.mddocs/cli-reference.mddocs/configuration.ja.mddocs/configuration.mddocs/workflows.ja.mddocs/workflows.mdpackage.jsonsrc/__tests__/initialization.test.tssrc/__tests__/kiro-client.test.tssrc/__tests__/kiro-config.test.tssrc/__tests__/kiro-provider-integration.test.tssrc/__tests__/kiro-provider.test.tssrc/__tests__/permission-profile-resolution.test.tssrc/__tests__/provider-contract-docs.test.tssrc/__tests__/spawn.test.tssrc/app/cli/program.tssrc/core/models/config-schemas.tssrc/core/models/config-types.tssrc/core/models/provider-profiles.tssrc/core/models/schema-base.tssrc/core/workflow/permission-profile-resolution.tssrc/infra/config/configNormalizers.tssrc/infra/config/env/global-current-env-specs.tssrc/infra/config/global/globalConfig.tssrc/infra/config/global/globalConfigCore.tssrc/infra/config/global/globalConfigResolvers.tssrc/infra/config/global/globalConfigSerializer.tssrc/infra/config/global/index.tssrc/infra/config/global/initialization.tssrc/infra/config/project/projectConfig.tssrc/infra/config/traced/tracedConfigSchema.tssrc/infra/kiro/client.tssrc/infra/kiro/index.tssrc/infra/kiro/process.tssrc/infra/kiro/types.tssrc/infra/providers/index.tssrc/infra/providers/kiro.tssrc/infra/providers/types.tssrc/shared/types/provider.tssrc/shared/utils/spawn.tssrc/types/cross-spawn.d.ts
Owner
Author
|
レビューしました。CI は 主な要修正点です。
Kiro 公式 docs では
TAKT のタスク本文は Markdown なので、箇条書きだけの依頼や
provider schema と CLI help の整合テストは有用ですが、Markdown の括弧内テキストや builtin config のコメント文字列まで unit test にするのは過剰です。以前の方針ともズレるので、消すか、コード契約のテストに絞るのがよいです。 全体として、 |
Owner
Author
Summaryタスク指示書: PR #776 Kiro provider レビュー指摘対応目的PR #776 対象
作業項目高: 無関係な依存ダウングレードを戻す対象:
作業:
確認:
高:
|
| 優先度 | 項目 |
|---|---|
| 高 | package.json / package-lock.json の無関係な依存ダウングレードを戻す |
| 高 | KIRO_API_KEY の親環境フォールバックを実装する |
| 高 | KIRO_HOME を Kiro child process に引き継ぐ |
| 高 | stdout/stderr バッファ超過時の child process 後始末を強化する |
| 高 | KIRO_API_KEY を削除するテストの環境復元を実装する |
| 中 | prompt が - で始まる入力の reject を見直す |
| 中 | brittle な provider contract docs テストを見直す |
| 中 | コメント指摘事項への対応状況を整理する |
確認コマンド
最低限、以下を実行して結果を記録する。
npm run lint
npm test
npm run test:e2e:mock変更内容に応じて、以下も実行する。
npm run build個別確認として、関連テストを先に実行してよい。
npm test -- src/__tests__/kiro-client.test.ts
npm test -- src/__tests__/kiro-provider.test.ts
npm test -- src/__tests__/provider-contract-docs.test.ts完了条件
- CodeRabbit Active Review Threads の妥当な指摘が修正済みである。
nrslibの4件の要修正点が修正済みである。package.json/package-lock.jsonに Kiro provider と無関係な依存ダウングレードが残っていない。KIRO_API_KEY、KIRO_HOME、必要な proxy/cert 系 env の child process 引き継ぎがテストで確認されている。- stdout/stderr バッファ超過時に graceful kill と force kill fallback が動作する。
-で始まる Markdown prompt が provider 側で不当に reject されない。- docs 文言に過度に依存する brittle な unit test が整理されている。
npm run lint、npm test、npm run test:e2e:mockの実行結果が記録されている。
Open Questions
- Kiro CLI が prompt 前の
--区切りを正式に受け付けるかは、実装コードまたは利用可能な CLI 仕様で確認すること。受け付けない場合は、prompt が option として解釈されない別方式で実装する。
Execution Report
Workflow review-fix-takt-default completed successfully.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/dependency-versions.test.ts`:
- Around line 175-185: The new test "locks runtime transitive dependencies to
patched security releases" in src/__tests__/dependency-versions.test.ts is
unrelated to the Kiro provider change; either remove these assertions (the
expect(...) calls that use readPackageLock() and getLockedPackage(...)) from
this PR or move them into a separate PR/test file dedicated to
dependency-version locking; if you must keep them here, add a clear
justification in the PR description and commit message explaining why the
dependency-lock assertions (calls to readPackageLock and getLockedPackage for
ajv, express-rate-limit, fast-uri, hono, ip-address, protobufjs, qs) are
included despite scope rules.
In `@src/infra/codex/client.ts`:
- Line 44: This change introduces unrelated, breaking behavior in the Codex
client (e.g., addition of CODEX_RETRY_MAX_DELAY_MS, altered stream `error`
handler that no longer breaks, new failure detection when `turn.completed` is
not seen, and suppression of retries after stream errors); revert these
modifications from the Kiro feature branch and isolate them into a separate PR
instead: restore the original retry/backoff logic and event handling in the
Codex client (functions/classes referenced like CODEX_RETRY_MAX_DELAY_MS, the
stream `error` event handler, the `turn.completed` observation logic, and the
retry suppression behavior) so the Kiro provider changes only touch
Kiro-specific code, then open a dedicated PR for any intended Codex client
contract changes.
In `@src/infra/config/global/globalConfigCore.ts`:
- Line 168: provider_profiles の default_permission_mode
を必須化したため既存設定がパースエラーになる破壊的変更が発生します。CHANGELOG.md
に該当エントリを追加し、「default_permission_mode が必須になったこと」「許容値は 'readonly' | 'edit' |
'full' であること」「既存設定の移行手順（全ての provider_profiles に例えば default_permission_mode:
'readonly' を追加する方法や一括置換コマンドの例）」「影響を受けるコード箇所（ProviderPermissionProfileSchema と
normalizeProviderProfiles）を追記」してください。これによりユーザーが設定を更新してパースエラーを回避できるように記載してください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d8571d8c-dff3-4faa-8010-5364b00c8a06
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (15)
package.jsonsrc/__tests__/codex-client-retry.test.tssrc/__tests__/dependency-versions.test.tssrc/__tests__/kiro-client.test.tssrc/__tests__/kiro-config.test.tssrc/__tests__/provider-contract-docs.test.tssrc/core/models/schema-base.tssrc/infra/codex/client.tssrc/infra/config/configNormalizers.tssrc/infra/config/global/globalConfigCore.tssrc/infra/config/project/projectConfig.tssrc/infra/kiro/client.tssrc/infra/kiro/process.tssrc/infra/kiro/types.tssrc/infra/providers/kiro.ts
💤 Files with no reviewable changes (3)
- src/infra/kiro/types.ts
- src/tests/provider-contract-docs.test.ts
- src/infra/providers/kiro.ts
Comment on lines
+175
to
+185
| it('locks runtime transitive dependencies to patched security releases', () => { | ||
| const packageLock = readPackageLock(); | ||
|
|
||
| expect(getLockedPackage(packageLock, 'node_modules/ajv').version).toBe('6.15.0'); | ||
| expect(getLockedPackage(packageLock, 'node_modules/express-rate-limit').version).toBe('8.5.2'); | ||
| expect(getLockedPackage(packageLock, 'node_modules/fast-uri').version).toBe('3.1.2'); | ||
| expect(getLockedPackage(packageLock, 'node_modules/hono').version).toBe('4.12.23'); | ||
| expect(getLockedPackage(packageLock, 'node_modules/ip-address').version).toBe('10.2.0'); | ||
| expect(getLockedPackage(packageLock, 'node_modules/protobufjs').version).toBe('7.6.1'); | ||
| expect(getLockedPackage(packageLock, 'node_modules/qs').version).toBe('6.15.2'); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Kiro プロバイダとは無関係な依存関係テストの追加
このテストは、ランタイムの推移的依存関係のバージョンをロックする重要なセキュリティ対策ですが、Kiro プロバイダ機能とは直接関係ありません。PR の範囲が拡大している可能性があります。
As per coding guidelines: "Scope creep / incidental contract changes: don't change observable contracts (error messages, public API shapes, CLI help text expectations, provider registry behavior) unless required by the Kiro feature."
別の PR として分離することを検討してください。ただし、既にマージ準備が整っている場合は、このままでも問題ありません。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/__tests__/dependency-versions.test.ts` around lines 175 - 185, The new
test "locks runtime transitive dependencies to patched security releases" in
src/__tests__/dependency-versions.test.ts is unrelated to the Kiro provider
change; either remove these assertions (the expect(...) calls that use
readPackageLock() and getLockedPackage(...)) from this PR or move them into a
separate PR/test file dedicated to dependency-version locking; if you must keep
them here, add a clear justification in the PR description and commit message
explaining why the dependency-lock assertions (calls to readPackageLock and
getLockedPackage for ajv, express-rate-limit, fast-uri, hono, ip-address,
protobufjs, qs) are included despite scope rules.
| const CODEX_TIMEOUT_MAX_RETRIES = 2; | ||
| const CODEX_RETRY_MAX_RETRIES = 8; | ||
| const CODEX_RETRY_BASE_DELAY_MS = 1000; | ||
| const CODEX_RETRY_MAX_DELAY_MS = 30_000; |
There was a problem hiding this comment.
Kiro プロバイダとは無関係な Codex クライアントの大幅な変更
このファイルには、Codex SDK クライアントのリトライロジックとストリームエラー処理に対する重要な変更が含まれていますが、Kiro プロバイダ機能とは完全に無関係です：
- リトライ遅延の上限追加（30秒キャップ）
errorイベント処理の変更（即座にbreakせず継続）turn.completedが観測されない場合の失敗判定ロジック追加- ストリームエラー後のリトライ抑制
これらは Codex プロバイダの動作を変更する重要な変更であり、別の PR として分離すべきです。
As per coding guidelines: "Scope creep / incidental contract changes: don't change observable contracts [...] unless required by the Kiro feature."
Codex クライアントの変更を別の PR に分離し、Kiro プロバイダの実装に集中することを推奨します。
Also applies to: 155-158, 358-390, 453-462, 480-480
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/infra/codex/client.ts` at line 44, This change introduces unrelated,
breaking behavior in the Codex client (e.g., addition of
CODEX_RETRY_MAX_DELAY_MS, altered stream `error` handler that no longer breaks,
new failure detection when `turn.completed` is not seen, and suppression of
retries after stream errors); revert these modifications from the Kiro feature
branch and isolate them into a separate PR instead: restore the original
retry/backoff logic and event handling in the Codex client (functions/classes
referenced like CODEX_RETRY_MAX_DELAY_MS, the stream `error` event handler, the
`turn.completed` observation logic, and the retry suppression behavior) so the
Kiro provider changes only touch Kiro-specific code, then open a dedicated PR
for any intended Codex client contract changes.
| providerProfiles: normalizeProviderProfiles( | ||
| parsed.provider_profiles as Record<string, { | ||
| default_permission_mode: string; | ||
| default_permission_mode: 'readonly' | 'edit' | 'full'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for default permission mode handling and migration guidance
# Search for default permission mode constants or fallbacks
rg -nP 'DEFAULT.*PERMISSION.*MODE' --type=ts
# Search for migration guides or changelog entries
fd -e md -e txt CHANGELOG MIGRATION
# Check test fixtures for provider_profiles without default_permission_mode
rg -nP 'provider_profiles.*:' --type=yaml --type=ts -A5 | rg -v 'default_permission_mode'Repository: nrslib/takt
Length of output: 10104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) 変更対象の実ファイル内容を確認
echo "---- globalConfigCore.ts (around default_permission_mode) ----"
nl -ba src/infra/config/global/globalConfigCore.ts | sed -n '140,210p'
echo "---- schema-base.ts (ProviderPermissionProfileSchema) ----"
# Try to locate by symbol name first
if rg -n "ProviderPermissionProfileSchema" src/core/models/schema-base.ts >/dev/null; then
nl -ba src/core/models/schema-base.ts | sed -n '1,240p'
# Narrow further if file is huge
nl -ba src/core/models/schema-base.ts | sed -n '240,520p'
rg -n "ProviderPermissionProfileSchema" -n src/core/models/schema-base.ts
else
rg -n "default_permission_mode" src/core/models/schema-base.ts
rg -n "PermissionProfileSchema" src/core/models/schema-base.ts
fi
# 2) default_permission_mode が YAML/fixtures/test で実際に書かれているか確認
echo "---- Search fixtures/configs for default_permission_mode ----"
rg -n "default_permission_mode" -S e2e fixtures builtins src/__tests__ src -g'*.yaml' -g'*.yml'
echo "---- Files that include provider_profiles but may omit default_permission_mode ----"
# List YAML files with provider_profiles
files=$(rg -l "provider_profiles:" -S -g'*.yaml' -g'*.yml' e2e fixtures builtins src/__tests__ src || true)
echo "$files" | sed 's/^/ - /'
# For each such file, check whether default_permission_mode appears anywhere in the file.
echo "---- provider_profiles files: contains default_permission_mode? ----"
if [ -n "${files:-}" ]; then
for f in $files; do
if rg -n "default_permission_mode" "$f" >/dev/null; then
echo "[OK] $f has default_permission_mode"
else
echo "[MISS] $f lacks default_permission_mode"
fi
done
fi
# 3) CHANGELOG / migration 記載の探索（前回の fd の誤指定を回避）
echo "---- CHANGELOG references for permission mode / kiro provider / default_permission_mode ----"
ls -la
if [ -f CHANGELOG.md ]; then
rg -n "kiro|default_permission_mode|permission_mode|provider_profiles" CHANGELOG.md || true
else
echo "No CHANGELOG.md at repo root"
fi
echo "---- Search docs for MIGRATION ----"
rg -n "MIGRATION|migration" -S -g'*.md' . || true
# 4) 既存のデフォルト補完があるか（resolution 側）
echo "---- Permission mode resolution (permission-profile-resolution.ts) ----"
nl -ba src/core/workflow/permission-profile-resolution.ts | sed -n '1,120p'
echo "---- provider-profiles.ts (DEFAULT_PROVIDER_PROFILE_PERMISSION_MODE) ----"
nl -ba src/core/models/provider-profiles.ts | sed -n '1,120p'Repository: nrslib/takt
Length of output: 179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- globalConfigCore.ts (look for default_permission_mode) ----"
rg -n "default_permission_mode" src/infra/config/global/globalConfigCore.ts
sed -n '140,210p' src/infra/config/global/globalConfigCore.ts | cat -n
echo "---- schema-base.ts (look for ProviderPermissionProfileSchema / default_permission_mode) ----"
rg -n "ProviderPermissionProfileSchema" src/core/models/schema-base.ts || true
rg -n "default_permission_mode" src/core/models/schema-base.ts || true
# Show surrounding definitions for any match in schema-base.ts
for m in $(rg -n "default_permission_mode" src/core/models/schema-base.ts | cut -d: -f1 | head -n 20); do
start=$((m-20))
end=$((m+60))
echo "---- schema-base.ts lines ${start}-${end} (around $m) ----"
sed -n "${start},${end}p" src/core/models/schema-base.ts | cat -n
done
echo "---- Permission mode resolution (permission-profile-resolution.ts) ----"
rg -n "DEFAULT_PROVIDER_PROFILE_PERMISSION_MODE|defaultPermissionMode|permission mode" src/core/workflow/permission-profile-resolution.ts
sed -n '1,140p' src/core/workflow/permission-profile-resolution.ts | cat -n
echo "---- provider-profiles.ts (DEFAULT_PROVIDER_PROFILE_PERMISSION_MODE) ----"
sed -n '1,120p' src/core/models/provider-profiles.ts | cat -n
echo "---- Search for default_permission_mode in YAML/fixtures/builtins/tests ----"
rg -n "default_permission_mode" -S \
-g'*.yaml' -g'*.yml' \
e2e fixtures builtins src/__tests__ src || true
echo "---- Files containing provider_profiles but missing default_permission_mode ----"
files=$(rg -l "provider_profiles:" -S -g'*.yaml' -g'*.yml' e2e fixtures builtins src/__tests__ src || true)
if [ -z "${files}" ]; then
echo "No provider_profiles YAML files found by search."
exit 0
fi
fail=0
while IFS= read -r f; do
if ! rg -n "default_permission_mode" "$f" >/dev/null 2>&1; then
echo "[MISS] $f"
fail=1
fi
done <<< "$files"
exit $failRepository: nrslib/takt
Length of output: 15009
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- builtins/*/config.yaml around provider_profiles ----"
for f in builtins/en/config.yaml builtins/ja/config.yaml; do
echo "### $f"
rg -n "provider_profiles:" "$f" || true
sed -n '70,110p' "$f" | cat -n
done
echo "---- e2e/fixtures/config.e2e.yaml: provider_profiles section ----"
rg -n "provider_profiles:|default_permission_mode:" e2e/fixtures/config.e2e.yaml || true
sed -n '1,60p' e2e/fixtures/config.e2e.yaml | cat -n
echo "---- e2e/specs/* for provider_profiles blocks (check for missing default_permission_mode) ----"
# Find test files that mention provider_profiles but not default_permission_mode anywhere in the file
spec_files=$(rg -l "provider_profiles:" e2e/specs -g'*.ts' || true)
if [ -z "${spec_files}" ]; then
echo "No e2e/specs/*.ts with provider_profiles found."
else
for f in $spec_files; do
if ! rg -n "default_permission_mode" "$f" >/dev/null 2>&1; then
echo "[MISS] $f has provider_profiles but no default_permission_mode"
# show nearby snippet
rg -n "provider_profiles:" "$f" -n -A8 | head -n 40
else
echo "[OK] $f"
fi
done
fi
echo "---- src/__tests__ for provider_profiles blocks (ts string) ----"
test_files=$(rg -l "provider_profiles:" src/__tests__ -g'*.ts' || true)
if [ -z "${test_files}" ]; then
echo "No src/__tests__/*.ts with provider_profiles found."
else
for f in $test_files; do
if ! rg -n "default_permission_mode" "$f" >/dev/null 2>&1; then
echo "[MISS] $f has provider_profiles but no default_permission_mode"
rg -n "provider_profiles:" "$f" -A10 | head -n 60
else
echo "[OK] $f"
fi
done
fiRepository: nrslib/takt
Length of output: 3803
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- CHANGELOG.md references ----"
if [ -f CHANGELOG.md ]; then
rg -n "kiro|default_permission_mode|permission_mode|provider_profiles|migration" CHANGELOG.md || true
else
echo "CHANGELOG.md not found"
fi
echo "---- provider_profiles: in all YAML/YML, check for default_permission_mode presence ----"
files=$(rg -l "provider_profiles:" -S -g'*.yaml' -g'*.yml' . || true)
echo "Found files:"
echo "$files" | sed 's/^/ - /'
fail=0
while IFS= read -r f; do
if ! rg -n "default_permission_mode" "$f" >/dev/null 2>&1; then
echo "[MISS] $f"
fail=1
fi
done <<< "$files"
exit $failRepository: nrslib/takt
Length of output: 1421
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "function normalizeProviderProfiles|normalizeProviderProfiles" src/infra/config/global/globalConfigCore.ts
rg -n "normalizeProviderProfiles" src/infra/config/global/globalConfigCore.ts -n -A80 -B20
# If it lives elsewhere, locate it
rg -n "normalizeProviderProfiles" src/infra/config -SRepository: nrslib/takt
Length of output: 10087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- src/infra/config/configNormalizers.ts: normalizeProviderProfiles/denormalizeProviderProfiles ----"
sed -n '1,260p' src/infra/config/configNormalizers.ts | cat -n
echo "---- Search legacy migration for default_permission_mode / permission_mode ----"
rg -n "default_permission_mode|defaultPermissionMode|permission_mode|required_permission_mode" src/infra/config -S
echo "---- src/infra/config/global/globalConfigLegacyMigration.ts (focus around permission/profile migration) ----"
# show file with line numbers but only relevant segments
if [ -f src/infra/config/global/globalConfigLegacyMigration.ts ]; then
rg -n "permission_mode|required_permission_mode|provider_profiles|default_permission_mode" src/infra/config/global/globalConfigLegacyMigration.ts || true
sed -n '1,260p' src/infra/config/global/globalConfigLegacyMigration.ts | cat -n
else
echo "File not found: src/infra/config/global/globalConfigLegacyMigration.ts"
fi
echo "---- Any other migration for global/project config (search) ----"
rg -n "legacyMigration|migrate|normalizeProviderProfiles|provider_profiles" src/infra/config/global src/infra/config/project -SRepository: nrslib/takt
Length of output: 14455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- CHANGELOG.md around provider_profiles section ----"
# Show the relevant area around the line numbers surfaced earlier
sed -n '1060,1120p' CHANGELOG.md | cat -n
echo "---- Search for 'default_permission_mode' in CHANGELOG.md (verbatim context) ----"
rg -n "default_permission_mode" CHANGELOG.md -n -C2 || true
echo "---- Search for 'migration' docs ----"
rg -n "MIGRATION|migration" -S -g'*.md' . || trueRepository: nrslib/takt
Length of output: 8156
default_permission_mode 必須化は既存設定に互換性影響（パースエラー）を生むため、注意喚起/移行記載が必要
provider_profiles の default_permission_mode は src/core/models/schema-base.ts の ProviderPermissionProfileSchema で必須になっており、src/infra/config/configNormalizers.ts の normalizeProviderProfiles でも省略時の補完はありません。結果として provider_profiles 内に default_permission_mode が無い設定は設定パース時に失敗します。ユーザー影響（破壊的変更）として、CHANGELOG.md の該当エントリに「省略不可であること／既存設定の対応方法」を追記してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/infra/config/global/globalConfigCore.ts` at line 168, provider_profiles の
default_permission_mode を必須化したため既存設定がパースエラーになる破壊的変更が発生します。CHANGELOG.md
に該当エントリを追加し、「default_permission_mode が必須になったこと」「許容値は 'readonly' | 'edit' |
'full' であること」「既存設定の移行手順（全ての provider_profiles に例えば default_permission_mode:
'readonly' を追加する方法や一括置換コマンドの例）」「影響を受けるコード箇所（ProviderPermissionProfileSchema と
normalizeProviderProfiles）を追記」してください。これによりユーザーが設定を更新してパースエラーを回避できるように記載してください。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
背景
Kiro CLI には headless 実行があり、TAKT の provider として扱える可能性がある。
公式ドキュメント上は
kiro-cli chat --no-interactive、KIRO_API_KEY、--trust-all-tools/--trust-tools、--resume-id、MCP 設定が提供されている。TAKT では Cursor / Copilot と同様の CLI provider パターンで、独立した
kiroprovider として追加するのが自然。目的
TAKT から Kiro CLI を provider として選択できるようにする。
または provider profile で利用できるようにする。
実装方針
ProviderType/ provider schema にkiroを追加する。src/infra/kiro/を新設し、Kiro CLI 用 client を実装する。src/infra/providers/kiro.tsを追加し、TAKT のProviderインターフェースへ接続する。kiro: new KiroProvider()を追加する。kiro_cli_pathkiro_api_keyTAKT_KIRO_CLI_PATHTAKT_KIRO_API_KEYkiro_api_keyは child process env のKIRO_API_KEYとして渡す。権限マッピング案
Kiro CLI の headless mode ではユーザー承認ができないため、TAKT の
permissionModeを Kiro の trust flag に変換する。full:--trust-all-toolsedit: 必要に応じて--trust-tools=read,grep,write,shell相当を検討read:--trust-tools=read,grep相当を検討Kiro の tool category 名と TAKT の
allowedToolsの意味が完全一致するとは限らないため、初期実装では保守的に扱う。初期スコープ
AgentResponse.contentに格納する。supportsStructuredOutput = falseとする。sessionId未取得でも許容する。--resume-idが利用可能な場合は、TAKT のsessionIdを渡せるようにする。初期スコープ外
modelから直接 CLI flag として渡すこと。--modelが確認できないため、初期実装では Kiro 側のchat.defaultModelまたは custom agent に任せる。mcpServersを実行ごとに直接 CLI 引数へ変換すること。.kiro/settings/mcp.json/~/.kiro/settings/mcp.jsonを読む設計のため、初期実装では Kiro 側設定に任せる。調査メモ
公式ドキュメント上の確認事項:
kiro-cli chat --no-interactive "your prompt here"KIRO_API_KEY--trust-all-tools,--trust-tools=<categories>--resume,--resume-id <ID>,--list-sessions.kiro/settings/mcp.jsonまたはkiro-cli mcp add/import0success,1failure,3MCP startup failureローカル確認では、現時点の環境に
kiro-cli/kiroは PATH 上に存在しなかった。確認方法
Kiro CLI が利用可能な環境で以下を確認する。
TAKT 実装後は以下を確認する。
provider: kiroを config / workflow / provider profile で指定できる。TAKT_KIRO_API_KEYまたはkiro_api_keyがKIRO_API_KEYとして Kiro CLI に渡る。TAKT_KIRO_CLI_PATHまたはkiro_cli_pathで executable path を上書きできる。permissionModeに応じて trust flag が付与される。参考
Execution Report
Workflow
takt-defaultcompleted successfully.Closes #773
Summary by CodeRabbit
リリースノート
New Features
Documentation