feat(herdr): add herdr reporter extension for lifecycle state and ses… - #1111
feat(herdr): add herdr reporter extension for lifecycle state and ses…#1111ZoltanOnody wants to merge 1 commit into
Conversation
…sion identity Add a kimchi-side herdr integration that reports lifecycle state (idle/working/blocked), blocked prompts, and session identity to herdr when running inside a herdr-managed pane. - src/extensions/herdr-reporter.ts: JSON-RPC socket client, single-writer async queue, lifecycle state machine, session reference reporting, and HERDR_ENV gating - src/cli.ts: register herdrReporterExtension in extensionFactories - src/extensions/herdr-reporter.test.ts: tests for env gating, state transitions, blocked refcounting, queue ordering, session reporting, socket error resilience, and beforeExit listener cleanup - docs/herdr.md: document running kimchi inside herdr and resuming with --session
Kimchi Code Review
Summary📊 Review Score: 88/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — A comprehensive new test file 📝 Found 1 issue(s). See inline comments for details. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
|
Documentation Changes Added
Actions
If neither actions are selected, on PR close/merge the docs branch in ReadMe will remain open. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 88/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 3/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — A comprehensive new test file src/extensions/herdr-reporter.test.ts covers environment parsing, reporter sequencing/queueing/draining, socket error handling, extension lifecycle, state-machine transitions, blocked refcounting, shutdown behavior, and a regression test for the module-level beforeExit listener registry. Tests mock node:net hermetically.
📝 Found 1 issue(s). See inline comments for details.
| publishState(true) | ||
| released = true | ||
| // De-register from the process-level backstop so repeated | ||
| // extension instances (e.g. test reloads) do not accumulate |
There was a problem hiding this comment.
The subscription created by pi.events.on("herdr:blocked", ...) is not captured or unsubscribed when the extension is released. In a long-running process or multi-session host that creates and releases several reporter instances, the shared event bus retains stale listeners that do nothing but check if (!rootSession) return, leaking memory and CPU cycles and potentially tripping the bus's own max-listeners warning.
💡 Suggestion: Store the unsubscribe function returned by pi.events.on("herdr:blocked", ...) and call it inside releaseReporter before removing the instance from beforeExitReleasers, e.g., const unsubscribeBlocked = pi.events.on("herdr:blocked", ...); ... unsubscribeBlocked().
|
Hey thanks for contribution. I didn't understand from PR, what is the purpose of this PR and did you see https://github.com/getkimchi/herdr-kimchi ? |
2 participants
…sion identity
Add a kimchi-side herdr integration that reports lifecycle state (idle/working/blocked), blocked prompts, and session identity to herdr when running inside a herdr-managed pane.
Linked issue
Closes #
What does this PR do?
Checklist
pnpm run test)pnpm run check)