Skip to content

feat(providers): add Kiro CLI adapters for LLM completion and agent execution#303

Open
BangShinChul wants to merge 4 commits intoQ00:mainfrom
BangShinChul:feat/kiro-cli-adapter
Open

feat(providers): add Kiro CLI adapters for LLM completion and agent execution#303
BangShinChul wants to merge 4 commits intoQ00:mainfrom
BangShinChul:feat/kiro-cli-adapter

Conversation

@BangShinChul
Copy link
Copy Markdown

Implements KiroCodeAdapter and KiroAgentAdapter — new provider and orchestrator adapters that shell out to kiro-cli chat --no-interactive for LLM completions and --trust-all-tools for autonomous agent execution.

This lets Ouroboros leverage a local Kiro CLI session, following the same subprocess pattern as ClaudeCodeAdapter and CodexCliLLMAdapter.

What changed

New adapters

File Description
providers/kiro_adapter.py KiroCodeAdapter — single-response LLM completion via subprocess, with retry, timeout, model-name mapping
orchestrator/kiro_adapter.py KiroAgentAdapter — autonomous agent execution with line-by-line streaming, retry logic, LLMAdapter bridge

Config & factory wiring

File Description
config/models.py Add "kiro" to runtime_backend Literal, add kiro_cli_path field
config/loader.py Add get_runtime(), get_kiro_cli_path() (OUROBOROS_RUNTIME, OUROBOROS_KIRO_CLI_PATH env vars)
config/__init__.py Export new helpers
providers/factory.py Register "kiro" / "kiro_cli" backend, wire into create_llm_adapter()
orchestrator/factory.py Wire KiroAgentAdapter into create_agent_runtime()
providers/__init__.py Export KiroCodeAdapter

Docs & tests

File Description
README.md, README.ko.md Add Kiro CLI quick-start guide
tests/unit/test_kiro_adapters.py 25 unit tests — command building, streaming, timeout, retry, env flags, prompt building, process cleanup

How to use

Set runtime in .env:

OUROBOROS_RUNTIME=kiro

Or via environment variable:

OUROBOROS_RUNTIME=kiro ooo interview "Build a task CLI"

Copy link
Copy Markdown
Owner

@Q00 Q00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see three blocking issues here. First, the new Kiro quick-start only tells users to set OUROBOROS_RUNTIME=kiro, but src/ouroboros/cli/commands/init.py now calls create_llm_adapter() without an explicit backend, and create_llm_adapter() still resolves from llm.backend rather than OUROBOROS_RUNTIME. That means interview/seed/QA flows can stay on the old LLM backend even though the docs say runtime selection is enough.

Second, src/ouroboros/mcp/tools/qa.py now hardcodes model="claude-sonnet-4-20250514" and drops the backend-aware get_qa_model(...) path, so non-Claude backends ignore configured QA models and the Kiro path gets a Claude model id.

Third, the kiro branch in src/ouroboros/providers/factory.py discards cwd, timeout, max_retries, and related adapter options, and src/ouroboros/providers/kiro_adapter.py always launches the subprocess with cwd=os.getcwd(). That makes Kiro behave differently from the existing adapters and breaks callers that rely on per-request working-directory control for brownfield interview / repo-aware flows.

@BangShinChul BangShinChul force-pushed the feat/kiro-cli-adapter branch 3 times, most recently from 1b53822 to fd499a8 Compare April 4, 2026 03:53
@BangShinChul
Copy link
Copy Markdown
Author

@Q00 Please re-review with the updates below.


Add Kiro CLI runtime support

Summary

Adds Kiro CLI as a first-class runtime backend alongside Claude Code and Codex CLI. A single environment variable (OUROBOROS_RUNTIME=kiro) is sufficient to route both agent execution and LLM traffic through Kiro CLI.

Issues addressed

This PR resolves the three blocking issues from the previous review:

  1. OUROBOROS_RUNTIME not routed to LLM backendget_llm_backend() now falls back to OUROBOROS_RUNTIME, so create_llm_adapter(backend=None) in init.py, qa.py, and all other callers automatically resolves to kiro. No per-handler wiring needed.

  2. QA hardcodes Claude model ID for non-Claude backends_default_model_for_backend() and _normalize_configured_model_for_backend() now recognize _KIRO_LLM_BACKENDS and return the "default" sentinel (same pattern as Codex), so get_qa_model() never injects a Claude model ID on the kiro path.

  3. KiroCodeAdapter discards cwd/timeout/max_retriesKiroCodeAdapter now accepts all three parameters and create_llm_adapter() forwards them. KiroAgentAdapter also accepts cwd. Both follow the same contract as the existing Codex adapters.

Changes

Config (de253b5)

  • Add "kiro" to OrchestratorConfig.runtime_backend Literal type
  • Add kiro_cli_path field with ~ expansion validator (same pattern as codex_cli_path)
  • Add get_kiro_cli_path() and get_runtime() helpers
  • get_agent_runtime_backend() and get_llm_backend() now fall back to OUROBOROS_RUNTIME env var
  • Add _KIRO_LLM_BACKENDS / _KIRO_DEFAULT_MODEL for backend-aware model defaults

Adapters (7cfcafa)

  • KiroCodeAdapter (providers): subprocess LLM completion via kiro-cli chat --no-interactive, with cwd/timeout/max_retries support
  • KiroAgentAdapter (orchestrator): agent runtime via kiro-cli chat --no-interactive --trust-all-tools, with cwd support — implements the AgentRuntime protocol
  • Wired into existing factory functions following the same branching pattern as Codex
  • resolve_llm_permission_mode returns "default" for kiro (no permission concept)
  • Lazy import with try/except guard in orchestrator/__init__.py

Tests (f58c34a)

  • 29 unit tests covering: factory resolve/create, LLM completion (success, retry, file-not-found, cwd propagation), agent execution (streaming, error, cwd), OUROBOROS_RUNTIME fallback routing, model default sentinel mapping

Docs (fd499a8)

  • Kiro CLI quick-start section in README.md and README.ko.md
  • OUROBOROS_RUNTIME and OUROBOROS_KIRO_CLI_PATH in .env.example

Design decisions

  • No MCP tool changes needed: get_llm_backend() fallback to OUROBOROS_RUNTIME means all existing handlers (QA, interview, seed, evaluate) automatically route to kiro — same as how Claude Code/Codex work today
  • No new orchestrator/factory.py: kiro branch added directly to existing runtime_factory.py to avoid duplication
  • Existing backends unaffected: OUROBOROS_RUNTIME is only set in kiro environments; Claude Code/Codex users see no behavior change

How to test

# .env
OUROBOROS_RUNTIME=kiro

# All ooo commands work: interview, seed, run, evolve, qa, etc.

@BangShinChul BangShinChul requested a review from Q00 April 4, 2026 04:00
@BangShinChul BangShinChul force-pushed the feat/kiro-cli-adapter branch from fd499a8 to 2e10f19 Compare April 4, 2026 04:22
Copy link
Copy Markdown
Owner

@Q00 Q00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excited to see Kiro CLI integration coming! The adapter structure and factory wiring look solid. However, comparing against the existing ClaudeCodeAdapter and CodexCliRuntime implementations, there are several safety patterns that need to be ported over before this is production-ready.

Also, please rebase onto main — there are recent changes that may affect the config/factory layers.


Critical

C1: No timeout on stdout reading loop (orchestrator/kiro_adapter.py)

async for raw_line in proc.stdout:   # ← no idle timeout — blocks forever if kiro-cli hangs
    ...
await asyncio.wait_for(proc.wait(), timeout=_DEFAULT_TIMEOUT)  # ← never reached

CodexCliRuntime applies first_chunk_timeout_seconds (60s) + chunk_timeout_seconds (300s) on each stream.read() call. If kiro-cli hangs silently, this adapter will block indefinitely.

C2: No asyncio.CancelledError handling — subprocess orphaned on task cancellation

When the orchestrator aborts an AC, CancelledError propagates through the async generator. Without explicit handling, the subprocess may be orphaned. CodexCliRuntime explicitly catches CancelledError, terminates the process, then re-raises.

C3: No child environment sanitization

env = {**os.environ, "OUROBOROS_SUBAGENT": "1"}  # inherits everything

CodexCliLLMAdapter._build_child_env() strips CLAUDECODE, OUROBOROS_AGENT_RUNTIME, OUROBOROS_LLM_BACKEND to prevent nested session detection and recursive MCP server startup. Without this, invoking Kiro from inside a Claude Code session will inherit CLAUDECODE=1 and potentially conflict.


High

H1: No _OUROBOROS_DEPTH recursion guard

If Kiro calls back into Ouroboros (which calls Kiro again), there's no depth ceiling. Existing adapters enforce _MAX_OUROBOROS_DEPTH = 5.

H2: No concurrent stderr drain in agent adapter

Stdout is read line-by-line while stderr is only read after proc.wait(). If kiro-cli fills the stderr pipe buffer (~64KB), the process deadlocks. CodexCliRuntime runs asyncio.create_task(self._collect_stream_lines(process.stderr)) concurrently.

H3: No response_format / JSON enforcement

When config.response_format requests json_schema, ClaudeCodeAdapter injects the schema as a prompt instruction and uses extract_json_payload() with retry. Without this, structured-output callers (seed architect, evaluator, QA judge) will receive raw prose and fail silently.

H4: allowed_tools parameter ignored

Interview mode passes allowed_tools=[] to block all tool use. The Kiro adapter accepts this but never applies it (neither as a CLI flag nor as a prompt constraint). CodexCliLLMAdapter embeds tool restrictions directly in the prompt text.


Medium

M1: "kiro" missing from _RUNTIME_HANDLE_BACKEND_ALIASES

orchestrator/adapter.py has _RUNTIME_HANDLE_BACKEND_ALIASES for RuntimeHandle construction. Without a "kiro" entry, RuntimeHandle(backend="kiro") will raise ValueError.

M2: Yielded AgentMessages don't carry resume_handle

Both ClaudeAgentAdapter and CodexCliRuntime attach resume_handle=current_handle to every yielded message. The session manager inspects this for persistence. Without it, ooo status and the TUI dashboard won't show Kiro runtime state.

M3: "kiro_cli" alias missing from runtime backend map

adapter.py adds "kiro": "kiro" but not "kiro_cli": "kiro", inconsistent with the _KIRO_BACKENDS = {"kiro", "kiro_cli"} sets in runtime_factory.py and providers/factory.py.


Low

L1: <system> XML tags as CLI positional argument

The system prompt is wrapped in <system> tags and passed as a CLI arg. CodexCliLLMAdapter uses ## System Instructions markdown headers instead. Worth verifying that kiro-cli actually interprets XML system tags.

L2: max_turns not enforced

The parameter is accepted but not passed to the CLI. Interview's max_turns=1 has no effect.


Recommendation

The existing adapters (claude_code_adapter.py, codex_cli_adapter.py, codex_cli_runtime.py) have accumulated these safety patterns through real production incidents. I'd recommend using CodexCliRuntime as the primary reference since it's the closest architectural match (subprocess + streaming). The key patterns to port:

  1. _build_child_env() with env stripping + depth guard
  2. Concurrent stderr drain via asyncio.create_task
  3. Per-read idle timeout in the stdout loop
  4. CancelledError handling with process termination
  5. JSON enforcement layer for structured output

Looking forward to seeing Kiro fully integrated! 🎉

@BangShinChul BangShinChul force-pushed the feat/kiro-cli-adapter branch from 2e10f19 to 3fa2d7c Compare April 4, 2026 16:19
Copy link
Copy Markdown
Contributor

@ouroboros-agent ouroboros-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Reviewing commit 3fa2d7c | Triggered by: new push by @BangShinChul

Branch: feat/kiro-cli-adapter | 13 files, +968/-13 | CI: MyPy Type Check pass

Issue #N Requirements

No linked issue referenced in PR body.

Previous Review Follow-up

First review — no previous findings.

Blocking Findings

# File:Line Severity Confidence Finding
1 src/ouroboros/orchestrator/kiro_adapter.py:104-145 blocker high KiroAgentAdapter accepts permission_mode and exposes it via the protocol, but the spawned command always includes --trust-all-tools. That widens execution to full tool trust even when callers/config requested a stricter mode, so selecting the Kiro runtime silently disables the framework's runtime safety contract instead of honoring it.
2 src/ouroboros/orchestrator/kiro_adapter.py:147-166 blocker high execute_task() drains stdout to exhaustion before it reads stderr, while both pipes are configured on the child process. If kiro-cli emits enough stderr output to fill the pipe, the child blocks before closing stdout and the adapter hangs until timeout; this is a classic subprocess deadlock path for long-running/error-heavy tasks.
3 src/ouroboros/orchestrator/runtime_factory.py:68-77 + src/ouroboros/orchestrator/kiro_adapter.py:61-70 blocker medium create_agent_runtime() builds a skill_dispatcher for non-Claude runtimes, but KiroAgentAdapter.__init__ drops all extra kwargs via **_kwargs and never stores/uses that dispatcher. As a result, Kiro is not actually a drop-in replacement for the existing runtime path: deterministic ooo/skill interception wired for Codex is silently lost when users switch OUROBOROS_RUNTIME=kiro.
4 src/ouroboros/orchestrator/kiro_adapter.py:119-128 blocker medium The tools / resume_handle / resume_session_id protocol inputs are accepted but ignored. For tools this means callers cannot narrow capabilities at runtime; for resume it means any orchestration path expecting backend-neutral continuation gets a fresh session instead. Given the class is presented as an AgentRuntime drop-in, this is a contract break rather than a follow-up enhancement.
5 src/ouroboros/providers/factory.py:97-142 blocker medium The factory computes a resolved permission mode for every backend, but the Kiro branch cannot apply it at all. Combined with finding #1 this means the public factory surface suggests policy parity with other CLI adapters while the new backend effectively hardcodes its own execution policy. I would either implement equivalent policy handling end-to-end or make the backend fail fast when a non-default permission contract is requested.

Test Coverage

The new unit tests cover command construction, retries, and happy-path streaming, but they do not exercise the two behavior regressions above:

  • no test asserts that runtime permission settings are enforced (or rejected) for the Kiro runtime;
  • no test simulates simultaneous stdout/stderr activity or large stderr output to catch the deadlock path in execute_task();
  • no test verifies that Kiro preserves the command-dispatch / skill-intercept behavior expected from the non-Claude runtime factory path;
  • no test verifies resume-handle semantics or explicitly documents that Kiro does not support resumption.

Design Notes

The overall shape is good and the config/factory wiring is straightforward, but right now the runtime adapter is weaker than the interface it is plugging into. The main design gap is that the implementation advertises parity with the existing orchestrator runtime contract while hardcoding looser execution semantics and omitting runtime services (dispatch/resume/tool scoping) that other backends preserve. I would be comfortable merging once the adapter either (a) truly honors the same runtime contract, or (b) narrows its supported surface explicitly and rejects unsupported modes instead of silently widening behavior.

Follow-up Recommendations

  • Once the blocking contract issues are fixed, add a small integration-style test layer that exercises the runtime factory against a mocked kiro-cli process, not just the adapter in isolation.
  • Consider documenting the exact Kiro CLI guarantees (tool trust model, session/resume support, stderr format, model naming rules) in the README so reviewers/users can reason about backend differences.

Files Reviewed

  • .env.example
  • README.ko.md
  • README.md
  • src/ouroboros/config/__init__.py
  • src/ouroboros/config/loader.py
  • src/ouroboros/config/models.py
  • src/ouroboros/orchestrator/__init__.py
  • src/ouroboros/orchestrator/adapter.py
  • src/ouroboros/orchestrator/kiro_adapter.py
  • src/ouroboros/orchestrator/runtime_factory.py
  • src/ouroboros/providers/factory.py
  • src/ouroboros/providers/kiro_adapter.py
  • tests/unit/test_kiro_adapters.py

Reviewed by ouroboros-agent[bot] via fallback manual deep analysis

@BangShinChul BangShinChul force-pushed the feat/kiro-cli-adapter branch from dbaa37b to 925b2de Compare April 4, 2026 16:44
@BangShinChul BangShinChul reopened this Apr 4, 2026
Copy link
Copy Markdown
Contributor

@ouroboros-agent ouroboros-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Reviewing commit 925b2de | Triggered by: new push by @BangShinChul

Branch: feat/kiro-cli-adapter | 13 files, +1142/-13 | CI: Ruff Lint: failure; Tests 3.12/3.13/3.14: failure; MyPy: success

Issue #N Requirements

Requirement Status
No linked issue detected from PR body. The PR goal is clear from the description: add Kiro CLI LLM + agent adapters and wire them as drop-in backends.

Previous Review Follow-up

Previous Finding Status
execute_task() could deadlock by draining stdout before stderr. WITHDRAWN / RESOLVEDsrc/ouroboros/orchestrator/kiro_adapter.py:172-187 now drains stderr concurrently and :233-258 adds startup/idle timeout handling.
Kiro runtime ignored tool constraints / resume metadata / runtime-handle continuity. MODIFIED — runtime handles are now propagated (src/ouroboros/orchestrator/kiro_adapter.py:189-195, :211-315), but resume semantics are still ignored and tool constraints are only prompt text, not runtime-enforced policy.
Kiro runtime widened execution policy compared with the existing runtime contract. MAINTAINEDsrc/ouroboros/orchestrator/kiro_adapter.py:145-170 still hardcodes --trust-all-tools regardless of requested permission_mode.
Kiro runtime dropped Codex-style dispatcher behavior wired by the runtime factory. MAINTAINEDsrc/ouroboros/orchestrator/runtime_factory.py:68-94 still computes dispatcher-backed runtime kwargs for non-Claude runtimes, but the Kiro branch does not pass or consume that dispatcher.

Blocking Findings

# File:Line Severity Confidence Finding
1 src/ouroboros/orchestrator/kiro_adapter.py:145-170 blocker high KiroAgentAdapter still advertises a configurable permission_mode, but _build_cmd() always launches kiro-cli chat --no-interactive --trust-all-tools. That means choosing the Kiro runtime silently widens tool trust even when callers/config requested stricter execution, which breaks the existing runtime safety contract rather than rejecting unsupported modes explicitly.
2 src/ouroboros/orchestrator/runtime_factory.py:68-94 + src/ouroboros/orchestrator/kiro_adapter.py:82-97 blocker high create_agent_runtime() still constructs non-Claude runtime context around a dispatcher/LLM backend model, but the Kiro branch bypasses runtime_kwargs entirely and KiroAgentAdapter.__init__ discards extra kwargs. Switching OUROBOROS_RUNTIME=kiro therefore drops the command-dispatch / skill-intercept path that Codex-based runtimes preserve, so this is not yet a true drop-in replacement for the orchestrator contract described by the factory surface.
3 src/ouroboros/orchestrator/kiro_adapter.py:198-330 blocker medium execute_task() / execute_task_to_result() accept resume_handle and resume_session_id, but the values are never consulted when building the command or process state. The adapter now forwards a fresh handle on emitted messages, but backend-neutral continuation still restarts a new Kiro subprocess instead of resuming prior execution, which is a contract break for callers that rely on resumable runtimes.
4 tests/unit/test_kiro_adapters.py blocker medium The new tests cover command construction, JSON extraction, retries, and process cleanup, but they still do not pin the three contract gaps above: there is no test proving permission settings are enforced or rejected for Kiro, no test proving runtime-factory dispatcher behavior is preserved, and no test documenting/guarding resume semantics. Given the runtime is being presented as production-selectable via OUROBOROS_RUNTIME=kiro, those gaps make the remaining contract regressions easy to miss.

Test Coverage

The latest push clearly improved safety coverage around stderr draining, timeouts, JSON extraction, and runtime-handle propagation. Remaining coverage gaps are the contract-level ones:

  • add a test that permission_mode either changes Kiro behavior or raises a clear unsupported-mode error;
  • add a factory-level test that Kiro preserves (or explicitly rejects) the dispatcher/skill-intercept behavior expected from the non-Claude runtime path;
  • add a resume test that either exercises real resumption semantics or locks in a deliberate not supported failure instead of silent restart.

Design Notes

The shape of the adapter is much better after this push: the deadlock path is fixed, timeout handling is more robust, and runtime handles are now carried through the stream. The remaining problem is that the runtime still claims broader parity than it actually provides. I’d be comfortable merging once the Kiro backend either honors the same runtime contract as other selectable orchestrator runtimes, or narrows its surface explicitly and fails fast when unsupported features (permission policy, dispatcher-backed command routing, resume semantics) are requested.

Follow-up Recommendations

  • Once the contract blockers are resolved, the PR would benefit from a small mocked integration test that exercises create_agent_runtime(backend="kiro") end-to-end instead of only adapter-local behavior.
  • If Kiro fundamentally cannot support the same permission/resume model, document that difference prominently in README and reject those modes at runtime rather than silently widening behavior.

Files Reviewed

  • .env.example
  • README.ko.md
  • README.md
  • src/ouroboros/config/__init__.py
  • src/ouroboros/config/loader.py
  • src/ouroboros/config/models.py
  • src/ouroboros/orchestrator/__init__.py
  • src/ouroboros/orchestrator/adapter.py
  • src/ouroboros/orchestrator/kiro_adapter.py
  • src/ouroboros/orchestrator/runtime_factory.py
  • src/ouroboros/providers/factory.py
  • src/ouroboros/providers/kiro_adapter.py
  • tests/unit/test_kiro_adapters.py

Reviewed by ouroboros-agent[bot] via fallback manual deep analysis

@BangShinChul BangShinChul requested a review from Q00 April 4, 2026 16:49
- Add get_kiro_cli_path() config loader
- Route OUROBOROS_RUNTIME=kiro to LLM and agent runtime backends
- Add Kiro to backend alias sets in config models
Add KiroCodeAdapter (LLM protocol) and KiroAgentAdapter (AgentRuntime
protocol) as drop-in backends for kiro-cli subprocess execution.

Safety patterns ported from CodexCliRuntime:
- _build_child_env() with env stripping + _OUROBOROS_DEPTH guard (max=5)
- Per-read idle timeout on stdout (startup=60s, idle=300s)
- Concurrent stderr drain via asyncio.create_task
- CancelledError handling with process termination + re-raise
- JSON enforcement layer with extract_json_payload() + retry
- allowed_tools enforcement via prompt constraints

Runtime contract compliance:
- permission_mode mapped to --trust-tools/--trust-all-tools flags
- skill_dispatcher accepted from runtime_factory via **runtime_kwargs
- resume_session_id mapped to --resume flag
- resume_handle attached to all yielded AgentMessages
- 'kiro_cli' added to _RUNTIME_HANDLE_BACKEND_ALIASES
Coverage includes:
- providers/factory.py: resolve + create for kiro backend
- providers/kiro_adapter.py: LLM completion, retry, JSON extraction
- orchestrator/runtime_factory.py: resolve + create for kiro runtime
- orchestrator/kiro_adapter.py: streaming, error handling, cwd
- config/loader.py: OUROBOROS_RUNTIME fallback routing
- Contract compliance: permission_mode mapping, factory dispatcher
  passthrough, resume flag generation
- Add OUROBOROS_RUNTIME=kiro to .env.example
- Add Kiro CLI quick-start section to README.md and README.ko.md
@BangShinChul BangShinChul force-pushed the feat/kiro-cli-adapter branch from 925b2de to 57a60c3 Compare April 4, 2026 17:02
@BangShinChul
Copy link
Copy Markdown
Author

Update: Addressed all review feedback

Rebased onto main and squashed into 4 clean commits. All safety patterns from the initial review and all blocker findings from the review bot have been incorporated into the adapter code directly — no separate fix commits.

Initial Review (C1–C3, H1–H4, M1–M3)

Finding Resolution
C1: No stdout idle timeout asyncio.wait_for(readline(), timeout=) with startup (60s) and idle (300s) timeouts
C2: No CancelledError handling except asyncio.CancelledError_terminate_process() → re-raise; stderr task cleaned up in finally
C3: No child env sanitization _build_child_env() strips OUROBOROS_AGENT_RUNTIME, OUROBOROS_LLM_BACKEND, CLAUDECODE
H1: No recursion guard _OUROBOROS_DEPTH incremented + ceiling at 5, raises RuntimeError on overflow
H2: No concurrent stderr drain asyncio.create_task(_collect_stderr()) with deque(maxlen=512)
H3: No JSON enforcement Schema injected into prompt when response_format is json_schema/json_object; response extracted via
extract_json_payload() with retry
H4: allowed_tools ignored tools=[] → prompt constraint blocking all tools; specific list → allowlist in prompt text
M1: kiro_cli missing from aliases Added "kiro_cli": "kiro" to _RUNTIME_HANDLE_BACKEND_ALIASES
M2: No resume_handle on messages All yielded AgentMessages now carry resume_handle=current_handle
M3: Backend alias inconsistency Verified _KIRO_BACKENDS = {"kiro", "kiro_cli"} consistent across runtime_factory.py and providers/factory.py
L1: <system> XML tags kiro-cli has no CLI-level system tag parsing — tags are interpreted by the LLM; no change needed
L2: max_turns not enforced kiro-cli has no --max-turns flag (kiro-cli chat --help confirmed); H4 prompt constraints cover interview mode

Review Bot Blockers (B1–B4)

Finding Resolution
B1: permission_mode ignored, --trust-all-tools hardcoded Removed hardcoded flag. default--trust-tools='', acceptEdits/bypassPermissions
--trust-all-tools
B2: skill_dispatcher not accepted from factory Kiro now created via **runtime_kwargs (same path as Codex); skill_dispatcher parameter accepted
and stored
B3: resume_session_id silently ignored Mapped to --resume flag when provided
B4: No contract compliance tests Added tests for permission mapping, factory dispatcher passthrough, and resume flag generation

Commit structure

c8133ac feat(config): add Kiro CLI runtime selection and LLM backend routing
84fca52 feat(kiro): add Kiro CLI adapters for LLM and agent execution
bdda0ad test(kiro): add unit tests for Kiro CLI adapters
57a60c3 docs: add Kiro CLI quick-start guide and env config

Test results

  • 35/35 passed on Python 3.12 and 3.13
  • ruff check + ruff format --check clean

Copy link
Copy Markdown
Contributor

@ouroboros-agent ouroboros-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Reviewing commit 57a60c3 | Triggered by: re-review requested by @BangShinChul

Branch: feat/kiro-cli-adapter | 13 files, +1214/-13 | CI: MyPy Type Check pass; Ruff Lint pass; Tests 3.12/3.13/3.14 pass

Issue #N Requirements

Requirement Status
Add Kiro CLI as a selectable runtime + LLM backend so OUROBOROS_RUNTIME=kiro routes both orchestration and LLM flows through Kiro. PARTIALLY MET — config/loader + factory wiring now route the backend correctly (src/ouroboros/config/loader.py, src/ouroboros/providers/factory.py, src/ouroboros/orchestrator/runtime_factory.py), but two runtime-contract gaps remain in the agent adapter.

Previous Review Follow-up

Previous Finding Status
Timeout / stderr-drain / cancellation safety gaps in KiroAgentAdapter. WITHDRAWN / RESOLVED — the current adapter now drains stderr concurrently, applies startup/idle timeouts, and terminates the subprocess on cancellation (src/ouroboros/orchestrator/kiro_adapter.py:177-188, :238-287, :306-321).
Permission mode was hardcoded to full trust. WITHDRAWN / RESOLVED_build_permission_args() now maps default to --trust-tools= and only uses --trust-all-tools for the broader modes (src/ouroboros/orchestrator/kiro_adapter.py:138-150).
Kiro runtime dropped the dispatcher-backed runtime context. MAINTAINEDruntime_factory now passes skill_dispatcher, but KiroAgentAdapter still only stores it and never consults it during execution (src/ouroboros/orchestrator/runtime_factory.py:62-83, src/ouroboros/orchestrator/kiro_adapter.py:80-92, :206-330).
Resume semantics were missing / silently ignored. MODIFIED — the adapter now toggles a bare --resume flag, but it still ignores the supplied resume_session_id value itself (src/ouroboros/orchestrator/kiro_adapter.py:153-173).

Blocking Findings

# File:Line Severity Confidence Finding
1 src/ouroboros/orchestrator/runtime_factory.py:62-83 + src/ouroboros/orchestrator/kiro_adapter.py:80-92,206-330 blocker high create_agent_runtime() now constructs Kiro with the same skill_dispatcher context used for non-Claude runtimes, but KiroAgentAdapter never uses that dispatcher when executing a task. So switching OUROBOROS_RUNTIME=kiro still drops the dispatcher / skill-intercept behavior that the factory contract advertises for Codex-style runtimes. Storing the object without consulting it does not make Kiro a true drop-in replacement here.
2 src/ouroboros/orchestrator/kiro_adapter.py:153-173 blocker high resume_session_id is now accepted by _build_cmd(), but the provided value is never passed through to the child process — the code only appends a bare --resume. If callers hand you a specific session id, this implementation resumes “something” (whatever Kiro’s default resume target is) rather than the requested session, which is still a contract break for backend-neutral continuation.

Test Coverage

The safety/test story is much better in this revision: the new suite covers timeout handling, stderr draining, permission-mode flag mapping, and the factory now at least passes dispatcher context in. The remaining gaps are the two contract-level ones above:

  • add a runtime-level test that proves dispatcher-backed command interception actually occurs during execute_task(), not just that _skill_dispatcher is non-None;
  • add a resume test that verifies the requested resume_session_id value is propagated correctly to the CLI invocation (or that unsupported targeted resume fails fast with a clear error instead of silently degrading to generic --resume).

Design Notes

This is much closer. The earlier safety concerns are addressed, the LLM/runtime routing looks coherent, and the adapter shape now matches the surrounding code better. The remaining problem is specifically about backend-contract parity: the runtime factory presents Kiro as another selectable orchestrator runtime, but targeted resume and dispatcher-backed interception still do not behave like the existing non-Claude runtime path. I’d be happy to re-review once Kiro either honors those behaviors end-to-end or rejects them explicitly instead of silently degrading them.

Follow-up Recommendations

  • If Kiro CLI fundamentally cannot resume a specific session id, make that explicit in the adapter surface and fail fast when resume_session_id is supplied.
  • If dispatcher-backed interception is intentionally unsupported for Kiro, narrow the runtime factory contract for this backend instead of wiring the dispatcher and then not using it.

Files Reviewed

  • .env.example
  • README.ko.md
  • README.md
  • src/ouroboros/config/__init__.py
  • src/ouroboros/config/loader.py
  • src/ouroboros/config/models.py
  • src/ouroboros/orchestrator/__init__.py
  • src/ouroboros/orchestrator/adapter.py
  • src/ouroboros/orchestrator/kiro_adapter.py
  • src/ouroboros/orchestrator/runtime_factory.py
  • src/ouroboros/providers/factory.py
  • src/ouroboros/providers/kiro_adapter.py
  • tests/unit/test_kiro_adapters.py

Reviewed by ouroboros-agent[bot] via fallback manual deep analysis

@shaun0927
Copy link
Copy Markdown
Collaborator

I like the direction here. A Kiro backend is consistent with where this project has been going: broader multi-runtime support, shared orchestration surfaces, and backend-specific adapters behind a common contract.

That said, I would not merge this PR in its current form.

The main issue is runtime-contract parity.

Right now the branch gets most of the way there on subprocess safety, config wiring, and LLM/runtime routing, but two gaps still matter at the maintainer bar for this repo:

  1. Dispatcher-backed runtime behavior is still not really preserved.
    The factory now passes skill_dispatcher, but the adapter still does not use it during task execution. So selecting OUROBOROS_RUNTIME=kiro still drops part of the non-Claude runtime behavior instead of being a true drop-in backend.

  2. Targeted resume still degrades silently.
    Accepting resume_session_id and then only passing a bare --resume is not the same thing as honoring the requested session. If Kiro cannot resume a specific session id, that needs to fail fast explicitly rather than silently resuming "something."

There is also a process concern:

  1. The branch is stale / conflicting with main.
    Even if the remaining contract issues were fixed, I would want this rebased before merge.

So my recommendation would be:

  • keep the multi-runtime direction,
  • rebase onto current main,
  • either implement full dispatcher + targeted-resume parity,
  • or narrow the backend surface explicitly and reject unsupported behaviors instead of silently degrading them.

In other words: directionally yes, current PR shape no. The backend is promising, but this repo has been moving toward stricter contracts, not backend-specific silent exceptions.

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.

3 participants