Conversation
|
@27mfp is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new Kimi CLI agent plugin with streaming JSONL parsing and event translation, registers and exports it as a builtin agent, integrates Kimi parsing into the TUI output pipeline, includes tests, updates skill-installer mapping, and adds documentation and a navigation entry. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant TUI as Ralph TUI
participant KimiPlugin as Kimi Agent Plugin
participant KimiCLI as Kimi CLI
participant Parser as Output Parser
participant Display as UI Display
User->>TUI: run command with Kimi agent
TUI->>KimiPlugin: execute(prompt, options)
KimiPlugin->>KimiPlugin: buildArgs() / getStdinInput()
KimiPlugin->>KimiCLI: spawn process (--print, --input-format text, --output-format stream-json)
KimiCLI-->>KimiPlugin: stream JSONL lines
KimiPlugin->>KimiPlugin: buffer partial lines, detect complete JSONL
loop per complete JSONL line
KimiPlugin->>KimiPlugin: parseKimiJsonLine() → AgentDisplayEvent[]
KimiPlugin->>Parser: emit events / onJsonlMessage
end
Parser->>Parser: formatKimiEventForDisplay()
Parser-->>Display: formatted segments (text, tool calls, errors)
Display-->>User: rendered output
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/plugins/agents/builtin/kimi.ts`:
- Around line 66-74: The current logic can emit duplicate error events when an
incoming object has both a content array and top-level error fields: the block
that iterates and pushes from event.content and the later check if (event.type
=== 'error' || event.error) can both run. Fix by short-circuiting after handling
event.content (e.g., return/continue or wrap the later checks in an else) so
once you push events from event.content you do not re-evaluate the top-level
error branch; specifically adjust the block that processes event.content and the
subsequent if (event.type === 'error' || event.error) / events.push calls to be
mutually exclusive.
In `@src/tui/output-parser.ts`:
- Around line 109-128: parseKimiJsonlLine currently rejects top-level text
events (e.g., {"type":"text","text":"..."}) because it only accepts objects with
parsed.role and parsed.content or error/type fields; update parseKimiJsonlLine
to also detect and accept top-level text events by checking if parsed.type ===
'text' and typeof parsed.text === 'string' (return { success: true, event:
parsed }), leaving the existing role/content and error checks intact so other
KimiEvent shapes are still validated.
- Around line 134-181: The function formatKimiEventForDisplay currently returns
undefined for top-level text events because it only treats missing content as
errors; update the early branch that checks !event.content to also handle
top-level text by returning event.text (or String(event.text)) when event.type
=== 'text' or event.text is present, optionally truncating to a sensible length,
before falling through to content processing; modify the block inside
formatKimiEventForDisplay that handles the absence of event.content (the if
(!event.content || !Array.isArray(event.content)) branch) to return the
top-level text value instead of undefined.
🧹 Nitpick comments (3)
src/plugins/agents/builtin/kimi.ts (2)
405-414: Redundant type checks invalidateModel.The parameter
modelis typed asstring, somodel === undefined(line 406) andtypeof model !== 'string'(line 410) are unreachable. This is harmless defensive code but adds unnecessary noise.Simplified version
override validateModel(model: string): string | null { - if (model === '' || model === undefined) { + if (model === '') { return null; } - // Kimi CLI accepts various model identifiers; basic validation only - if (typeof model !== 'string' || model.trim().length === 0) { + if (model.trim().length === 0) { return 'Model name must be a non-empty string'; } return null; }
30-31: Minor: redundant length check.
!jsonLinealready evaluates totruefor an empty string, makingjsonLine.length === 0redundant.Simplified guard
- if (!jsonLine || jsonLine.length === 0) return []; + if (!jsonLine) return [];src/tui/output-parser.ts (1)
1042-1060: Fragile string-prefix matching for segment colouring.The colour logic checks
displayText.startsWith('[Tool:')anddisplayText.startsWith('[Tool Error]')rather than inspecting the event structure. Other agents (OpenCode, Gemini, Codex) checkevent.typedirectly. IfformatKimiEventForDisplayformatting ever changes, this breaks silently.This is minor given Kimi tool events are nested in the
contentarray and don't have a simple top-leveltype, but worth noting for future maintainability.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive support for Moonshot AI's Kimi Code CLI as a new agent plugin for Ralph TUI. The implementation follows established patterns from existing agent plugins (Gemini, Codex, Cursor) and includes full integration with the TUI's output parsing, subagent tracing, and skill installation systems.
Changes:
- Added KimiAgentPlugin with stream-json output parsing and stdin prompt delivery
- Integrated Kimi parsing into TUI output-parser.ts for real-time event display
- Added comprehensive test suite (43 tests) covering meta, initialization, JSONL parsing, and edge cases
- Created detailed documentation page with installation instructions, configuration, and troubleshooting
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/plugins/agents/builtin/kimi.ts | Core plugin implementation with JSONL parsing, Windows UTF-8 compatibility, and stdin prompt delivery |
| src/plugins/agents/builtin/kimi.test.ts | Comprehensive test suite (401 lines, 43 tests) covering all plugin functionality |
| src/plugins/agents/builtin/index.ts | Plugin registration, factory export, and class export |
| src/setup/skill-installer.ts | Added kimi to AGENT_ID_MAP for skill installation support |
| src/tui/output-parser.ts | Added Kimi stream-json parsing functions and integrated into all 5 parsing entry points |
| website/content/docs/plugins/agents/kimi.mdx | Complete documentation with prerequisites, usage, configuration, and troubleshooting |
| website/lib/navigation.ts | Added Kimi to agent navigation (alphabetically positioned between GitHub Copilot and Kiro) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix duplicate error events: use else-if chain in parseKimiJsonLine to
prevent emitting both content-array errors and top-level error events
for the same JSON object
- Add missing top-level text event handling in output-parser.ts
parseKimiJsonlLine and formatKimiEventForDisplay so {type:"text"}
events are recognized and displayed
- Use shared extractErrorMessage utility instead of inline error
extraction, consistent with Gemini/Codex/Cursor plugins
- Remove redundant type checks in validateModel (parameter is already
typed as string)
- Remove redundant length check (empty string is already falsy)
- Replace fragile string-prefix matching for segment coloring with
event-structure-based detection
https://claude.ai/code/session_01FKwSxkif6BK1evwQ2RqNjt
- Fix duplicate error events: use else-if chain in parseKimiJsonLine to
prevent emitting both content-array errors and top-level error events
for the same JSON object
- Add missing top-level text event handling in output-parser.ts
parseKimiJsonlLine and formatKimiEventForDisplay so {type:"text"}
events are recognized and displayed
- Use shared extractErrorMessage utility instead of inline error
extraction, consistent with Gemini/Codex/Cursor plugins
- Remove redundant type checks in validateModel (parameter is already
typed as string)
- Remove redundant length check (empty string is already falsy)
- Replace fragile string-prefix matching for segment coloring with
event-structure-based detection
https://claude.ai/code/session_01FKwSxkif6BK1evwQ2RqNjt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/tui/output-parser.ts (1)
146-152: Consider using the sharedextractErrorMessageutility here for consistency withkimi.ts.
kimi.ts(line 69) usesextractErrorMessagefromutils.jsfor exactly this pattern. The inlinetypeof event.error === 'string' ? ... : ...chain duplicates that logic.♻️ Proposed refactor
Add the import at the top of the file:
+import { extractErrorMessage } from '../plugins/agents/utils.js';Then simplify the error block:
- if (event.type === 'error' || event.error) { - const msg = typeof event.error === 'string' ? event.error : - typeof event.error === 'object' && event.error !== null && 'message' in event.error - ? String((event.error as { message?: unknown }).message) - : event.message || 'Unknown error'; - return `Error: ${msg}`; - } + if (event.type === 'error' || event.error) { + const msg = extractErrorMessage(event.error) || extractErrorMessage(event.message) || 'Unknown error'; + return `Error: ${msg}`; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/output-parser.ts` around lines 146 - 152, Replace the inline error-extraction logic in the event handler with the shared utility: import extractErrorMessage from the utils module at the top of the file, then call extractErrorMessage(event.error ?? event) (or otherwise pass the event/error into extractErrorMessage) inside the block that currently computes msg and return `Error: ${...}`; update the error branch that checks event.type === 'error' || event.error to use extractErrorMessage instead of the ternary/object checks so the code mirrors the behavior used in kimi.ts.src/plugins/agents/builtin/kimi.ts (1)
402-407: Simplify or remove thevalidateModeloverride—both branches returnnull.The method always returns
nullregardless of input, making it functionally identical to the base class. The guard clause checking for empty strings is dead code. Either simplify toreturn null;or remove the override entirely.Proposed simplification
- override validateModel(model: string): string | null { - if (model === '' || model.trim().length === 0) { - return null; - } - return null; - } + override validateModel(_model: string): string | null { + return null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/agents/builtin/kimi.ts` around lines 402 - 407, The override validateModel in the class (function validateModel) always returns null and the empty-string guard is dead code; either remove the entire validateModel override so the base class implementation is used, or simplify the override to a single unconditional return null; (update the validateModel method in src/plugins/agents/builtin/kimi.ts accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tui/output-parser.ts`:
- Around line 1059-1061: The isError check in the output parser uses the loose
comparison event.error !== undefined which treats null as an error; update the
logic that sets isError (the const isError in the same block) to use a truthy
check (e.g. !!event.error) so its behavior matches formatKimiEventForDisplay;
specifically change the event.error !== undefined part of the isError expression
to a boolean/coercion check to ensure null is treated as no error.
---
Nitpick comments:
In `@src/plugins/agents/builtin/kimi.ts`:
- Around line 402-407: The override validateModel in the class (function
validateModel) always returns null and the empty-string guard is dead code;
either remove the entire validateModel override so the base class implementation
is used, or simplify the override to a single unconditional return null; (update
the validateModel method in src/plugins/agents/builtin/kimi.ts accordingly).
In `@src/tui/output-parser.ts`:
- Around line 146-152: Replace the inline error-extraction logic in the event
handler with the shared utility: import extractErrorMessage from the utils
module at the top of the file, then call extractErrorMessage(event.error ??
event) (or otherwise pass the event/error into extractErrorMessage) inside the
block that currently computes msg and return `Error: ${...}`; update the error
branch that checks event.type === 'error' || event.error to use
extractErrorMessage instead of the ternary/object checks so the code mirrors the
behavior used in kimi.ts.
Add Moonshot AI's Kimi Code CLI as a new agent plugin for Ralph TUI. - New KimiAgentPlugin with stream-json output parsing - TUI output parser integration for real-time formatted display - 43 unit tests with >50% code coverage - Documentation page (kimi.mdx) and navigation entry - AGENT_ID_MAP registration for skill installation - Windows compatibility with PYTHONUTF8 env var injection
The add-skill tool expects 'kimi-cli' not 'kimi' as the agent identifier. This was discovered during Linux end-to-end testing where skill installation reported 'Invalid agents: kimi'.
- Fix duplicate error events: use else-if chain in parseKimiJsonLine to
prevent emitting both content-array errors and top-level error events
for the same JSON object
- Add missing top-level text event handling in output-parser.ts
parseKimiJsonlLine and formatKimiEventForDisplay so {type:"text"}
events are recognized and displayed
- Use shared extractErrorMessage utility instead of inline error
extraction, consistent with Gemini/Codex/Cursor plugins
- Remove redundant type checks in validateModel (parameter is already
typed as string)
- Remove redundant length check (empty string is already falsy)
- Replace fragile string-prefix matching for segment coloring with
event-structure-based detection
https://claude.ai/code/session_01FKwSxkif6BK1evwQ2RqNjt
Change `event.error !== undefined` to `!!event.error` so that null is treated as no-error, matching the behavior of formatKimiEventForDisplay which uses the truthy check `event.error`.
0b222ef to
5ed32a7
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #306 +/- ##
==========================================
+ Coverage 44.93% 45.00% +0.07%
==========================================
Files 97 98 +1
Lines 30546 31020 +474
==========================================
+ Hits 13727 13962 +235
- Misses 16819 17058 +239
🚀 New features to boost your workflow:
|
|
Thanks @27mfp - fixeed a few outstanding issues and merging now - will be in the 0.9.0 release |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/plugins/agents/builtin/kimi.ts (1)
402-407: Deadifbranch invalidateModel— both code paths returnnull.The condition
model === '' || model.trim().length === 0is never meaningful here: (a)validateSetupalready guards withmodel !== ''before callingvalidateModel, and (b) even when the condition is true, it returnsnull— identical to the fallthrough. The entireifblock can be removed.♻️ Proposed fix
override validateModel(model: string): string | null { - if (model === '' || model.trim().length === 0) { - return null; - } return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/agents/builtin/kimi.ts` around lines 402 - 407, The validateModel method contains a dead if branch that always returns null and is redundant because validateSetup already guards model !== '' before calling validateModel; remove the entire conditional inside override validateModel (leave the method as a single return null) so the function is concise and no-op as intended (refer to validateModel and validateSetup to confirm the guard behavior).src/plugins/agents/builtin/kimi.test.ts (1)
114-174: SimplifyTestableKimiPlugin—protectedmethods are directly accessible in subclasses.The
as unknown as { buildArgs: ... }casts insidetestBuildArgs/testGetStdinInputare unnecessary; subclasses can callthis.buildArgs(prompt)directly. TypingpluginasTestableKimiPluginalso removes the repeated(plugin as TestableKimiPlugin)casts throughout the tests.♻️ Proposed simplification
describe('KimiAgentPlugin buildArgs', () => { - let plugin: KimiAgentPlugin; + let plugin: TestableKimiPlugin; - // Create a test subclass to access protected method class TestableKimiPlugin extends KimiAgentPlugin { testBuildArgs(prompt: string): string[] { - return (this as unknown as { buildArgs: (p: string) => string[] }).buildArgs(prompt); + return this.buildArgs(prompt); } testGetStdinInput(prompt: string): string | undefined { - return (this as unknown as { getStdinInput: (p: string) => string | undefined }).getStdinInput(prompt); + return this.getStdinInput(prompt); } } beforeEach(() => { plugin = new TestableKimiPlugin(); }); // ... test('includes --print for non-interactive mode', async () => { await plugin.initialize({}); - const args = (plugin as TestableKimiPlugin).testBuildArgs('test prompt'); + const args = plugin.testBuildArgs('test prompt'); expect(args).toContain('--print'); }); // (apply same pattern to all other tests in this describe block)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/agents/builtin/kimi.test.ts` around lines 114 - 174, The TestableKimiPlugin wrapper is over-using type casts; since protected methods are callable from subclasses, change testBuildArgs to call this.buildArgs(prompt) and testGetStdinInput to call this.getStdinInput(prompt) directly, and type the test variable plugin as TestableKimiPlugin (replace let plugin: KimiAgentPlugin with let plugin: TestableKimiPlugin) so you can remove all (plugin as TestableKimiPlugin) casts in tests; update references to buildArgs and getStdinInput accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tui/output-parser.ts`:
- Around line 76-105: formatKimiEventForDisplay and parseKimiJsonLine disagree
on how tool errors are detected (formatKimiEventForDisplay checks both
item.is_error and item.return_value?.is_error, while parseKimiJsonLine only
checks item.is_error); to fix, make the detection consistent by updating
parseKimiJsonLine to also check item.return_value?.is_error (and/or
item.return_value?.output/message) so it defensively handles both the direct
content form and any nested return_value form, and update the KimiEvent
type/comments if needed to reflect that both shapes are supported; ensure both
functions (formatKimiEventForDisplay and parseKimiJsonLine) use the same
predicate for tool error detection.
---
Nitpick comments:
In `@src/plugins/agents/builtin/kimi.test.ts`:
- Around line 114-174: The TestableKimiPlugin wrapper is over-using type casts;
since protected methods are callable from subclasses, change testBuildArgs to
call this.buildArgs(prompt) and testGetStdinInput to call
this.getStdinInput(prompt) directly, and type the test variable plugin as
TestableKimiPlugin (replace let plugin: KimiAgentPlugin with let plugin:
TestableKimiPlugin) so you can remove all (plugin as TestableKimiPlugin) casts
in tests; update references to buildArgs and getStdinInput accordingly.
In `@src/plugins/agents/builtin/kimi.ts`:
- Around line 402-407: The validateModel method contains a dead if branch that
always returns null and is redundant because validateSetup already guards model
!== '' before calling validateModel; remove the entire conditional inside
override validateModel (leave the method as a single return null) so the
function is concise and no-op as intended (refer to validateModel and
validateSetup to confirm the guard behavior).
Summary
Adds Moonshot AI's Kimi Code CLI as a new agent plugin for Ralph TUI.
Changes
New Files
Modified Files
kimi: 'kimi'to AGENT_ID_MAPHow Kimi CLI Integration Works
kimi --print --input-format text --output-format stream-json [--model NAME]PYTHONUTF8=1andPYTHONIOENCODING=utf-8are injected to prevent charmap encoding errorsChecklist (per CONTRIBUTING.md)
Code Quality
bun run typecheck)bun run lint0 errors)Documentation
Agent Plugin Checklist
Summary by CodeRabbit
New Features
Tests
Documentation
Chores