Dev - #2521
Dev#2521
Conversation
src/lib/genie-config.ts computed ~/.genie once from homedir() at import time, ignoring GENIE_HOME — the one env var documented to relocate ALL global state. Any host with a real ~/.genie/config.json leaked that config into 'isolated' tests and subprocesses (e.g. omni approvals enabled made the default-config dispatcher tests fail on such hosts). Config paths now resolve through workspace.ts genieHome() per call, and the two dispatcher tests that depended on default config actually isolate it: omni-dispatch boot-seam sets GENIE_HOME to a tmpdir, and the fail-closed regression gate spawns dist/genie.js with an isolated GENIE_HOME so 'default path' means default config on every host. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BEJCsZTjxLyrM8BQKjGEVs
fix(config): resolve genie config dir via GENIE_HOME, lazily
Closes the three findings confirmed by the independent reviews on #2516: - Reaction frames (and blank bodies) in a route-mapped chat no longer spawn a claude run, publish a reply, or mutate the reacted-to message's status ack — they are stored to the inbox only. The approval-chat reaction path is untouched, including when the route chat IS the approval chat (new coexistence test). - buildClaudeArgs inserts '--' before the message positional so a hyphen-leading message is always a prompt, never parsed as a flag (verified live against the claude CLI). - Non-zero exits now surface a bounded tail of the child's stderr (fallback stdout) in the error notice instead of a bare exit code; SpawnClaudeResult carries stderr through the existing drain pattern. 52 tests (45 pre-existing + 7 new), typecheck and biome clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sg8vJv9r2yqmnbtVPqM2vG
…uard fix(omni-runner): reaction guard, -- terminator, stderr on failure
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughVersion metadata was bumped across Genie manifests. ChangesGenie config and versioning
omni-runner behavior
Hermes Genie install and status docs
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request isolates global state in tests using GENIE_HOME, updates config resolution to honor GENIE_HOME lazily, and improves the omni-runner by capturing stderr on non-zero exits, terminating CLI options with -- to support hyphen-leading prompts, and guarding against runs triggered by reactions or empty messages. Feedback is provided regarding a bug in the tailOf function where a max value of 1 results in slice(-0), which returns the entire string instead of truncating it.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function tailOf(text: string, max: number): string { | ||
| if (max <= 0) return ''; | ||
| if (text.length <= max) return text; | ||
| return `…${text.slice(-(max - 1))}`; | ||
| } |
There was a problem hiding this comment.
In JavaScript/TypeScript, calling string.slice(-0) is equivalent to string.slice(0), which returns the entire string instead of an empty string. If max is 1, -(max - 1) evaluates to -0. As a result, tailOf(text, 1) will return … followed by the entire text string, violating the length constraint and leaking the entire diagnostic.
To prevent this edge case, we can use text.length - (max - 1) as the start index for slice, which correctly evaluates to text.length when max is 1 and returns an empty string.
| function tailOf(text: string, max: number): string { | |
| if (max <= 0) return ''; | |
| if (text.length <= max) return text; | |
| return `…${text.slice(-(max - 1))}`; | |
| } | |
| function tailOf(text: string, max: number): string { | |
| if (max <= 0) return ''; | |
| if (text.length <= max) return text; | |
| return '…' + text.slice(text.length - (max - 1)); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/__tests__/dispatch-fail-closed-regression.test.ts`:
- Around line 44-45: The test setup for dispatch fail-closed regression is
sharing one ISOLATED_HOME across both cases, which can leak state between tests.
Update the suite to create a fresh GENIE_HOME per test using the same
beforeEach/afterEach isolation pattern used in the other hook tests, and ensure
each test tears down its own temporary home instead of relying on a single
afterAll cleanup.
In `@src/lib/genie-config.ts`:
- Around line 19-21: `getGenieConfigPath()` is duplicating path resolution by
calling `genieHome()` directly instead of reusing the existing `getGenieDir()`
helper. Update `getGenieConfigPath()` to compose the config path from
`getGenieDir()` so the path logic stays centralized and consistent, and keep the
change localized to the `getGenieConfigPath`/`getGenieDir` functions in this
module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8162ae1d-b9b9-4e28-82de-fcad1c5b56b2
📒 Files selected for processing (10)
.claude-plugin/marketplace.jsonpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/package.jsonsrc/hooks/__tests__/dispatch-fail-closed-regression.test.tssrc/hooks/__tests__/omni-dispatch.test.tssrc/lib/genie-config.test.tssrc/lib/genie-config.tssrc/lib/omni-runner.test.tssrc/lib/omni-runner.ts
| const ISOLATED_HOME = mkdtempSync(join(tmpdir(), 'genie-dispatch-regression-')); | ||
| afterAll(() => rmSync(ISOLATED_HOME, { recursive: true, force: true })); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A5 "driveDispatch\(" src/hooks/__tests__/dispatch-fail-closed-regression.test.tsRepository: automagik-dev/genie
Length of output: 881
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dispatch-fail-closed-regression.test.ts =="
cat -n src/hooks/__tests__/dispatch-fail-closed-regression.test.ts | sed -n '1,220p'
echo
echo "== nearby tests using isolated home =="
rg -n -A4 -B4 "GENIE_HOME|mkdtempSync|beforeEach|afterEach|afterAll" src/hooks/__tests__/*.tsRepository: automagik-dev/genie
Length of output: 21916
Use a fresh GENIE_HOME per test. ISOLATED_HOME is shared across both cases and only cleaned up in afterAll, so any dispatch-side writes can leak state between tests. Match the beforeEach/afterEach isolation pattern used in the other hook tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/__tests__/dispatch-fail-closed-regression.test.ts` around lines 44
- 45, The test setup for dispatch fail-closed regression is sharing one
ISOLATED_HOME across both cases, which can leak state between tests. Update the
suite to create a fresh GENIE_HOME per test using the same beforeEach/afterEach
isolation pattern used in the other hook tests, and ensure each test tears down
its own temporary home instead of relying on a single afterAll cleanup.
Source: Coding guidelines
| export function getGenieConfigPath(): string { | ||
| return GENIE_CONFIG_FILE; | ||
| return join(genieHome(), 'config.json'); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Reuse getGenieDir() instead of re-deriving genieHome().
getGenieConfigPath() re-calls genieHome() directly rather than composing with getGenieDir(), which already exists for this purpose. Minor duplication of the path-resolution logic.
♻️ Proposed refactor
export function getGenieConfigPath(): string {
- return join(genieHome(), 'config.json');
+ return join(getGenieDir(), 'config.json');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getGenieConfigPath(): string { | |
| return GENIE_CONFIG_FILE; | |
| return join(genieHome(), 'config.json'); | |
| } | |
| export function getGenieConfigPath(): string { | |
| return join(getGenieDir(), 'config.json'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/genie-config.ts` around lines 19 - 21, `getGenieConfigPath()` is
duplicating path resolution by calling `genieHome()` directly instead of reusing
the existing `getGenieDir()` helper. Update `getGenieConfigPath()` to compose
the config path from `getGenieDir()` so the path logic stays centralized and
consistent, and keep the change localized to the
`getGenieConfigPath`/`getGenieDir` functions in this module.
Profile-based Hermes hosts (sticky $HERMES_HOME/active_profile) load plugins from $HERMES_HOME/profiles/<name>/plugins/, so a default-dir-only install is invisible there — the QA follow-up from the wish's live smoke. install-local.sh now function-izes the guarded install and repeats it into the active profile's plugins dir when one exists; --no-profile opts out. Verified: no-profile host, profile host (both dirs), dangling profile name, --no-profile, --copy over symlink, and the unrelated-content refusal path all behave. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sg8vJv9r2yqmnbtVPqM2vG
Status DONE, success/QA criteria ticked (all verified during execution), Review Results table filled with per-group verdicts, promotion reviews, and shipped versions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sg8vJv9r2yqmnbtVPqM2vG
…e-hosts fix(hermes-plugin): install into the active Hermes profile's plugins dir
…okkeeping chore(wish): close out hermes-khaw-native-surface bookkeeping
…ver races the pending one The routed-run path fired the pending and final status reactions as two independent fire-and-forget HTTP calls; a run finishing before the pending call landed could reorder the final ack behind it at the API, leaving a finished run permanently showing the pending glyph (route reactions have no reconciliation pass). emitReaction now returns its always-fulfilled settlement promise and runOneShot chains the final emit on it — errors stay swallowed, nothing user-visible is delayed, and the caller still never awaits the acks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sg8vJv9r2yqmnbtVPqM2vG
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/hermes-genie/scripts/install-local.sh`:
- Around line 79-83: The active_profile value used by install-local.sh is only
whitespace-stripped before being inserted into the profiles path, so add a
defensive validation step in the active_profile handling block before calling
install_into. Ensure the script only accepts safe profile names in the path
construction for hermes_home/profiles/$active_profile/plugins, and reject any
value containing traversal or separator characters so install_into cannot write
outside the intended profiles directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0e8adcaf-32c4-43b2-80bc-a34eca4c46ff
📒 Files selected for processing (7)
.claude-plugin/marketplace.json.genie/wishes/hermes-khaw-native-surface/WISH.mdpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/package.jsonplugins/hermes-genie/README.mdplugins/hermes-genie/scripts/install-local.sh
| if [ "$profile_install" = "1" ] && [ -f "$hermes_home/active_profile" ]; then | ||
| active_profile="$(head -n1 "$hermes_home/active_profile" | tr -d '[:space:]')" | ||
| if [ -n "$active_profile" ] && [ -d "$hermes_home/profiles/$active_profile" ]; then | ||
| install_into "$hermes_home/profiles/$active_profile/plugins" | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
Unsanitized active_profile value used in path construction.
active_profile is read directly from $hermes_home/active_profile and only whitespace-stripped before being interpolated into $hermes_home/profiles/$active_profile/plugins. If that file ever contains path-traversal sequences (e.g. ../../etc), install_into would write outside the intended profiles directory. Low risk today since the file is host-managed, but worth a defensive check given this same wish document notes a path-traversal exploit was found and fixed elsewhere in this codebase (Group 1 plugin-core).
🛡️ Optional guard
active_profile="$(head -n1 "$hermes_home/active_profile" | tr -d '[:space:]')"
- if [ -n "$active_profile" ] && [ -d "$hermes_home/profiles/$active_profile" ]; then
+ if [ -n "$active_profile" ] && [[ "$active_profile" != *..* ]] && [ -d "$hermes_home/profiles/$active_profile" ]; then
install_into "$hermes_home/profiles/$active_profile/plugins"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/hermes-genie/scripts/install-local.sh` around lines 79 - 83, The
active_profile value used by install-local.sh is only whitespace-stripped before
being inserted into the profiles path, so add a defensive validation step in the
active_profile handling block before calling install_into. Ensure the script
only accepts safe profile names in the path construction for
hermes_home/profiles/$active_profile/plugins, and reject any value containing
traversal or separator characters so install_into cannot write outside the
intended profiles directory.
fix(omni-runner): sequence route status acks — final reaction waits for the pending one
Promotion readiness — batch of 2026-07-05Everything queued on dev since the last promotion, each independently reviewed and CI-green before merge:
With these, every finding from the #2516 independent-review record is either fixed and merged or explicitly documented as theoretical. Merge is yours per §19. 🤖 Generated with Claude Code |
…ul template - version.yml: PR-event workflow_run completions (always job-skipped) shared the concurrency group with push-event bumps and cancelled them mid-flight (observed 2026-07-04T23:11:46Z — missed dev release). Group now event-scoped; cancel-in-progress off (bumps queue). - release.yml: comments/input description now state all channel manifests commit to MAIN (matches install.sh MANIFEST_BASE). - CLAUDE.md: CLI table 12 -> 14 (adds install, mcp). - orchestration-guard.ts: nudge strings now recommend live v5 primitives (task list/status, board --wish, SendMessage tool). - genie-soul.md: full v5 rewrite of dispatch/orchestration sections; dead-verb grep zero; dead 'genie init agent' removed -> /wizard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N1FLEV2Qse3jbX5Wz1sjWE
…udges fix: close version-race, purge dead v4 verbs from nudges + soul template
1 participant
Summary by CodeRabbit