Repository navigation
Fix Kimi hook config path and migrate legacy block - #8278
Conversation
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates Kimi hook installation to use ChangesKimi hook configuration support
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant HooksCLI
participant CurrentConfig
participant LegacyConfig
User->>HooksCLI: setup or uninstall kimi
HooksCLI->>CurrentConfig: install or remove cmux block
HooksCLI->>LegacyConfig: remove legacy cmux block
CurrentConfig-->>HooksCLI: updated configuration result
LegacyConfig-->>HooksCLI: updated configuration result
HooksCLI-->>User: report operation result
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (24 passed)
✨ Finishing Touches🧪 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: 1
🤖 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 `@cmuxTests/KimiHookConfigLocationTests.swift`:
- Line 159: Replace the deprecated readDataToEndOfFile() call in
KimiHookConfigLocationTests with the throwing FileHandle.readToEnd() API,
updating the surrounding test flow to handle its optional result and propagate
or handle read errors appropriately while preserving the existing
output-processing behavior.
🪄 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
Run ID: 101e9677-496b-440e-97dd-22765ce2e407
📒 Files selected for processing (2)
cmux.xcodeproj/project.pbxprojcmuxTests/KimiHookConfigLocationTests.swift
Greptile SummaryThis PR fixes the Kimi hook integration by pointing cmux at the actual live config (
Confidence Score: 5/5Safe to merge — install and uninstall paths are well-isolated, legacy cleanup is best-effort, and all previously identified issues have been addressed. The refactor is mechanically straightforward: a catalog entry change, a two-location file rewrite with symlink-aware deduplication, and downgraded error handling for the legacy path. The new test suite covers fresh setup, env-var overrides, symlinked directories, user-declined confirmations, and unreadable legacy configs — matching every non-trivial branch in production. Localization has EN and JA entries consistent with the existing catalog. No correctness issues or rule violations were found in the changed files. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI as cmux CLI
participant Active as Active config
participant Legacy as Legacy config
User->>CLI: cmux hooks setup kimi
CLI->>Active: readAgentHookConfig
Active-->>CLI: current content or empty string
CLI->>CLI: compute activeEdit
alt paths are distinct after symlink resolution
CLI->>Legacy: readAgentHookConfig
alt read succeeds
Legacy-->>CLI: legacy content
CLI->>CLI: compute legacyEdit
else read fails
CLI->>CLI: "legacyCleanupFailed = true"
end
end
alt edits pending and no --yes flag
CLI->>User: show diff previews and Proceed prompt
alt user declines
CLI->>User: Aborted
end
end
alt activeEdit is set
CLI->>CLI: createDirectory if missing
CLI->>Active: write new content atomically
CLI->>User: hooks installed message
else
CLI->>User: already up to date message
end
alt legacyEdit is set
CLI->>Legacy: write stripped content atomically
CLI->>User: legacy removal message
end
alt legacyCleanupFailed
CLI->>User: stderr warning with re-run instructions
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI as cmux CLI
participant Active as Active config
participant Legacy as Legacy config
User->>CLI: cmux hooks setup kimi
CLI->>Active: readAgentHookConfig
Active-->>CLI: current content or empty string
CLI->>CLI: compute activeEdit
alt paths are distinct after symlink resolution
CLI->>Legacy: readAgentHookConfig
alt read succeeds
Legacy-->>CLI: legacy content
CLI->>CLI: compute legacyEdit
else read fails
CLI->>CLI: "legacyCleanupFailed = true"
end
end
alt edits pending and no --yes flag
CLI->>User: show diff previews and Proceed prompt
alt user declines
CLI->>User: Aborted
end
end
alt activeEdit is set
CLI->>CLI: createDirectory if missing
CLI->>Active: write new content atomically
CLI->>User: hooks installed message
else
CLI->>User: already up to date message
end
alt legacyEdit is set
CLI->>Legacy: write stripped content atomically
CLI->>User: legacy removal message
end
alt legacyCleanupFailed
CLI->>User: stderr warning with re-run instructions
end
Reviews (7): Last reviewed commit: "Merge origin/main into issue-8273-kimi-h..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@codex review |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 16 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxTests/KimiHookConfigLocationTests.swift (1)
157-163: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDrain subprocess output concurrently.
Reading the pipe to EOF before
waitUntilExit()can deadlock when CLI output fills the pipe buffer. Drain stdout/stderr while the process runs, then await process termination and collect the captured data.🤖 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 `@cmuxTests/KimiHookConfigLocationTests.swift` around lines 157 - 163, Update the subprocess execution flow around process.run() in KimiHookConfigLocationTests so output is drained concurrently while the process runs, rather than reading to EOF before waitUntilExit(). Keep stdout and stderr captured through the existing output pipe, await process termination, then collect the drained data for assertions.
🤖 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.
Outside diff comments:
In `@cmuxTests/KimiHookConfigLocationTests.swift`:
- Around line 157-163: Update the subprocess execution flow around process.run()
in KimiHookConfigLocationTests so output is drained concurrently while the
process runs, rather than reading to EOF before waitUntilExit(). Keep stdout and
stderr captured through the existing output pipe, await process termination,
then collect the drained data for assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4437dd05-90d4-4d5a-acff-807aa172ebe4
📒 Files selected for processing (7)
CLI/CMUXCLI+AgentHookCatalog.swiftCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+KimiHooks.swiftcmuxTests/KimiHookConfigLocationTests.swiftdocs/agent-hooks.mddocs/cli-contract.mddocs/feed.md
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@CLI/CMUXCLI`+KimiHooks.swift:
- Around line 104-114: Update the catch block in the legacy cleanup flow to
include String(describing: error) in the localized warning, preserving the
existing active and legacy config paths while exposing the underlying removal
failure for diagnostics.
🪄 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
Run ID: a851140a-f9a2-43a7-8a63-216464f829d1
📒 Files selected for processing (3)
CLI/CMUXCLI+KimiHooks.swiftResources/Localizable.xcstringscmuxTests/KimiHookConfigLocationTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cmuxTests/KimiHookConfigLocationTests.swift`:
- Around line 168-173: Align the Kimi hook installation implementation and tests
on one confirmation policy for legacy cleanup: update the flow exercised by the
decline case so legacy processing and path output match the intended behavior,
and update the already-current active-config case accordingly so its automatic
legacy cleanup behavior is consistent. Apply the corresponding expectations at
cmuxTests/KimiHookConfigLocationTests.swift lines 168-173 and 192-213; inspect
the surrounding installation/confirmation symbols to preserve the chosen policy
across both scenarios.
🪄 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
Run ID: 69db8be5-2724-4f20-b10d-6700834b2363
📒 Files selected for processing (1)
cmuxTests/KimiHookConfigLocationTests.swift
|
Updated review fix pushed at @codex review |
|
To use Codex here, create a Codex account and connect to github. |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 16 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Commit summary ==='
git show --no-ext-diff --stat --oneline 019e624dfa
echo
echo '=== Incremental diff ==='
git show --no-ext-diff --format=fuller --find-renames --find-copies 019e624dfa -- \
'CLI/CMUXCLI+KimiHooks.swift' \
'cmuxTests/KimiHookConfigLocationTests.swift'
echo
echo '=== Affected Swift file outlines ==='
for f in CLI/CMUXCLI+KimiHooks.swift cmuxTests/KimiHookConfigLocationTests.swift; do
if [[ -f "$f" ]]; then
echo "--- $f ---"
ast-grep outline "$f" --items all --view expanded || true
fi
done
echo
echo '=== Relevant Kimi uninstall/setup symbols and tests ==='
rg -n -C 5 \
'func (uninstallKimiHooks|removeKimiHooks|installKimiHooks)|legacyKimiConfigURL|canonicalKimiConfigURL|unreadable|uninstall kimi' \
CLI/CMUXCLI+KimiHooks.swift cmuxTests/KimiHookConfigLocationTests.swiftLength of output: 13546 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Kimi hook implementation ==='
nl -ba CLI/CMUXCLI+KimiHooks.swift | sed -n '1,280p'
echo
echo '=== Kimi config location tests ==='
nl -ba cmuxTests/KimiHookConfigLocationTests.swift | sed -n '1,430p'
echo
echo '=== New localization entries and adjacent Kimi keys ==='
rg -n -C 6 '"cli\.hooks\.kimi\.(legacyCleanupWarning|legacyUninstallWarning)"|legacy Kimi|legacy hook block' \
Resources/Localizable.xcstrings
echo
echo '=== Existing warning/reporting patterns for tolerant cleanup failures ==='
rg -n -C 5 'report.*Warning|could not remove|cleanup.*fail|Legacy.*Warning' \
CLI cmuxTests Resources/Localizable.xcstringsLength of output: 1046 🏁 Script executed: #!/bin/bash
set -euo pipefail
print_range() {
local file="$1"
local start="$2"
local end="$3"
echo "=== ${file}:${start}-${end} ==="
awk -v start="$start" -v end="$end" 'NR >= start && NR <= end { printf "%6d %s\n", NR, $0 }' "$file"
}
print_range CLI/CMUXCLI+KimiHooks.swift 1 280
echo
print_range cmuxTests/KimiHookConfigLocationTests.swift 1 430
echo
echo '=== New localization entries and adjacent Kimi keys ==='
rg -n -C 6 '"cli\.hooks\.kimi\.(legacyCleanupWarning|legacyUninstallWarning)"|legacy Kimi|legacy hook block' \
Resources/Localizable.xcstrings || true
echo
echo '=== Existing warning/reporting patterns for tolerant cleanup failures ==='
rg -n -C 5 'report.*Warning|could not remove|cleanup.*fail|Legacy.*Warning' \
CLI cmuxTests Resources/Localizable.xcstrings || trueLength of output: 51324
The change correctly keeps active-config uninstall failures fatal, tolerates only legacy cleanup failures, emits an actionable localized warning, and adds regression coverage that verifies the active cmux block is removed while the unreadable legacy path is preserved. ✅ Action performedReview finished.
|
1 participant
Summary
~/.kimi/config.tomlKIMI_SHARE_DIRoverride and retire the unsupportedKIMI_CODE_HOMEoverrideKIMI_CODE_HOME/~/.kimi-codeconfigs during setup and uninstallNotificationreplaces unsupportedPermissionRequest; unsupportedInterruptis removed)[[hooks]]entries in both filesArchitecture
AgentHookDefremains the single source of truth for the active vendor config contract. Kimi-specific legacy cleanup stays in the existing TOML-format installer/uninstaller, so the migration does not add generic catalog fields that no other agent needs. The marker-block transformer remains the only content mutation path.Regression coverage
The first commit contains executable bundled-CLI regression tests only and is intentionally red against the old destination. The second commit fixes setup,
KIMI_SHARE_DIR, legacy cleanup, uninstall cleanup, and the Kimi 1.49 event set, preserving the red-to-green history.Verification
swift testinPackages/macOS/CMUXAgentLaunch: 235 tests passed./scripts/lint-pbxproj-test-wiring.sh: 517 test files checked./scripts/reload.sh --tag issue-8273-kimi: succeeded without launchkimi_cli.config.load_config: passedkimi --quiet --prompt "Say hi in exactly two words.": returnedHello there!CMUX_BUNDLED_CLI_PATH: exactly onehooks kimi stopinvocation, plus one session-start, prompt-submit, and session-end invocation~/.kimi/config.toml: seven vibe-island references retained and one cmux marker block installedThe repository revision does not contain
scripts/swift_file_length_budget.pyor.github/swift-file-length-budget.tsv, so the requested checker could not run. Manual counts are 174 lines forCMUXCLI+KimiHooks.swiftand 193 for the new test file; neither budget TSV was changed. The localization audit found no new runtime string keys, parsedLocalizable.xcstrings, and updated all English Markdown surfaces that named the old path; these docs have no parallel localized Markdown catalogs.Fixes #8273
Summary by CodeRabbit
PermissionRequest/InterrupttowardNotification.~/.kimi/config.toml, with legacy marker-block cleanup supported during setup/uninstall.