Dev - #1010
Dev#1010
Conversation
Add `genie brain update` to pull latest from GitHub, rebuild, and run migrations. Add `genie brain version` to show local vs latest version. After every brain command, a cache-only (no network, sync) check prints an update hint when a newer version is available.
- checkForUpdates() now accepts optional cachePath param for testing - Tests use tmpdir() instead of writing to ~/.genie/ (EACCES in CI) - Removes module re-import hacks, tests function directly with temp path
feat(brain): add update, version commands and auto-check hint
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdded brain update/version-check CLI commands, persistent git-driven version cache, and tests; bumped multiple package/plugin version fields; expanded agent frontmatter to include Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant User as User/CLI
participant CLI as brain.ts
participant Git as Git
participant Bun as Bun
participant Cache as Version Cache (fs)
end
User->>CLI: "genie brain update"
CLI->>Git: verify .git exists
CLI->>CLI: read pre-pull local version
CLI->>Git: git pull origin main
CLI->>Bun: bun install
CLI->>Bun: bun run build
CLI->>CLI: optionally run migrations
CLI->>Cache: write refreshed version cache
CLI->>User: report result
User->>CLI: "genie brain version"
CLI->>CLI: resolve local version (import/pkg)
CLI->>Cache: checkForUpdates() (read/refresh cache)
alt updateAvailable
CLI->>User: show local + latest + update hint
else upToDate
CLI->>User: show local + "up to date"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 0d43aec6cf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const brain = await import(BRAIN_PKG); | ||
| if (brain.runAllMigrations) { | ||
| await brain.runAllMigrations(); |
There was a problem hiding this comment.
After updateBrain() imports @automagik/genie-brain once to read oldVersion, this later import() call reuses the cached module instance, so runAllMigrations() executes the pre-update code. When the pulled version introduces new migrations, genie brain update can report success while skipping required schema updates, leaving the installation in a partially upgraded state.
Useful? React with 👍 / 👎.
| // Compare: strip prefix digit for comparison (dev uses 1.x, main uses 0.x) | ||
| const localCore = version.replace(/^\d+\./, ''); | ||
| const latestCore = latestVersion.replace(/^\d+\./, ''); | ||
| const updateAvailable = latestCore > localCore; |
There was a problem hiding this comment.
The update check uses plain string comparison (latestCore > localCore), which is lexicographic rather than numeric/semver-aware. This misorders multi-digit segments (for example 260403.10 vs 260403.9) and can suppress valid update notifications, so users may be told they are up to date when a newer brain tag exists.
Useful? React with 👍 / 👎.
![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 the current code and only fix it if needed.
Inline comments:
In `@src/term-commands/brain.ts`:
- Around line 121-128: Wrap the sequence of execSync calls that update and
rebuild the brain (the git pull and the two bun commands that reference
BRAIN_DIR) in a try-catch block so failures are caught; on error, log a concise,
user-friendly message that includes the error details (error.message or the
caught error) and return/exit with a non-zero status instead of letting the raw
stack trace bubble up. Ensure the try covers the execSync(`git -C "${BRAIN_DIR}"
pull origin main`, ...) and both execSync('bun install', { cwd: BRAIN_DIR, ...
}) and execSync('bun run build', { cwd: BRAIN_DIR, ... }) so any of those
failures are handled consistently.
- Around line 76-79: The current comparison using latestCore > localCore
performs lexicographic string comparison and fails for multi-digit segments
(e.g., "9" vs "10"); replace that with a numeric segment-wise comparison: add a
helper function (e.g., compareVersions) that splits a version string by '.',
converts segments to numbers, compares each segment returning
positive/negative/zero, then set updateAvailable = compareVersions(latestCore,
localCore) > 0; update references to localCore, latestCore, and updateAvailable
accordingly.
🪄 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: dd8c5c2d-75dc-4804-88cc-e2eb4f8086b1
📒 Files selected for processing (6)
.claude-plugin/marketplace.jsonpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/package.jsonsrc/term-commands/brain.test.tssrc/term-commands/brain.ts
| console.log(' Updating brain from GitHub...'); | ||
|
|
||
| // git pull origin main | ||
| execSync(`git -C "${BRAIN_DIR}" pull origin main`, { stdio: 'inherit' }); | ||
|
|
||
| // Rebuild | ||
| execSync('bun install', { cwd: BRAIN_DIR, stdio: 'inherit' }); | ||
| execSync('bun run build', { cwd: BRAIN_DIR, stdio: 'inherit' }); |
There was a problem hiding this comment.
Missing error handling for git/bun commands.
If git pull or bun install/build fails, execSync throws and the user sees a raw stack trace. Consider wrapping in try-catch with a user-friendly message.
Suggested pattern
console.log(' Updating brain from GitHub...');
- // git pull origin main
- execSync(`git -C "${BRAIN_DIR}" pull origin main`, { stdio: 'inherit' });
-
- // Rebuild
- execSync('bun install', { cwd: BRAIN_DIR, stdio: 'inherit' });
- execSync('bun run build', { cwd: BRAIN_DIR, stdio: 'inherit' });
+ try {
+ execSync(`git -C "${BRAIN_DIR}" pull origin main`, { stdio: 'inherit' });
+ execSync('bun install', { cwd: BRAIN_DIR, stdio: 'inherit' });
+ execSync('bun run build', { cwd: BRAIN_DIR, stdio: 'inherit' });
+ } catch {
+ console.error(' Update failed. Check network and try again.');
+ return false;
+ }📝 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.
| console.log(' Updating brain from GitHub...'); | |
| // git pull origin main | |
| execSync(`git -C "${BRAIN_DIR}" pull origin main`, { stdio: 'inherit' }); | |
| // Rebuild | |
| execSync('bun install', { cwd: BRAIN_DIR, stdio: 'inherit' }); | |
| execSync('bun run build', { cwd: BRAIN_DIR, stdio: 'inherit' }); | |
| console.log(' Updating brain from GitHub...'); | |
| try { | |
| execSync(`git -C "${BRAIN_DIR}" pull origin main`, { stdio: 'inherit' }); | |
| execSync('bun install', { cwd: BRAIN_DIR, stdio: 'inherit' }); | |
| execSync('bun run build', { cwd: BRAIN_DIR, stdio: 'inherit' }); | |
| } catch { | |
| console.error(' Update failed. Check network and try again.'); | |
| return false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/term-commands/brain.ts` around lines 121 - 128, Wrap the sequence of
execSync calls that update and rebuild the brain (the git pull and the two bun
commands that reference BRAIN_DIR) in a try-catch block so failures are caught;
on error, log a concise, user-friendly message that includes the error details
(error.message or the caught error) and return/exit with a non-zero status
instead of letting the raw stack trace bubble up. Ensure the try covers the
execSync(`git -C "${BRAIN_DIR}" pull origin main`, ...) and both execSync('bun
install', { cwd: BRAIN_DIR, ... }) and execSync('bun run build', { cwd:
BRAIN_DIR, ... }) so any of those failures are handled consistently.
…gents The council orchestrator and all 10 council members had tools: ["Read", "Glob", "Grep"] — no Bash access. This meant the orchestrator could not run `genie spawn` to place members in the correct tmux session, falling back to CC's native Agent tool which creates a separate tmux session. Members also couldn't run `genie chat send/read` despite their <deliberation> sections instructing them to do so. Changes: - Council orchestrator: add Bash tool, add explicit <spawning> section with `genie spawn` instructions, add constraint to never use Agent tool - All 10 council members: add Bash tool, remove permissionMode: plan (members need to execute genie chat commands autonomously) This ensures all council agents route through genie's tmux topology management (resolveSpawnTeamWindow → ensureTeamWindow) instead of bypassing it via CC's Agent tool.
fix(council): route spawning through genie tmux topology
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/genie/agents/council--deployer/AGENTS.md`:
- Line 8: The tools list for this agent includes "Bash" but no permissionMode is
declared, making execution potentially too permissive; update the agent's
manifest that contains the tools entry (the block with tools: ["Read", "Glob",
"Grep", "Bash"]) to add an explicit non‑permissive permissionMode (e.g.,
permissionMode: "explicit") and enumerate only the allowed tools/permissions for
this agent; ensure the plugin manifest under the same plugins/** bundle is
updated consistently with the Claude Code plugin guidelines so Bash use is
restricted to the declared explicit permissions.
In `@plugins/genie/agents/council--ergonomist/AGENTS.md`:
- Line 8: The manifest entry that now includes "Bash" in the tools array (tools:
["Read", "Glob", "Grep", "Bash"]) must explicitly retain a permissionMode to
avoid widening execution privileges; update the AGENTS.md/plugin manifest so
that alongside the tools array you add permissionMode: plan (e.g., ensure the
same block containing tools also contains permissionMode: plan) and verify the
change follows the plugins/** Claude Code plugin manifest conventions for
consistency.
In `@plugins/genie/agents/council--operator/AGENTS.md`:
- Line 8: The manifest currently lists tools: ["Read","Glob","Grep","Bash"] with
Bash enabled but missing the permissionMode guard; update the AGENTS.md plugin
manifest entry that defines tools to explicitly add permissionMode: "plan" for
the Bash tool (i.e., ensure the Bash tool declaration includes permissionMode:
plan), and verify the corresponding plugin manifest under the plugins/** Claude
Code plugin is updated consistently so the spawn-time permissions are explicit
and not defaulting to permissive.
In `@plugins/genie/agents/council--questioner/AGENTS.md`:
- Line 8: The AGENTS.md entry enabling tools includes "Bash" but removed an
explicit permissionMode; restore permissionMode: plan alongside the tools
declaration to enforce approval-before-execution (update the same AGENTS.md
block where tools: ["Read", "Glob", "Grep", "Bash"] appears), and ensure the
corresponding plugin manifest under plugins/** (the Claude Code plugin entries)
is updated to match this permissionMode change so the runtime uses plan-mode for
Bash invocations.
In `@plugins/genie/agents/council--sentinel/AGENTS.md`:
- Line 8: The tools list was extended to include "Bash" but omitted an explicit
permissionMode, which can allow fallback to a permissive runtime like
bypassPermissions; update the agent manifest/AGENTS.md entry so that the tools
array (the "tools" key) includes a corresponding "permissionMode" field set to a
restrictive value (e.g., "restricted" or "allow-listed") for this agent,
explicitly declare permissionMode alongside the "Bash" tool entry, and ensure
the manifest follows the plugins/** guidelines for Claude Code plugins so that
bypassPermissions is not implicitly enabled.
In `@plugins/genie/agents/council--tracer/AGENTS.md`:
- Line 8: The tools list now includes "Bash" but the manifest frontmatter no
longer sets permissionMode; restore an explicit permissionMode entry in the
AGENTS.md frontmatter (e.g., permissionMode: "restricted" or the project's
standard value) to make the runtime permission policy explicit, and ensure the
tools array (tools: ["Read","Glob","Grep","Bash"]) remains unchanged; verify the
updated manifest follows the plugins/** Claude Code plugin guidelines for
permissionMode and tool exposure consistency.
In `@plugins/genie/agents/council/AGENTS.md`:
- Around line 20-37: The examples and guidance reference non-existent commands
(`genie spawn`, `genie broadcast`, `genie send`) and should be updated to the
real CLI verbs: use `genie agent spawn` to spawn council members, replace the
sentence "All spawning goes through `genie spawn`" with "All spawning goes
through `genie agent spawn`", use `genie agent send "<topic>" --broadcast --team
$GENIE_TEAM` for team broadcasts, and use `genie agent send "<instructions>"
--to council--<member> --team $GENIE_TEAM` for per-member messages; ensure
`--broadcast` is included where the doc previously said `genie broadcast`.
🪄 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: d2b18940-a641-4f82-84ca-b76b5c895565
📒 Files selected for processing (11)
plugins/genie/agents/council--architect/AGENTS.mdplugins/genie/agents/council--benchmarker/AGENTS.mdplugins/genie/agents/council--deployer/AGENTS.mdplugins/genie/agents/council--ergonomist/AGENTS.mdplugins/genie/agents/council--measurer/AGENTS.mdplugins/genie/agents/council--operator/AGENTS.mdplugins/genie/agents/council--questioner/AGENTS.mdplugins/genie/agents/council--sentinel/AGENTS.mdplugins/genie/agents/council--simplifier/AGENTS.mdplugins/genie/agents/council--tracer/AGENTS.mdplugins/genie/agents/council/AGENTS.md
| promptMode: append | ||
| tools: ["Read", "Glob", "Grep"] | ||
| permissionMode: plan | ||
| tools: ["Read", "Glob", "Grep", "Bash"] |
There was a problem hiding this comment.
Adding Bash requires explicit non-permissive permission mode.
Line 8 is fine only if permission behavior is pinned. Right now permissionMode is absent, which can make this agent run too permissively.
As per coding guidelines plugins/**: Claude Code plugin. Verify plugin manifest updates are consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/council--deployer/AGENTS.md` at line 8, The tools list
for this agent includes "Bash" but no permissionMode is declared, making
execution potentially too permissive; update the agent's manifest that contains
the tools entry (the block with tools: ["Read", "Glob", "Grep", "Bash"]) to add
an explicit non‑permissive permissionMode (e.g., permissionMode: "explicit") and
enumerate only the allowed tools/permissions for this agent; ensure the plugin
manifest under the same plugins/** bundle is updated consistently with the
Claude Code plugin guidelines so Bash use is restricted to the declared explicit
permissions.
| promptMode: append | ||
| tools: ["Read", "Glob", "Grep"] | ||
| permissionMode: plan | ||
| tools: ["Read", "Glob", "Grep", "Bash"] |
There was a problem hiding this comment.
Permission mode must stay explicit after adding Bash.
Line 8 adds Bash; without explicit permissionMode, this can unintentionally broaden execution permissions. Re-add permissionMode: plan.
As per coding guidelines plugins/**: Claude Code plugin. Verify plugin manifest updates are consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/council--ergonomist/AGENTS.md` at line 8, The manifest
entry that now includes "Bash" in the tools array (tools: ["Read", "Glob",
"Grep", "Bash"]) must explicitly retain a permissionMode to avoid widening
execution privileges; update the AGENTS.md/plugin manifest so that alongside the
tools array you add permissionMode: plan (e.g., ensure the same block containing
tools also contains permissionMode: plan) and verify the change follows the
plugins/** Claude Code plugin manifest conventions for consistency.
| promptMode: append | ||
| tools: ["Read", "Glob", "Grep"] | ||
| permissionMode: plan | ||
| tools: ["Read", "Glob", "Grep", "Bash"] |
There was a problem hiding this comment.
Bash enablement needs explicit permission guard.
Line 8 enables Bash, but permissionMode is missing; this can default to a permissive mode at spawn time. Keep permissionMode: plan explicit.
As per coding guidelines plugins/**: Claude Code plugin. Verify plugin manifest updates are consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/council--operator/AGENTS.md` at line 8, The manifest
currently lists tools: ["Read","Glob","Grep","Bash"] with Bash enabled but
missing the permissionMode guard; update the AGENTS.md plugin manifest entry
that defines tools to explicitly add permissionMode: "plan" for the Bash tool
(i.e., ensure the Bash tool declaration includes permissionMode: plan), and
verify the corresponding plugin manifest under the plugins/** Claude Code plugin
is updated consistently so the spawn-time permissions are explicit and not
defaulting to permissive.
| promptMode: append | ||
| tools: ["Read", "Glob", "Grep"] | ||
| permissionMode: plan | ||
| tools: ["Read", "Glob", "Grep", "Bash"] |
There was a problem hiding this comment.
Line 8 introduces risk without explicit permission pinning.
Bash is enabled, but permissionMode is no longer explicit. Please restore permissionMode: plan to prevent unintended permissive execution.
As per coding guidelines plugins/**: Claude Code plugin. Verify plugin manifest updates are consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/council--questioner/AGENTS.md` at line 8, The AGENTS.md
entry enabling tools includes "Bash" but removed an explicit permissionMode;
restore permissionMode: plan alongside the tools declaration to enforce
approval-before-execution (update the same AGENTS.md block where tools: ["Read",
"Glob", "Grep", "Bash"] appears), and ensure the corresponding plugin manifest
under plugins/** (the Claude Code plugin entries) is updated to match this
permissionMode change so the runtime uses plan-mode for Bash invocations.
| promptMode: append | ||
| tools: ["Read", "Glob", "Grep"] | ||
| permissionMode: plan | ||
| tools: ["Read", "Glob", "Grep", "Bash"] |
There was a problem hiding this comment.
Security regression: explicit permission mode is now missing.
Line 8 adds Bash, but without an explicit permissionMode this agent can fall back to a permissive runtime mode (bypassPermissions), which is risky for a security-focused agent.
Suggested fix
tools: ["Read", "Glob", "Grep", "Bash"]
+permissionMode: planAs per coding guidelines plugins/**: Claude Code plugin. Verify plugin manifest updates are consistent.
📝 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.
| tools: ["Read", "Glob", "Grep", "Bash"] | |
| tools: ["Read", "Glob", "Grep", "Bash"] | |
| permissionMode: plan |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/council--sentinel/AGENTS.md` at line 8, The tools list
was extended to include "Bash" but omitted an explicit permissionMode, which can
allow fallback to a permissive runtime like bypassPermissions; update the agent
manifest/AGENTS.md entry so that the tools array (the "tools" key) includes a
corresponding "permissionMode" field set to a restrictive value (e.g.,
"restricted" or "allow-listed") for this agent, explicitly declare
permissionMode alongside the "Bash" tool entry, and ensure the manifest follows
the plugins/** guidelines for Claude Code plugins so that bypassPermissions is
not implicitly enabled.
| promptMode: append | ||
| tools: ["Read", "Glob", "Grep"] | ||
| permissionMode: plan | ||
| tools: ["Read", "Glob", "Grep", "Bash"] |
There was a problem hiding this comment.
Expanded tool capability without explicit permission control.
Line 8 adds Bash, but frontmatter no longer pins permissionMode. That combination can materially weaken runtime safety. Keep permission mode explicit in this manifest.
As per coding guidelines plugins/**: Claude Code plugin. Verify plugin manifest updates are consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/council--tracer/AGENTS.md` at line 8, The tools list now
includes "Bash" but the manifest frontmatter no longer sets permissionMode;
restore an explicit permissionMode entry in the AGENTS.md frontmatter (e.g.,
permissionMode: "restricted" or the project's standard value) to make the
runtime permission policy explicit, and ensure the tools array (tools:
["Read","Glob","Grep","Bash"]) remains unchanged; verify the updated manifest
follows the plugins/** Claude Code plugin guidelines for permissionMode and tool
exposure consistency.
| Council members MUST be spawned via `genie spawn` — this routes through genie's tmux topology management and places members in the correct session/window. | ||
|
|
||
| **Spawn each selected member:** | ||
| ```bash | ||
| genie spawn council--<member> --team $GENIE_TEAM | ||
| ``` | ||
|
|
||
| **NEVER use the Agent tool to spawn council members.** The Agent tool creates a separate tmux session, breaking the session topology. All spawning goes through `genie spawn`. | ||
|
|
||
| **Post topic to team chat after spawning:** | ||
| ```bash | ||
| genie broadcast "COUNCIL TOPIC: <topic>" --team $GENIE_TEAM | ||
| ``` | ||
|
|
||
| **Send instructions to members:** | ||
| ```bash | ||
| genie send "<instructions>" --to council--<member> --team $GENIE_TEAM | ||
| ``` |
There was a problem hiding this comment.
Use actual CLI command signatures in orchestration examples.
Lines 20-37 document genie spawn, genie broadcast, and genie send, but the referenced command implementations expose genie agent spawn and genie agent send (with --broadcast). This will break copy/paste execution.
Proposed doc fix
-Council members MUST be spawned via `genie spawn` — this routes through genie's tmux topology management and places members in the correct session/window.
+Council members MUST be spawned via `genie agent spawn` — this routes through genie's tmux topology management and places members in the correct session/window.
```bash
-genie spawn council--<member> --team $GENIE_TEAM
+genie agent spawn council--<member> --team $GENIE_TEAM-NEVER use the Agent tool to spawn council members. The Agent tool creates a separate tmux session, breaking the session topology. All spawning goes through genie spawn.
+NEVER use the Agent tool to spawn council members. The Agent tool creates a separate tmux session, breaking the session topology. All spawning goes through genie agent spawn.
-genie broadcast "COUNCIL TOPIC: <topic>" --team $GENIE_TEAM
+genie agent send "COUNCIL TOPIC: <topic>" --broadcast --team $GENIE_TEAM-genie send "<instructions>" --to council--<member> --team $GENIE_TEAM
+genie agent send "<instructions>" --to council--<member> --team $GENIE_TEAM</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @plugins/genie/agents/council/AGENTS.md around lines 20 - 37, The examples
and guidance reference non-existent commands (genie spawn, genie broadcast,
genie send) and should be updated to the real CLI verbs: use genie agent spawn to spawn council members, replace the sentence "All spawning goes through
genie spawn" with "All spawning goes through genie agent spawn", use genie agent send "<topic>" --broadcast --team $GENIE_TEAM for team broadcasts, and
use genie agent send "<instructions>" --to council--<member> --team $GENIE_TEAM for per-member messages; ensure --broadcast is included where the
doc previously said genie broadcast.
</details>
<!-- fingerprinting:phantom:triton:hawk:82119aad-e16c-45a7-bfb1-3ebe794f31c2 -->
<!-- This is an auto-generated comment by CodeRabbit -->
The --session flag overrides the team config's tmuxSessionName, creating a separate tmux session instead of using the parent session resolved at team creation. This caused workers (reviewers, council members) to land in isolated sessions instead of the project session. - Remove --session <team> from council SKILL.md spawn commands - Remove --session <team> from council member config examples - Add "Spawn Session Rule" to genie-orchestration.md - Add --session constraint to team-lead AGENTS.md
…e-rule fix(spawn): remove --session <team> override that breaks tmux topology
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/genie/agents/team-lead/AGENTS.md`:
- Line 121: Add input validation in the spawn CLI handler so that when a team is
specified (either via the --team option or implicitly via the GENIE_TEAM env
var) the command rejects any supplied --session flag; locate the spawn entry
point in src/term-commands/agent/spawn.ts and before calling handleWorkerSpawn,
check for options.session combined with options.team or process.env.GENIE_TEAM
and return an error/exit with a helpful message; this prevents the
sessionOverride (set from options.session and used by
handleWorkerSpawn/sessionOverride) from taking precedence over the team's
configured session and enforces the rule documented in AGENTS.md.
In `@plugins/genie/rules/genie-orchestration.md`:
- Around line 22-34: The spawn rule is documented but not enforced: update the
validation in handleWorkerSpawn (in src/term-commands/agents.ts) to detect when
both --team and --session are supplied and reject or warn; specifically, if team
is provided and session is also passed, throw or return an error/warning
instructing the caller to omit --session (or instead resolve the tmuxSessionName
from the team config and use that value), include a clear message like "When
--team is set, do not pass --session; session will be resolved from team config"
and ensure the handler returns a failure code so the prohibited combination
cannot proceed.
🪄 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: 4e0e8865-e008-4e0c-b2b6-b137024602d4
📒 Files selected for processing (4)
plugins/genie/agents/team-lead/AGENTS.mdplugins/genie/rules/genie-orchestration.mdskills/council/SKILL.mdskills/council/members/config.md
| - NEVER write code. `genie work` dispatches engineers who write code. | ||
| - NEVER push to main or master. | ||
| - NEVER use the Agent tool — use `genie work` to dispatch. | ||
| - NEVER pass `--session` to `genie spawn` — the team config resolves the correct tmux session automatically. Passing `--session <team>` creates a separate session, breaking topology. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check spawn command definition and validation logic
echo "=== Spawn command accepts both --team and --session ==="
rg -n -A 2 "\.option.*--session" src/term-commands/agent/spawn.ts
echo ""
echo "=== handleWorkerSpawn sets sessionOverride without validation ==="
rg -n -C 3 "sessionOverride.*options\.session" src/term-commands/agents.ts
echo ""
echo "=== Search for any validation that rejects both flags together ==="
rg -n "session.*team|team.*session" src/term-commands/agents.ts | rg -i "error|throw|reject"Repository: automagik-dev/genie
Length of output: 909
🏁 Script executed:
cat -n plugins/genie/agents/team-lead/AGENTS.md | sed -n '115,130p'Repository: automagik-dev/genie
Length of output: 764
🏁 Script executed:
#!/bin/bash
# Get context around handleWorkerSpawn to understand what it does with --team and --session
echo "=== handleWorkerSpawn implementation context ==="
rg -n -B 10 -A 15 "async function handleWorkerSpawn" src/term-commands/agents.ts | head -60Repository: automagik-dev/genie
Length of output: 1346
🏁 Script executed:
#!/bin/bash
# Check if there's validation that prevents --team and --session together
echo "=== Check for validation before handleWorkerSpawn call ==="
rg -n -B 15 "handleWorkerSpawn" src/term-commands/agents.ts | grep -A 15 "team\|session" | head -40Repository: automagik-dev/genie
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Find where sessionOverride is used
echo "=== Where is sessionOverride used? ==="
rg -n "sessionOverride" src/term-commands/agents.ts | head -20
echo ""
echo "=== Check the spawn function that uses these params ==="
rg -n "function.*spawn\|export.*spawn" src/genie-commands/ | grep -v "AGENTS.md"Repository: automagik-dev/genie
Length of output: 689
🏁 Script executed:
#!/bin/bash
# Check what doSpawn or spawn function does with sessionOverride
echo "=== Search for spawn function that accepts sessionOverride param ==="
rg -n "sessionOverride" src/genie-commands/ -A 2 -B 2Repository: automagik-dev/genie
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Examine the complete flow - find where options are parsed and passed to handleWorkerSpawn
echo "=== Command definition for spawn in agents.ts ==="
rg -n "\.command.*spawn\|\.subcommand.*spawn" src/term-commands/agents.ts -A 30 | head -60
echo ""
echo "=== Look for validation of team vs session flags ==="
rg -n "options\\.team\|options\\.session" src/term-commands/agents.ts | head -20Repository: automagik-dev/genie
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Check if there's any conditional logic that prevents both flags
echo "=== Check if options.team blocks options.session or vice versa ==="
rg -n "if.*team\|if.*session" src/term-commands/agents.ts -A 3 -B 1 | grep -A 3 -B 1 "team.*session\|session.*team"
echo ""
echo "=== View the actual spawn command definition ==="
rg -n "new Command\|spawn\(\)" src/term-commands/agents.ts | head -10Repository: automagik-dev/genie
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Find where the spawn command is registered
echo "=== Find spawn command registration ==="
fd -t f "agents.ts" src/term-commands/ -exec grep -l "spawn" {} \;
echo ""
echo "=== Look at the whole agents.ts file structure ==="
wc -l src/term-commands/agents.ts
echo ""
echo "=== Search for Command or command near spawn ==="
rg -n "Command|\.command|\.action" src/term-commands/agents.ts | grep -i spawnRepository: automagik-dev/genie
Length of output: 521
🏁 Script executed:
#!/bin/bash
# Let's find spawn.ts directly
echo "=== Find spawn.ts file ==="
fd -t f "spawn.ts" src/
echo ""
echo "=== Check spawn.ts for validation ==="
cat -n src/term-commands/agent/spawn.ts | head -80Repository: automagik-dev/genie
Length of output: 2345
🏁 Script executed:
#!/bin/bash
# Final verification: confirm that sessionOverride completely bypasses team resolution
echo "=== Confirm sessionOverride priority in spawn execution ==="
rg -n -B 5 -A 5 "let sessionName = sessionOverride" src/term-commands/agents.tsRepository: automagik-dev/genie
Length of output: 479
Add validation to reject --session flag when --team is provided.
The constraint documented in AGENTS.md (line 121) prohibits passing --session to genie spawn, warning that it breaks topology by overriding the team's configured session. However, handleWorkerSpawn in src/term-commands/agents.ts accepts both --team and --session without validation. At line 1280, sessionOverride is set directly from options.session, and at line 693 of src/term-commands/agents.ts, the override takes priority over the team's configured session name (lines 694–696). This allows users to violate the documented constraint and cause exactly the topology breakage it warns against.
Add validation in src/term-commands/agent/spawn.ts before calling handleWorkerSpawn to reject the combination of --team (or implicit team via GENIE_TEAM) and --session together.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/team-lead/AGENTS.md` at line 121, Add input validation
in the spawn CLI handler so that when a team is specified (either via the --team
option or implicitly via the GENIE_TEAM env var) the command rejects any
supplied --session flag; locate the spawn entry point in
src/term-commands/agent/spawn.ts and before calling handleWorkerSpawn, check for
options.session combined with options.team or process.env.GENIE_TEAM and return
an error/exit with a helpful message; this prevents the sessionOverride (set
from options.session and used by handleWorkerSpawn/sessionOverride) from taking
precedence over the team's configured session and enforces the rule documented
in AGENTS.md.
| ## Spawn Session Rule | ||
|
|
||
| NEVER pass `--session <team-name>` to `genie spawn`. The team config already stores the correct `tmuxSessionName` (resolved at team creation from the parent session). Passing `--session` overrides this and creates a separate tmux session, breaking the topology. | ||
|
|
||
| ```bash | ||
| # WRONG — creates separate session | ||
| genie spawn reviewer --team my-team --session my-team | ||
|
|
||
| # CORRECT — uses team's configured session | ||
| genie spawn reviewer --team my-team | ||
| ``` | ||
|
|
||
| The `--session` flag is for rare manual overrides only. When `--team` is set, let genie resolve the session from the team config. |
There was a problem hiding this comment.
Rule clearly documented, but enforcement missing.
The "Spawn Session Rule" provides clear rationale and examples. However, as noted in the review of plugins/genie/agents/team-lead/AGENTS.md:121, the implementation in src/term-commands/agents.ts doesn't enforce this constraint - both --team and --session can be passed together, allowing the prohibited pattern.
Consider adding validation in handleWorkerSpawn to reject the combination or emit a warning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/rules/genie-orchestration.md` around lines 22 - 34, The spawn
rule is documented but not enforced: update the validation in handleWorkerSpawn
(in src/term-commands/agents.ts) to detect when both --team and --session are
supplied and reject or warn; specifically, if team is provided and session is
also passed, throw or return an error/warning instructing the caller to omit
--session (or instead resolve the tmuxSessionName from the team config and use
that value), include a clear message like "When --team is set, do not pass
--session; session will be resolved from team config" and ensure the handler
returns a failure code so the prohibited combination cannot proceed.
Two bugs from PR review of #1010: 1. Version comparison used string comparison (`latestCore > localCore`) which fails on multi-digit segments: "260403.9" > "260403.10" is true lexicographically but wrong numerically. Added compareVersions() that splits on "." and compares segments as numbers. 2. Post-update migrations ran via `import()` which returns the cached pre-update module — new migrations from the pulled version would be silently skipped. Replaced with subprocess execution via `bun -e` which loads the rebuilt code from disk. Tests: 6 new tests for compareVersions covering multi-digit segments, different segment counts, and major version differences.
fix(brain): numeric version comparison + subprocess migrations
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/term-commands/brain.ts (1)
132-139:⚠️ Potential issue | 🟠 MajorMissing error handling for git/bun commands.
If
git pullorbun install/buildfails,execSyncthrows and the user sees a raw stack trace. This was flagged in a previous review and remains unaddressed.🛡️ Wrap commands in try-catch
console.log(' Updating brain from GitHub...'); + try { - // git pull origin main - execSync(`git -C "${BRAIN_DIR}" pull origin main`, { stdio: 'inherit' }); - - // Rebuild - execSync('bun install', { cwd: BRAIN_DIR, stdio: 'inherit' }); - execSync('bun run build', { cwd: BRAIN_DIR, stdio: 'inherit' }); + execSync(`git -C "${BRAIN_DIR}" pull origin main`, { stdio: 'inherit' }); + execSync('bun install', { cwd: BRAIN_DIR, stdio: 'inherit' }); + execSync('bun run build', { cwd: BRAIN_DIR, stdio: 'inherit' }); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + console.error(` Update failed: ${msg}`); + return false; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/term-commands/brain.ts` around lines 132 - 139, Wrap the three execSync calls that update and rebuild the brain (the git pull and the two bun commands referencing BRAIN_DIR) in a try-catch block so failures are caught; on error log a clear, contextual message including the caught error (use processLogger.error or console.error) and exit with a non-zero code (e.g., process.exit(1)) to avoid raw stack traces and stop further execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/term-commands/brain.test.ts`:
- Around line 17-41: The test suite creates tempDir in beforeAll and removes it
in afterAll which risks stale state between tests; change the fixture to create
the temp directory in beforeEach (set tempDir and cachePath there) and move the
rmSync cleanup into afterEach (remove tempDir recursively and force) so each
test runs with a fresh tmpdir; update the beforeEach that currently only removes
cachePath to either rely on the new per-test tmpdir or ensure it only
manipulates cachePath within the test-specific tempDir.
---
Duplicate comments:
In `@src/term-commands/brain.ts`:
- Around line 132-139: Wrap the three execSync calls that update and rebuild the
brain (the git pull and the two bun commands referencing BRAIN_DIR) in a
try-catch block so failures are caught; on error log a clear, contextual message
including the caught error (use processLogger.error or console.error) and exit
with a non-zero code (e.g., process.exit(1)) to avoid raw stack traces and stop
further execution.
🪄 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: a4430a87-5d11-4523-9cfe-5e4f0151adb0
📒 Files selected for processing (2)
src/term-commands/brain.test.tssrc/term-commands/brain.ts
| describe('checkForUpdates', () => { | ||
| let tempDir: string; | ||
| let cachePath: string; | ||
|
|
||
| beforeAll(() => { | ||
| tempDir = join(tmpdir(), `brain-test-${Date.now()}`); | ||
| mkdirSync(tempDir, { recursive: true }); | ||
| cachePath = join(tempDir, 'brain-version-check.json'); | ||
| }); | ||
|
|
||
| afterAll(() => { | ||
| try { | ||
| rmSync(tempDir, { recursive: true, force: true }); | ||
| } catch { | ||
| /* best effort */ | ||
| } | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| try { | ||
| rmSync(cachePath); | ||
| } catch { | ||
| /* ok */ | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using afterEach for tmpdir cleanup per project conventions.
Per coding guidelines, test fixtures should use tmpdir with cleanup in afterEach. Currently, the temp directory is cleaned in afterAll, which means if a test fails, subsequent tests in the same suite run with potentially stale state. The beforeEach only cleans the cache file, not the entire temp directory.
This is minor since the current approach works, but afterEach would provide better test isolation.
♻️ Suggested adjustment
describe('checkForUpdates', () => {
let tempDir: string;
let cachePath: string;
- beforeAll(() => {
+ beforeEach(() => {
tempDir = join(tmpdir(), `brain-test-${Date.now()}`);
mkdirSync(tempDir, { recursive: true });
cachePath = join(tempDir, 'brain-version-check.json');
});
- afterAll(() => {
+ afterEach(() => {
try {
rmSync(tempDir, { recursive: true, force: true });
} catch {
/* best effort */
}
});
-
- beforeEach(() => {
- try {
- rmSync(cachePath);
- } catch {
- /* ok */
- }
- });As per coding guidelines: "Use tmpdir with cleanup in afterEach for test fixtures"
📝 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.
| describe('checkForUpdates', () => { | |
| let tempDir: string; | |
| let cachePath: string; | |
| beforeAll(() => { | |
| tempDir = join(tmpdir(), `brain-test-${Date.now()}`); | |
| mkdirSync(tempDir, { recursive: true }); | |
| cachePath = join(tempDir, 'brain-version-check.json'); | |
| }); | |
| afterAll(() => { | |
| try { | |
| rmSync(tempDir, { recursive: true, force: true }); | |
| } catch { | |
| /* best effort */ | |
| } | |
| }); | |
| beforeEach(() => { | |
| try { | |
| rmSync(cachePath); | |
| } catch { | |
| /* ok */ | |
| } | |
| }); | |
| describe('checkForUpdates', () => { | |
| let tempDir: string; | |
| let cachePath: string; | |
| beforeEach(() => { | |
| tempDir = join(tmpdir(), `brain-test-${Date.now()}`); | |
| mkdirSync(tempDir, { recursive: true }); | |
| cachePath = join(tempDir, 'brain-version-check.json'); | |
| }); | |
| afterEach(() => { | |
| try { | |
| rmSync(tempDir, { recursive: true, force: true }); | |
| } catch { | |
| /* best effort */ | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/term-commands/brain.test.ts` around lines 17 - 41, The test suite creates
tempDir in beforeAll and removes it in afterAll which risks stale state between
tests; change the fixture to create the temp directory in beforeEach (set
tempDir and cachePath there) and move the rmSync cleanup into afterEach (remove
tempDir recursively and force) so each test runs with a fresh tmpdir; update the
beforeEach that currently only removes cachePath to either rely on the new
per-test tmpdir or ensure it only manipulates cachePath within the test-specific
tempDir.
4.260403.1 was already published to npm but no git tag existed, causing the version script to regenerate the same version. Created the missing tag and bumped to .2.
2 participants
Summary by CodeRabbit
New Features
genie brain versionandgenie brain update; update checks now notify when updates are availableTests
Documentation
--sessionwhen spawning/orchestratingChores