Skip to content

fix: use kiro-cli-chat directly to prevent orphan child processes#129

Open
vokako wants to merge 2 commits intoopenclaw:mainfrom
vokako:fix/kiro-orphan-process-v2
Open

fix: use kiro-cli-chat directly to prevent orphan child processes#129
vokako wants to merge 2 commits intoopenclaw:mainfrom
vokako:fix/kiro-orphan-process-v2

Conversation

@vokako
Copy link
Copy Markdown
Contributor

@vokako vokako commented Mar 13, 2026

Problem

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 — the child process kiro-cli-chat continues running as an orphan.

These orphaned processes accumulate over time and cause resource conflicts, resulting in ACP_TURN_FAILED errors (exit code 1) on subsequent messages in persistent sessions. The first message in a session works fine, but follow-up messages fail because the stale kiro-cli-chat process is still holding resources.

Fix

Point the built-in kiro agent directly at kiro-cli-chat acp, bypassing the wrapper entirely. This ensures clean process lifecycle management without needing process group kill logic.

One-line change in src/agent-registry.ts:

-  kiro: "kiro-cli acp",
+  kiro: "kiro-cli-chat acp",

Related

Testing

  • Created persistent kiro session with acpx kiro sessions new
  • Sent multiple consecutive messages — all succeeded with session reuse
  • Verified only 1 kiro-cli-chat acp process running (no orphans)
  • Previously: 6+ orphan processes accumulated after a few session cycles

@vokako vokako force-pushed the fix/kiro-orphan-process-v2 branch from 9ea1acc to 2316e1a Compare March 13, 2026 01:59
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)
@vokako vokako force-pushed the fix/kiro-orphan-process-v2 branch from 2316e1a to f7bd853 Compare March 13, 2026 02:48
@dutifulbob
Copy link
Copy Markdown
Collaborator

Triage result

Human attention: ⚠️ Required
Recommendation: 🏁 escalate to a human
Human decision needed: decide whether to require manual Kiro validation before landing, or approve based on code review and normal CI alone.

Quick read

This PR is a focused bug fix: it changes the built-in kiro command from kiro-cli acp to kiro-cli-chat acp so acpx talks to the real ACP server directly instead of a wrapper.

The initial merge conflict against origin/main was limited to skills/acpx/SKILL.md and had a clear mechanical resolution path.

The remaining blocker in this flow is validation: the fix was not proven locally here because no targeted repro/test command could be derived from the repo state for this Kiro-specific behavior.

Intent

Change acpx's built-in kiro agent to launch Kiro's real ACP server directly so Kiro sessions shut down cleanly and persistent follow-up messages keep working.

Underlying problem: kiro-cli acp is a wrapper that spawns kiro-cli-chat; acpx only terminates the direct child, so stale kiro-cli-chat processes remain and later persistent-session turns can fail.

Why

  • src/client.ts shuts agents down by ending stdin and then sending SIGTERM/SIGKILL to the spawned child only; it does not do process-tree cleanup.
  • The PR changes src/agent-registry.ts from kiro-cli acp to kiro-cli-chat acp, which directly matches that failure mode.
  • The repo already uses agent-specific built-in command mappings, and the PR updates the related docs and skill docs consistently.

Codex review

  • Solution verdict: good_enough
  • Validation path: bug
  • Validation status: fix_not_proven
  • No targeted repro/test command could be derived from the local repo context for this Kiro-specific lifecycle issue.
  • No deeper Codex review findings were produced in this handoff path.

CI/CD

  • CI status in this run: not evaluated here.
  • Final conflict-gate status in this run: not available.
  • Initial conflict status: detected against origin/main in skills/acpx/SKILL.md, with a clear resolution path.

Recommendation

Escalate to a human. The code change is small and plausibly correct, but someone needs to decide whether normal repo review/CI is sufficient here or whether this PR should get manual Kiro-specific verification before landing:

  • reproduce the prior failure in a persistent Kiro session if possible
  • confirm follow-up turns now succeed
  • confirm kiro-cli-chat does not remain orphaned after session shutdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants