feat: add Kiro CLI as built-in agent with proper process group cleanup#42
feat: add Kiro CLI as built-in agent with proper process group cleanup#42thepagent wants to merge 6 commits intoopenclaw:mainfrom
Conversation
8f14577 to
bec9f62
Compare
* docs: update acp_kiro.md with process group fix and PR #42 reference * fix: correct acpx PR link to openclaw/acpx#42 * ci: retrigger check-commit-author * ci: retrigger after check-commit-author fix --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
|
Is this ready and tested? happy to merge it |
|
Hi @osolmaz, yes it's basically doing well and we have a group of people using this now on daily basis. I think it's safe to ship. Will resolve the conflicts and turn into ready for review. |
…ard ACP session/update
… processes kiro-cli is a wrapper that forks kiro-cli-chat as the actual ACP server. Sending SIGTERM to the wrapper does not kill the child process, which becomes an orphan and accumulates over time. Fix: spawn the agent with detached:true so it becomes a process group leader, then use process.kill(-pgid, SIGTERM/SIGKILL) to kill the entire process group including all child processes. Also adds onAgentPid callback to runQueuedTask/runSessionPrompt to allow callers to track the agent pid for cleanup purposes.
fa8889e to
5cd01fa
Compare
|
Thanks for the PR. I’m going to close this one in favor of #40. The Kiro registry addition itself looks useful, but this branch also folds in broader process cleanup/runtime behavior changes. I want to keep the Kiro built-in addition narrow and consistent with the other provider additions, so I’m taking the smaller PR for that. If you want, please feel free to reopen the process cleanup part as a separate focused PR. I think that piece deserves its own review on the runtime behavior rather than being bundled into the provider add. |
kiro-cli is a wrapper that forks kiro-cli-chat as the actual ACP server process. When acpx sends SIGTERM to kiro-cli on session close, only the wrapper is killed while kiro-cli-chat continues running as an orphan process. These orphaned processes accumulate over time and cause port/resource conflicts that result in ACP_TURN_FAILED errors on subsequent messages in persistent sessions. Fix: point the built-in kiro agent directly at kiro-cli-chat acp, bypassing the wrapper entirely. This ensures clean process lifecycle without needing process group kill logic. Related: openclaw#42 (proposed process group cleanup approach — this commit provides a simpler fix by avoiding the wrapper altogether)
kiro-cli is a wrapper that forks kiro-cli-chat as the actual ACP server process. When acpx sends SIGTERM to kiro-cli on session close, only the wrapper is killed while kiro-cli-chat continues running as an orphan process. These orphaned processes accumulate over time and cause port/resource conflicts that result in ACP_TURN_FAILED errors on subsequent messages in persistent sessions. Fix: point the built-in kiro agent directly at kiro-cli-chat acp, bypassing the wrapper entirely. This ensures clean process lifecycle without needing process group kill logic. Related: openclaw#42 (proposed process group cleanup approach — this commit provides a simpler fix by avoiding the wrapper altogether)
kiro-cli is a wrapper that forks kiro-cli-chat as the actual ACP server process. When acpx sends SIGTERM to kiro-cli on session close, only the wrapper is killed while kiro-cli-chat continues running as an orphan process. These orphaned processes accumulate over time and cause port/resource conflicts that result in ACP_TURN_FAILED errors on subsequent messages in persistent sessions. Fix: point the built-in kiro agent directly at kiro-cli-chat acp, bypassing the wrapper entirely. This ensures clean process lifecycle without needing process group kill logic. Related: openclaw#42 (proposed process group cleanup approach — this commit provides a simpler fix by avoiding the wrapper altogether)
kiro-cli is a wrapper that forks kiro-cli-chat as the actual ACP server process. When acpx sends SIGTERM to kiro-cli on session close, only the wrapper is killed while kiro-cli-chat continues running as an orphan process. These orphaned processes accumulate over time and cause port/resource conflicts that result in ACP_TURN_FAILED errors on subsequent messages in persistent sessions. Fix: point the built-in kiro agent directly at kiro-cli-chat acp, bypassing the wrapper entirely. This ensures clean process lifecycle without needing process group kill logic. Related: openclaw#42 (proposed process group cleanup approach — this commit provides a simpler fix by avoiding the wrapper altogether)
Summary
Adds Kiro CLI as a built-in agent and fixes orphan child processes caused by wrapper binary patterns.
Problem
kiro-cli acpis a wrapper that forkskiro-cli-chatas the actual ACP server. The oldagent.kill()only killed the wrapper — the child became an orphan and accumulated over time.This affects any agent using a wrapper binary pattern.
Fix
Spawn with
detached: trueso the agent becomes a process group leader. On close, useprocess.kill(-pgid, SIGTERM)to kill the entire group.Falls back to
child.kill()if process group kill is unsupported.Tests
Two new tests in
test/process-group-kill.test.tsprove the fix:process group kill terminates parent and grandchild (no orphan)— verifies the fix workskilling only parent pid leaves grandchild as orphan (demonstrates the bug)— reproduces the original bugChanges
src/agent-registry.ts— addkiro: "kiro-cli acp"as built-in agentsrc/client.ts—detached: trueon spawn +process.kill(-pgid)interminateAgentProcesssrc/session-runtime.ts— addonAgentPidcallback torunQueuedTask/runSessionPrompttest/agent-registry.test.ts— update for 6 agentstest/process-group-kill.test.ts— new: prove process group kill vs orphanRelated