morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4618] feat(clients): register Kiro during setup #6335

New issue

Open

opened 2026-10-31 01:13:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:13:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4618
Author: @k1shan-k
Created: 10/16/2026
Status: 🔄 Open

Base: main ← Head: feat/kiro-client-setup

📝 Commits (2)

d63971a feat(clients): register Kiro during setup
9bc8ff4 test(clients): cover Kiro strict-syntax uninstall and clarify limits

📊 Changes

13 files changed (+781 additions, -56 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/references/connection-and-recovery.md (+2 -2)
📝 docs/client-configuration.md (+49 -0)
📝 docs/installation.md (+3 -0)
📝 src/application/ClientConfigurationDocument.ts (+107 -2)
📝 src/application/ClientConfigurationFiles.ts (+28 -4)
📝 src/application/ClientRegistrationIdentity.ts (+36 -0)
📝 src/application/SetupClientConfiguration.ts (+19 -13)
📝 src/application/SetupPlan.ts (+18 -1)
📝 src/application/SupportedClients.ts (+10 -0)
📝 src/application/Uninstall.ts (+23 -34)
➕ tests/acceptance/setup/kiroClientSetup.test.ts (+139 -0)
📝 tests/boundary/filesystem/clientLifecycle.test.ts (+1 -0)
➕ tests/boundary/filesystem/kiroClientConfiguration.test.ts (+346 -0)

📄 Description

Summary

Fixes #2385

rea setup had no Kiro option, so Kiro users had to add REA to their MCP
config by hand. This adds a kiro client: rea setup --client kiro registers
REA in ~/.kiro/settings/mcp.json and installs the skill into ~/.kiro/skills.
Kiro IDE and Kiro CLI share that file, so one client ID covers both.

Problem and expected behavior

Kiro users should get the same setup flow as everyone else: a plan, an
approval prompt, a registration Kiro actually loads, and a rerun that changes
nothing. doctor should report the registration and uninstall should remove
only REA's entry. Other servers and unrelated keys, like Kiro IDE's powers,
must be left alone.

Change and scope

A new kiro client definition and format. The entry is command, args,
env and disabled: false.
The file is parsed as strict JSON. Kiro CLI's default (V2) engine skips an
mcp.json that has comments, trailing commas or a BOM, so setup, doctor and
uninstall refuse to edit one and leave its bytes alone.
On refresh, setup keeps the user's disabledTools, timeout and env. It
keeps autoApprove only when the existing rea entry really is REA's.
Otherwise, a different server that happened to be called rea with
autoApprove: ["*"] would pass that approval to every REA tool.
V2 also skips the whole file when an entry has a badly typed timeout or
disabledTools. Setup and doctor check REA's entry for that and name the
field to fix.
The setup plan says which existing values are kept before you approve it.
Two small refactors to keep one owner for each rule. Kimi's enabledTools
special case now uses the same kept-members table (behavior unchanged).
Uninstall's "is this entry ours" check moved to ClientRegistrationIdentity,
so setup and uninstall share it.

Not included:

Doctor doesn't check other servers' entries for bad types; the docs point to
the kiro-cli mcp list warning for that.
Doctor doesn't look at a workspace .kiro/settings/mcp.json or a custom
agent's mcpServers, which override the user file. WorkBuddy and Cursor work
the same way today.
Setup doesn't write a timeout (see evidence), and it doesn't call
kiro-cli mcp add, which needs a logged-in session and rewrites the file.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

Semantic owner and earliest changed stage: SUPPORTED_CLIENT_DEFINITIONS in
src/application/SupportedClients.ts. The Kiro format, entry shape and
kept-member rules live in ClientConfigurationDocument.ts.
Contribution scope fit and linked maintainer decision: this adds a setup
client inside the existing setup boundaries; it's not a provider and it
doesn't touch targets. The discussion is on #2385; no maintainer decision has
been posted there yet.
CLI and MCP/tool-catalog contract: adds --client kiro. The catalog's
setup_clients picks it up from the definitions. The setup plan text now
lists kept values for Kimi and Kiro. No MCP tool changes.
Provider, bridge, target-format, or platform compatibility: none. Kiro's
paths are home-relative on every OS; only macOS was tested.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable):
after approval, setup writes ~/.kiro/settings/mcp.json (with a
.rea.backup) and ~/.kiro/skills/reverse-engineer-anything. autoApprove
is only carried over from REA's own entry. Uninstall removes only REA's entry
and keeps the mcpServers table, because Kiro skips a file without one.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs):
nothing generated is committed. The built catalog gains kiro, and the
skill's recovery reference now lists all 26 client IDs.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated:
- tests/boundary/filesystem/kiroClientConfiguration.test.ts (17 tests):
  paths and skill root; create; upsert with a backup; rerun with no changes;
  kept members from REA's own entry; a foreign autoApprove that is dropped;
  disabled: true re-enabled; comment, trailing-comma and BOM files refused
  by setup, doctor and uninstall; four badly typed timeout/disabledTools
  cases, named in the error, where uninstall still works; an empty file;
  uninstall keeping an empty mcpServers.
- tests/acceptance/setup/kiroClientSetup.test.ts: the public CLI going
  plan → approval required → apply → doctor → rerun → uninstall.
- clientLifecycle.test.ts: the detection order includes kiro.
- I removed each new rule (the autoApprove gate, the type checks, the
  doctor-side check) one at a time, and each removal made its tests fail.
Base reproduction or other evidence:
- On main, kiro isn't in SUPPORTED_CLIENT_DEFINITIONS, so
  --client kiro is rejected. This comes from reading the source; I didn't
  run it on the base commit.
- With a real kiro-cli 2.32.0 on macOS, kiro-cli mcp list and
  mcp status load what setup writes. @rea/analyze_javascript_application
  completed on V2 ("2 modules: index.js, util.js") and on V3, each through a
  workspace agent that allowed only that tool. Without that allowance, V2's
  non-interactive mode denied the call.
- V2 skipped the whole file for comments, trailing commas, a BOM, an empty
  file, a missing mcpServers, a string/decimal/null/negative timeout, or a
  non-array disabledTools. It still loaded a file with a malformed
  autoApprove, unknown fields or type: "stdio". V3 loaded a file with a
  comment.
- Using a small local stdio server: a 4 s timeout didn't stop a 10 s call,
  and with no timeout set neither engine cut off a 480 s call. That's why
  setup doesn't write one.

User-visible CLI/MCP output (if applicable): rea setup --client kiro --dry-run --json
on an existing REA 6.1.0 entry that has autoApprove. The action detail ends with:

Existing server environment overrides are retained. Existing disabledTools and
timeout values are retained. Existing autoApprove values are retained only from
REA's own registration.

After --yes, the entry is:

"rea": {
  "command": "<repo>/scripts/rea.mjs",
  "args": ["mcp"],
  "disabled": false,
  "autoApprove": ["list_documents"]
}

and doctor --client kiro reports it aligned.

Remaining proof gaps: Kiro IDE; Windows paths and the cmd.exe launcher;
skill discovery from ~/.kiro/skills in a live session, which comes from
Kiro's docs rather than a run.

Evidence-contract checklist: not applicable; this PR doesn't change Evidence.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Rebased onto main at 55cce6fc1. All on head 9bc8ff4eb, macOS, Node 24.14.0
(the repo pins 24.18.0):

npm run check: passed.
npm run check:fast: passed (also run by the pre-push hook).
npm run docs:check: passed.
npm run verify:package: passed (setup/uninstall CLI, 207 MCP tools, skill install).
npm pack --dry-run: passed (rea-agents 6.7.0, 1941 files).
npm run test:focused -- on all of src/application/*.test.ts,
tests/boundary/filesystem/*.test.ts, tests/acceptance/setup/*.test.ts and
tests/boundary/cli/cliSetup.test.ts (260 files after the rebase): 3539 passed, 30 skipped, 1 failed. The failure is
jsonOutputOwnership.test.ts ("preserves cancellation when staging cleanup
also fails"). It fails the same way on clean main at 55cce6fc1, and this
PR doesn't touch it.
Real kiro-cli 2.32.0, V2 and V3: as described above.
The earlier CI failure in tests/boundary/providers/kuna/kunaProvider.test.ts
was already broken on this PR's old base. I reproduced it there without the
Kiro changes. It passes on current main and on this branch after the rebase
(31/31).

Compatibility, safety, and release

Breaking changes or migration steps: none. Kimi behaves the same, and the
invalid-env error text is unchanged.
Real Hopper/Ghidra, browser, or OS coverage: not needed for this change.
Kiro was tested on macOS only.
Package or release metadata impact: kiro needs a release after 6.7.0 (the latest on npm);
docs/installation.md says so.
Security, privacy, process, or containment review: autoApprove is never
carried over from an entry REA doesn't own. Writes happen only after
approval, with a backup and 0600 permissions. Setup never runs kiro-cli.

Author readiness

Human owner: @k1shan-k

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4618 **Author:** [@k1shan-k](https://github.com/k1shan-k) **Created:** 10/16/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/kiro-client-setup` --- ### 📝 Commits (2) - [`d63971a`](https://github.com/morluto/rea/commit/d63971a12d41ce33d1a91e825f0fb908b31cce93) feat(clients): register Kiro during setup - [`9bc8ff4`](https://github.com/morluto/rea/commit/9bc8ff4eb3e93ff006dfc3c520b9646be7a23bf6) test(clients): cover Kiro strict-syntax uninstall and clarify limits ### 📊 Changes **13 files changed** (+781 additions, -56 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/references/connection-and-recovery.md` (+2 -2) 📝 `docs/client-configuration.md` (+49 -0) 📝 `docs/installation.md` (+3 -0) 📝 `src/application/ClientConfigurationDocument.ts` (+107 -2) 📝 `src/application/ClientConfigurationFiles.ts` (+28 -4) 📝 `src/application/ClientRegistrationIdentity.ts` (+36 -0) 📝 `src/application/SetupClientConfiguration.ts` (+19 -13) 📝 `src/application/SetupPlan.ts` (+18 -1) 📝 `src/application/SupportedClients.ts` (+10 -0) 📝 `src/application/Uninstall.ts` (+23 -34) ➕ `tests/acceptance/setup/kiroClientSetup.test.ts` (+139 -0) 📝 `tests/boundary/filesystem/clientLifecycle.test.ts` (+1 -0) ➕ `tests/boundary/filesystem/kiroClientConfiguration.test.ts` (+346 -0) </details> ### 📄 Description  ## Summary Fixes #2385 `rea setup` had no Kiro option, so Kiro users had to add REA to their MCP config by hand. This adds a `kiro` client: `rea setup --client kiro` registers REA in `~/.kiro/settings/mcp.json` and installs the skill into `~/.kiro/skills`. Kiro IDE and Kiro CLI share that file, so one client ID covers both. ## Problem and expected behavior Kiro users should get the same setup flow as everyone else: a plan, an approval prompt, a registration Kiro actually loads, and a rerun that changes nothing. `doctor` should report the registration and `uninstall` should remove only REA's entry. Other servers and unrelated keys, like Kiro IDE's `powers`, must be left alone. ## Change and scope - A new `kiro` client definition and format. The entry is `command`, `args`, `env` and `disabled: false`. - The file is parsed as strict JSON. Kiro CLI's default (V2) engine skips an `mcp.json` that has comments, trailing commas or a BOM, so setup, doctor and uninstall refuse to edit one and leave its bytes alone. - On refresh, setup keeps the user's `disabledTools`, `timeout` and `env`. It keeps `autoApprove` only when the existing `rea` entry really is REA's. Otherwise, a different server that happened to be called `rea` with `autoApprove: ["*"]` would pass that approval to every REA tool. - V2 also skips the whole file when an entry has a badly typed `timeout` or `disabledTools`. Setup and doctor check REA's entry for that and name the field to fix. - The setup plan says which existing values are kept before you approve it. - Two small refactors to keep one owner for each rule. Kimi's `enabledTools` special case now uses the same kept-members table (behavior unchanged). Uninstall's "is this entry ours" check moved to `ClientRegistrationIdentity`, so setup and uninstall share it. Not included: - Doctor doesn't check other servers' entries for bad types; the docs point to the `kiro-cli mcp list` warning for that. - Doctor doesn't look at a workspace `.kiro/settings/mcp.json` or a custom agent's `mcpServers`, which override the user file. WorkBuddy and Cursor work the same way today. - Setup doesn't write a `timeout` (see evidence), and it doesn't call `kiro-cli mcp add`, which needs a logged-in session and rewrites the file. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: `SUPPORTED_CLIENT_DEFINITIONS` in `src/application/SupportedClients.ts`. The Kiro format, entry shape and kept-member rules live in `ClientConfigurationDocument.ts`. - Contribution scope fit and linked maintainer decision: this adds a setup client inside the existing setup boundaries; it's not a provider and it doesn't touch targets. The discussion is on #2385; no maintainer decision has been posted there yet. - CLI and MCP/tool-catalog contract: adds `--client kiro`. The catalog's `setup_clients` picks it up from the definitions. The setup plan text now lists kept values for Kimi and Kiro. No MCP tool changes. - Provider, bridge, target-format, or platform compatibility: none. Kiro's paths are home-relative on every OS; only macOS was tested. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): after approval, setup writes `~/.kiro/settings/mcp.json` (with a `.rea.backup`) and `~/.kiro/skills/reverse-engineer-anything`. `autoApprove` is only carried over from REA's own entry. Uninstall removes only REA's entry and keeps the `mcpServers` table, because Kiro skips a file without one. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): nothing generated is committed. The built catalog gains `kiro`, and the skill's recovery reference now lists all 26 client IDs. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/filesystem/kiroClientConfiguration.test.ts` (17 tests): paths and skill root; create; upsert with a backup; rerun with no changes; kept members from REA's own entry; a foreign `autoApprove` that is dropped; `disabled: true` re-enabled; comment, trailing-comma and BOM files refused by setup, doctor and uninstall; four badly typed `timeout`/`disabledTools` cases, named in the error, where uninstall still works; an empty file; uninstall keeping an empty `mcpServers`. - `tests/acceptance/setup/kiroClientSetup.test.ts`: the public CLI going plan → approval required → apply → doctor → rerun → uninstall. - `clientLifecycle.test.ts`: the detection order includes `kiro`. - I removed each new rule (the `autoApprove` gate, the type checks, the doctor-side check) one at a time, and each removal made its tests fail. - Base reproduction or other evidence: - On `main`, `kiro` isn't in `SUPPORTED_CLIENT_DEFINITIONS`, so `--client kiro` is rejected. This comes from reading the source; I didn't run it on the base commit. - With a real kiro-cli 2.32.0 on macOS, `kiro-cli mcp list` and `mcp status` load what setup writes. `@rea/analyze_javascript_application` completed on V2 ("2 modules: index.js, util.js") and on V3, each through a workspace agent that allowed only that tool. Without that allowance, V2's non-interactive mode denied the call. - V2 skipped the whole file for comments, trailing commas, a BOM, an empty file, a missing `mcpServers`, a string/decimal/null/negative `timeout`, or a non-array `disabledTools`. It still loaded a file with a malformed `autoApprove`, unknown fields or `type: "stdio"`. V3 loaded a file with a comment. - Using a small local stdio server: a 4 s `timeout` didn't stop a 10 s call, and with no `timeout` set neither engine cut off a 480 s call. That's why setup doesn't write one. - User-visible CLI/MCP output (if applicable): `rea setup --client kiro --dry-run --json` on an existing REA 6.1.0 entry that has `autoApprove`. The action detail ends with: ```text Existing server environment overrides are retained. Existing disabledTools and timeout values are retained. Existing autoApprove values are retained only from REA's own registration. ``` After `--yes`, the entry is: ```json "rea": { "command": "<repo>/scripts/rea.mjs", "args": ["mcp"], "disabled": false, "autoApprove": ["list_documents"] } ``` and `doctor --client kiro` reports it `aligned`. - Remaining proof gaps: Kiro IDE; Windows paths and the `cmd.exe` launcher; skill discovery from `~/.kiro/skills` in a live session, which comes from Kiro's docs rather than a run. Evidence-contract checklist: not applicable; this PR doesn't change Evidence. For evidence-bearing changes: - [ ] Observed, derived, and inferred claims remain distinguishable. - [ ] Artifact identity, source provenance, and failed attempts remain preserved. - [ ] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Rebased onto main at `55cce6fc1`. All on head `9bc8ff4eb`, macOS, Node 24.14.0 (the repo pins 24.18.0): - `npm run check`: passed. - `npm run check:fast`: passed (also run by the pre-push hook). - `npm run docs:check`: passed. - `npm run verify:package`: passed (setup/uninstall CLI, 207 MCP tools, skill install). - `npm pack --dry-run`: passed (rea-agents 6.7.0, 1941 files). - `npm run test:focused --` on all of `src/application/*.test.ts`, `tests/boundary/filesystem/*.test.ts`, `tests/acceptance/setup/*.test.ts` and `tests/boundary/cli/cliSetup.test.ts` (260 files after the rebase): 3539 passed, 30 skipped, 1 failed. The failure is `jsonOutputOwnership.test.ts` ("preserves cancellation when staging cleanup also fails"). It fails the same way on clean `main` at `55cce6fc1`, and this PR doesn't touch it. - Real kiro-cli 2.32.0, V2 and V3: as described above. - The earlier CI failure in `tests/boundary/providers/kuna/kunaProvider.test.ts` was already broken on this PR's old base. I reproduced it there without the Kiro changes. It passes on current `main` and on this branch after the rebase (31/31). ## Compatibility, safety, and release - Breaking changes or migration steps: none. Kimi behaves the same, and the invalid-env error text is unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not needed for this change. Kiro was tested on macOS only. - Package or release metadata impact: `kiro` needs a release after 6.7.0 (the latest on npm); `docs/installation.md` says so. - Security, privacy, process, or containment review: `autoApprove` is never carried over from an entry REA doesn't own. Writes happen only after approval, with a backup and 0600 permissions. Setup never runs `kiro-cli`. ## Author readiness  Human owner: @k1shan-k - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge  - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:13:39 +00:00

bsakaguchi referenced this issue

2026-11-01 21:47:21 +00:00

[PR #6496] [MERGED] docs(ci): add a repeatable static analysis example #7369