Repository navigation
refactor(adapters): split kiro.ts into cohesive leaf modules - #4493
Conversation
Pure-move split of src/storage/cleanup.ts (3141 lines) into src/storage/cleanup/ leaves: types, paths, db, satellite, pending, reconcile, staging, preview, execute, restore. The original file remains a facade re-exporting every public name. No behavior change; no consumer edits.
Pure-move split of src/service.ts (5558 lines) into src/service/ leaves: state (paths/install-state), guards (ownership/auth/manager-command), health (port resolution + serving checks), launchd, systemd, windows-scheduler (schtasks + elevation finalize), windows-taskxml (task XML build/verify), windows-ops (Windows install/stop/uninstall + stage tracking), repair, orchestration (install/uninstall/stop dispatch), diagnostics, cli (arg parsing/dispatch). src/service.ts is now a facade re-exporting every original export. One semantic-preserving adjustment: import.meta.dir at the old file depth is carried by serviceSourceDir in state.ts.
github-actions
Bot
added
the
chore
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
the
intake: hygiene-blocked
|
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
|
This PR is a pure-move refactor with zero behavior change: the original file stays as a facade re-exporting every name it exported before, so no consumer and no test is edited. The hygiene gate counts any non-comment change under Adding a new focused test here would be the wrong artifact. The oracle for a pure move is the existing suite for the moved subsystem, unchanged, which is exactly what proves the relocation preserved behavior. Those suites run on the stack tip (#4493). Fidelity was verified statically instead, and the checks found real defects rather than rubber-stamping: every baseline top-level declaration resolves to exactly one leaf with a comment-normalized identical body, the facade's export set matches the baseline exactly, every module-level mutable binding and test hook has a single declaration site, and every relative import specifier resolves. Local product suite, typecheck, and build were NOT RUN by operator instruction. |
lidge-jun
added
the
test-exception-approved
리뷰 · 우선순위 62 / 80설명 모듈 수준 mutable 상태가 없어서 싱글톤 포크 위험이 cleanup/service보다 낮습니다. Map/Set은 함수 안 지역 값이라 함수와 함께 이동합니다. leaf 그래프도 한 방향(wire/reasoning → usage/conversation → payload/stream → adapter)입니다. 이 브랜치는 #4492( 우선순위 62는 tip으로서 CI 증명 역할은 크지만, 내용 자체는 세 장 중 가장 기계적이고 사용자 버그 수정이 아니기 때문입니다. 다만 Kiro 어댑터는 트래픽이 있는 경로라, facade 재수출이 빠지거나 stream leaf import가 깨지면 로그인 다음 단계에서 바로 터집니다. package 2.53.0, types/config 분할과 무관합니다. 참고로 같은 시각의 Kiro 계정 붕괴 수정 #4482는 oauth 쪽(
심볼 스택 tip CI - 세 분할이 한 트리에서 같이 검증되는지 여기가 게이트. 심볼 hygiene 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
github-actions
Bot
removed
the
intake: hygiene-blocked
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56bc78ada5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| }; | ||
| } | ||
| // Thin facade over the cohesive leaf modules under ./kiro/. Every name this file exported |
There was a problem hiding this comment.
This split moves the Kiro implementation into src/adapters/kiro/, but structure/runtime.md:168 still describes src/adapters/kiro.ts and the pre-existing kiro-*.ts helpers as the implementation owners, leaving the maintained source map inaccurate. Update that entry to include the facade and new leaf directory and describe their responsibilities; adapter changes are required to update their mapped structure documentation in the same change.
AGENTS.md reference: src/AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…eaves [skip ci] Hosted CI on the stack tip went red because source-contract tests still read the 30-line facade at src/service.ts. Point each assertion at the owning leaf so the existing oracles keep their original strings, slices, and negative matches. Import-specifier depth is the only assertion text that changed, because the leaves sit one directory lower.
56bc78a to
60b21c0
Compare
github-actions
Bot
added
intake: hygiene-blocked
lidge-jun
added
the
test-exception-approved
github-actions
Bot
removed
the
intake: hygiene-blocked
… [skip ci] Hosted CI shard 2/4 still read src/service.ts from launchd-repair.test.ts. Point each slice at the owning leaf and keep the original assertion strings.
Pure-move split of the 2319-line kiro.ts god file into src/adapters/kiro/ leaves: wire.ts (wire identity/constants + message shapes), usage.ts (token estimation + message-text walkers), reasoning.ts (reasoning mode + thinking-tag injection), conversation.ts (capability + conversation-state validation), payload.ts (buildKiroPayload assembly), stream.ts (eventstream parsing + bounded fallback), adapter.ts (createKiroAdapter). kiro.ts is now a thin facade re-exporting the same public surface. Zero behavior change; no consumer edits.
The Kiro implementation now lives under src/adapters/kiro/, with src/adapters/kiro.ts as the facade. Keep the structure map aligned so structure:check still names a path inside the owned area.
60b21c0 to
3176ac8
Compare
github-actions
Bot
added
intake: hygiene-blocked
lidge-jun
added
the
test-exception-approved
github-actions
Bot
removed
the
intake: hygiene-blocked
dev landed #4425 on the old src/service.ts after this stack branched. Carry the LogonTrigger UserId and the matching source-contract tests into src/service/windows-taskxml.ts so a non-elevated install still registers without asking for UAC.
Keep the service facade. The Windows logon-trigger fix from #4425 is already in src/service/windows-taskxml.ts.
…un#4493) Merge the god-file round 1 stack: storage/cleanup, service, and kiro facades.
1 participant
Summary
src/adapters/kiro.tswas 2319 lines mixing wire identity, token estimation, reasoning and thinking control, conversation-state validation, payload assembly, and the streaming event parser. This splits it into seven leaves undersrc/adapters/kiro/and leaves the original path as an 8-line facade re-exporting all seven previously exported names, so no consumer and no test is edited.The file has no module-level mutable state, so there is nothing to fork: every
MapandSetin it is function-local and moved with its enclosing function. The leaf graph is acyclic —wireandreasoningare sinks, feedingusageandconversation, thenpayloadandstream, thenadapter.stream.tsis 1153 lines, above the size the rest of the split targets.parseKiroAttemptEventsalone is roughly 730 lines of one stateful eventstream loop sharing about twenty closure locals; splitting it would mean parameterizing that state, which is a rewrite rather than a move, so it stays whole.Third and final PR of the stack, on top of the
service.tssplit. This is the stack tip, so CI runs here and its tree contains all three splits.Verification
parseKiroStreamgenerator, 7 re-exported by the facade, none missing and none added.src/lab/import, no relative specifier left unresolvable.Checklist