Skip to content

chore: rolling promotion dev -> main - #557

Merged
namastex888 merged 16 commits into
mainfrom
dev
Mar 14, 2026
Merged

namastex888 merged 16 commits into
mainfrom
dev

Conversation

@namastex888

@namastex888 namastex888 commented Mar 14, 2026 •

Copy link
Copy Markdown
Contributor

Rolling Promotion PR

Auto-maintained rolling promotion PR from dev to main.

Process:

  • This PR is automatically created and kept open
  • Agent monitors CI status and fixes issues
  • Human reviews and merges when ready
  • Label ready-to-merge added when all checks pass

Human approval required for merge to production.

Summary by CodeRabbit

  • New Features
    • Update command: --next and --stable options; update flow now syncs plugin assets after CLI updates.
    • New state subcommand: reset to reset an in‑progress group back to ready.
  • Behavior / UX
    • Team commands now operate against a global teams store (configurable via GENIE_HOME).
    • New config option: updateChannel (latest|next).
  • Reliability
    • Improved concurrency via a shared file-lock mechanism for mailboxes, teams, state, and chat.
  • Chores
    • Bumped package/plugin versions to 3.260314.8.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request represents an automated rolling promotion from the dev branch to main. Its primary purpose is to propagate the latest stable version of the genie plugin and its associated components to the main codebase, ensuring that the production branch remains up-to-date with validated changes. This automated process streamlines continuous delivery, with human review and approval serving as the final gate before merging to production.

Highlights

  • Version Update: The genie plugin and related components have been updated to version 3.260314.3 across various configuration and source files, reflecting the latest development state.
Changelog
  • .claude-plugin/marketplace.json
    • Updated the genie plugin version from 3.260310.5 to 3.260314.3.
  • openclaw.plugin.json
    • Updated the Genie plugin version from 3.260310.5 to 3.260314.3.
  • package.json
    • Updated the @automagik/genie package version from 3.260310.5 to 3.260314.3.
  • plugins/genie/.claude-plugin/plugin.json
    • Updated the genie plugin version from 3.260310.5 to 3.260314.3.
  • plugins/genie/package.json
    • Updated the genie-plugin version from 3.260310.5 to 3.260314.3.
  • src/lib/version.ts
    • Updated the VERSION constant from 3.260310.5 to 3.260314.3.
Activity
  • This is an automatically created and maintained rolling promotion PR. No human activity has occurred yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request contains automated version bumps as part of a rolling promotion from the dev branch to main. The version has been consistently updated from 3.260310.5 to 3.260314.3 across all relevant configuration and source files. The changes are correct and align with the expected outcome of the automated versioning script. No issues were found.

@coderabbitai

coderabbitai Bot commented Mar 14, 2026 •

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Runtime VERSION now reads package.json; a new cross-process file-lock utility was added and adopted across state, team, mailbox, registry, and chat modules; teams moved to global GENIE_HOME with API signature changes; update flow became channel-aware and synchronizes plugins; added resetGroup API and CLI reset command; multiple manifest/version bumps.

Changes

Cohort / File(s) Summary
Version bumps & manifests
\.claude-plugin/marketplace.json, openclaw.plugin.json, package.json, plugins/genie/.claude-plugin/plugin.json, plugins/genie/package.json
Bumped package/plugin manifest versions from 3.260310.5 → 3.260314.8.
Runtime version detection
src/lib/version.ts
Replaced hardcoded VERSION with runtime readVersionFromPackageJson() that probes multiple package.json paths and falls back to 0.0.0-unknown.
CI workflow
.github/workflows/version.yml
Added bun run build step and conditional npm publish gated by NPM_TOKEN.
File-lock utility
src/lib/file-lock.ts
New cross-process lock: acquireLock(lockPath), withLock, and LOCK_* constants; stale-cleanup, retries, and timeouts.
Lock adoption — registry & directories
src/lib/agent-directory.ts, src/lib/agent-registry.ts
Removed internal lock code; now call external acquireLock(lockPath) around load/modify/save sequences.
Lock adoption — mailbox & team chat
src/lib/mailbox.ts, src/lib/team-chat.ts
Writes now acquire per-file locks and ensure directories exist before locking.
Wish-state: validation, locking, reset
src/lib/wish-state.ts, src/lib/wish-state.test.ts
Added group validation (self/missing/cycle), switched to external lock, added resetGroup API, tightened completeGroup precondition; tests updated.
Team manager: global storage & API
src/lib/team-manager.ts, src/lib/team-manager.test.ts
Teams moved to global GENIE_HOME (~/.genie, configurable), public APIs changed to drop repo param (getTeam, listTeams, hireAgent, fireAgent, listMembers, disbandTeam); added validateBranchName; tests adapted.
CLI: update command (channel) & state reset
src/genie-commands/update.ts, src/genie.ts, src/term-commands/state.ts
updateCommand gains --next/--stable options, persists channel, passes channel to bun/npm install flows and syncs Claude plugin assets using file-lock; state CLI adds reset <ref> invoking wishState.resetGroup.
Term commands: team/msg/agents adjustments
src/term-commands/team.ts, src/term-commands/msg.ts, src/term-commands/agents.ts
Removed repoPath usage from many team flows and helpers (signatures updated); agent spawn may override agent.repoPath with team worktree; cwd propagation added to tmux spawn.
Dispatch / wish parsing
src/term-commands/dispatch.ts, src/term-commands/dispatch.test.ts
parseWishGroups made case-insensitive (gim) and exported; tests added for varied heading cases.
Protocol router minor
src/lib/protocol-router-spawn.ts
resolveParentSession ignores repoPath and resolves team by name only.
Types: config field added
src/types/genie-config.ts
Added updateChannel: z.enum(['latest','next']).default('latest') to GenieConfigSchema.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant CLI as "CLI (update/reset)"
participant Installer as "Installer (bun/npm)"
participant Lock as "file-lock (acquireLock/withLock)"
participant FS as "Filesystem / Plugin Cache"
CLI->>Installer: install package for channel
Installer-->>FS: write installed package files
CLI->>Lock: acquireLock(pluginCachePath)
Lock-->>CLI: return releaseFn
CLI->>FS: copy/sync plugin assets into cache
CLI->>Lock: call releaseFn()
Lock-->>FS: remove .lock file

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the PR's purpose as a rolling promotion from dev to main branch, which is directly supported by the commit messages and PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Test User and others added 5 commits March 14, 2026 14:12
… version

Group 1: Extract shared file-lock utility (src/lib/file-lock.ts)
- Deduplicate lock pattern from agent-directory, wish-state, agent-registry
- All 3 modules now import from file-lock.ts

Group 4: Spawn & team fixes
- Fix spawn CWD to use team worktree path for built-in agents (#546)
- Add validateBranchName() in team-manager (#551)
- Make parseWishGroups() case-insensitive (#554)

Group 5: Global team configs (#558)
- Move team configs from <repo>/.genie/teams/ to ~/.genie/teams/
- Drop repoPath param from getTeam/listTeams/listMembers
- All team commands resolve repo from stored config, not CWD
- Update all callers across codebase

Group 6: Fix genie update (#559)
- Read version from package.json at runtime instead of hardcoded

3 test failures remain (team-manager signature changes in test mocks)
Groups 2 (concurrency) and 3 (wish state hardening) still pending.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
- Fix 3 failing tests: isolate GENIE_HOME in team.test.ts and
  msg.test.ts so team manager reads from test-local directories
- Add file-lock concurrency protection to mailbox.send(),
  mailbox.markDelivered(), and team-chat.postMessage()
- Add wish-state validation: self-dep, dangling dep, and cycle
  detection (Kahn's algorithm) in createState()
- Enforce strict completeGroup() transitions (must be in_progress)
- Add resetGroup() to revert in_progress groups back to ready
- Add JSDoc to getState() documenting lockless-by-design reads
- Add reset CLI command in state.ts
- Add comprehensive tests for validation and resetGroup
Add build + publish steps to version.yml so dev merges
publish to npm under the `next` dist-tag.

Install with: bun add -g @automagik/genie@next

Co-Authored-By: Paperclip <noreply@paperclip.ing>
genie update --next    Switch to dev builds (@next npm tag)
genie update --stable  Switch to stable releases (@latest npm tag)
genie update           Uses last selected channel (default: latest)

Channel preference persisted in ~/.genie/config.json (updateChannel field).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
fix: v2 QA fixes — 12 bugs, 7 groups, 708 tests pass

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/term-commands/dispatch.ts (2)

55-68: ⚠️ Potential issue | 🟠 Major

extractGroup remains case-sensitive while parseWishGroups is now case-insensitive.

If a WISH.md uses lowercase headings like ### group 1:, parseWishGroups (line 147) will parse them correctly, but extractGroup will fail to find those groups since it uses case-sensitive patterns (lines 56 and 64). This creates inconsistent behavior.

🐛 Proposed fix to make extractGroup case-insensitive
 export function extractGroup(content: string, groupName: string): string | null {
-  const pattern = new RegExp(`^### Group ${escapeRegExp(groupName)}:`, 'm');
+  const pattern = new RegExp(`^### Group ${escapeRegExp(groupName)}:`, 'im');
   const match = content.match(pattern);
   if (!match || match.index === undefined) return null;

   const start = match.index;
   const afterHeading = content.slice(start);

   // Find next group heading or HR separator
-  const nextBoundary = afterHeading.slice(1).search(/^### Group \d|^---$/m);
+  const nextBoundary = afterHeading.slice(1).search(/^### Group \d|^---$/im);
   const end = nextBoundary !== -1 ? start + 1 + nextBoundary : content.length;

   return content.slice(start, end).trim();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/term-commands/dispatch.ts` around lines 55 - 68, extractGroup is using
case-sensitive regexes while parseWishGroups is case-insensitive, causing
mismatches for headings like "### group 1:"; update extractGroup to use
case-insensitive matching by adding the 'i' flag to the RegExp constructed in
pattern (where escapeRegExp(groupName) is used) and make the nextBoundary search
regex (/^### Group \d|^---$/m) also case-insensitive (add the 'i' flag) so both
the heading detection and the next-boundary search will match
lowercase/uppercase variants consistently.

147-157: ⚠️ Potential issue | 🟠 Major

Inconsistent case-sensitivity between group matching regexes.

Line 147 uses /gim (case-insensitive), but line 156 uses /m without i. If a document has mixed-case headings like ### group 1: followed by ### GROUP 2:, the main loop will find both groups, but the nextGroupIdx search will fail to find the next group boundary when it uses different casing.

🐛 Proposed fix
     // Find the next group heading or end of content
     const rest = content.slice(start + match[0].length);
-    const nextGroupIdx = rest.search(/^### Group \d+:/m);
+    const nextGroupIdx = rest.search(/^### Group \d+:/im);
     const section = nextGroupIdx !== -1 ? rest.slice(0, nextGroupIdx) : rest;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/term-commands/dispatch.ts` around lines 147 - 157, The group boundary
search is using inconsistent regex flags: groupPattern is /^### Group (\d+):/gim
but nextGroupIdx uses rest.search(/^### Group \d+:/m), which breaks on
mixed-case headings; update the search to use the same case-insensitive pattern
(use the same flags as groupPattern or at least add the i flag) when computing
nextGroupIdx so the loop and boundary detection are consistent (refer to
groupPattern, match, rest, nextGroupIdx and content to locate and fix).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/version.yml:
- Around line 84-98: Workflow pushes a git tag before running the "Build CLI"
and "Publish dev release to npm" steps, risking orphan tags if the build/publish
fails; move the tag push so it occurs after the "Publish dev release to npm"
step (only on successful publish) or add cleanup logic to delete the pushed tag
on publish failure (e.g., in the publish step's failure handler) so tags are
only kept when bun publish --tag next succeeds; update the job that currently
performs the tag push to reference the new post-publish placement and ensure it
runs conditionally based on publish success.

In `@src/genie-commands/update.ts`:
- Around line 289-293: The resolveChannel function currently prefers
options.next when both options.next and options.stable are passed; change it to
detect the conflicting flags (both true) and throw a user-facing error (e.g.,
throw new Error or a CLI-specific exit with a clear message) instead of silently
choosing 'next'; update the same conflicting-flag logic in the other
channel-resolution block in this file (the second branch handling next/stable
around lines 317-327) to perform the same validation so any input with both
--next and --stable fails fast and consistently.
- Around line 307-314: persistChannel currently swallows any error from
loadGenieConfig()/saveGenieConfig, hiding failures when users explicitly switch
channels; update persistChannel to handle errors by catching exceptions from
loadGenieConfig/saveGenieConfig and either logging a clear, user-facing error
(e.g., via console.error or the existing logger) that includes the caught error
details and the channel attempted, and then rethrow or return a rejected Promise
so callers of persistChannel (the command handler for update --next/--stable)
can surface the failure to the user; reference the persistChannel function and
the loadGenieConfig/saveGenieConfig calls to locate where to add the error
reporting and propagation.

In `@src/genie.ts`:
- Around line 97-99: Update the .description(...) text for the Update command to
reflect both channel options instead of saying "latest version": change the
string passed to the .description call (adjacent to .option('--next', ...) and
.option('--stable', ...)) to mention that the command can switch channels
(stable/@latest or dev/@next) or to a specific channel like "Update or switch
Genie CLI channel (stable `@latest` or dev `@next`)"; keep the rest of the command
builder intact.

In `@src/lib/file-lock.ts`:
- Around line 83-89: The current timeout branch unlinks lockPath when Date.now()
> deadline which can evict a live holder; instead, change the timeout handling
in the acquire/lock loop to fail fast: do not call unlink(lockPath) on timeout,
and throw a clear LockTimeout error (include lockPath and LOCK_TIMEOUT_MS) so
the caller can handle retry/abort; if you must attempt removal, first read the
lock file and verify it is actually stale (compare embedded timestamp or owner
PID) and that the owning process is dead before unlinking—ensure release() still
only removes locks it created.
- Around line 72-78: The lock acquisition fails if the lock file's parent
directory doesn't exist; in acquireLock/tryCreateLock ensure the parent
directory for lockPath is created before attempting open(lockPath, 'wx'):
compute the parent with path.dirname(lockPath) and call
fs.promises.mkdir(parentDir, { recursive: true }) (or equivalent) prior to
opening the lock file, and swallow/ignore EEXIST from mkdir so concurrent
callers don't error; keep existing retry/timeout logic in acquireLock and only
add the directory-creation step before tryCreateLock attempts to open the lock
file.

In `@src/lib/team-chat.ts`:
- Around line 55-73: Compute chatFilePath(repoPath, teamName) once and reuse it
instead of calling chatFilePath twice: call const filePath =
chatFilePath(repoPath, teamName) before acquireLock, pass filePath into
acquireLock and use the same filePath for appendFile, keeping the existing
chatDir(dir)/mkdir and release() logic unchanged (symbols: chatDir,
chatFilePath, acquireLock, appendFile, release, msg).

In `@src/lib/team-manager.test.ts`:
- Line 220: The test is using expect(...).rejects without awaiting it, causing
false positives; update the two assertions that call
expect(hireAgent('nonexistent', 'agent')).rejects.toThrow(...) (and the similar
one at the other occurrence) to await the assertion (i.e., prepend await) so the
promise rejection is actually observed and the toThrow check is executed against
hireAgent in the test for the hireAgent function.
- Around line 51-53: Save the current value of process.env.GENIE_HOME before
setting it to TEST_GENIE_HOME in the test setup, and restore that saved value in
the test teardown (e.g., inside afterEach or afterAll). Specifically, capture
const originalGenieHome = process.env.GENIE_HOME before mutating
process.env.GENIE_HOME = TEST_GENIE_HOME, and in the teardown restore
process.env.GENIE_HOME = originalGenieHome (or delete it if originalGenieHome
was undefined) so global env state is not leaked by the tests.

In `@src/lib/team-manager.ts`:
- Around line 57-65: The teamFilePath/safeFileName logic is producing lossy,
repo-agnostic filenames causing collisions across repos and branch names; update
safeFileName/teamFilePath to include a unique repo identifier (e.g., owner/name
or repo path) when building the filename and use a reversible, collision-safe
encoding (such as base64 or encodeURIComponent) instead of simple slash-to-dash
replacement; ensure the same change is applied to the other occurrence
referenced by createTeam (functions/methods: safeFileName, teamFilePath, and
createTeam) so filenames are unique per repo and no two branch names map to the
same file.
- Around line 84-105: The validateBranchName function currently misses several
git-invalid cases; update validateBranchName to reject empty string (if name ===
''), consecutive slashes (if name.includes('//')), the @{ sequence (if
name.includes('@{')), and any path segment that begins with a dot (e.g., .hidden
— implement with name.split('/').some(seg => seg.startsWith('.')) or at minimum
name.startsWith('.')). Add these checks to push corresponding messages into the
errors array (same style as the existing checks) or replace the whole validator
by delegating to Git's check-ref-format; reference validateBranchName when
making the changes.

In `@src/lib/wish-state.ts`:
- Around line 119-180: The validation currently allows duplicate group.name
values which cause later entries to overwrite earlier ones and mask errors;
update validateGroupRefs (called by validateGroups) to detect duplicates by
iterating groups and tracking seen names, throwing an Error when a duplicate
group.name is found (include the offending group.name in the message); also
harden detectCycles initialization (inDegree and adjacency setup used by
detectCycles) to assert or throw if a name is already present when building
those maps to avoid silent overwrites.

In `@src/term-commands/agents.ts`:
- Around line 745-749: The team worktree override is clobbering authoritative
spawn roots: change the block that sets agent = { ...agent, repoPath:
teamConfig.worktreePath } so it only applies when no explicit spawn cwd or
directory-agent root exists; specifically, in resolveAgentForSpawn (and the code
using teamManager.getTeam), check options?.cwd first and if present do not
override agent.repoPath, and also detect directory-agent roots (e.g., agent.type
=== 'directory' or agent.dir present) and skip applying teamConfig.worktreePath
for those agents—only set agent.repoPath to teamConfig.worktreePath when neither
options.cwd nor a directory-agent root is defined.

In `@src/term-commands/dispatch.test.ts`:
- Around line 510-545: Add unit tests that exercise extractGroup directly to
assert it is case-insensitive (matching the behavior of parseWishGroups): create
test cases in dispatch.test.ts that call extractGroup with headings like '###
group 1: Foo' and '### Group 2: Bar' and assert the function returns the
expected group object (not null) and extracts name '1'/'2' and proper dependsOn
parsing; also include a mixed-case heading test to ensure extractGroup('###
GROUP 3: Baz') behaves the same as parseWishGroups for group detection and
dependency parsing. Ensure tests reference the extractGroup function by name so
regressions are caught.
- Around line 529-533: Update the test for parseWishGroups to assert not just
groups.length but also that the parsed group objects contain the correct names
and dependencies: after calling parseWishGroups(content) check that one group
has name "GROUP 1: Loud" with depends-on equal to "none" (or empty array as your
parser returns) and the other has name "Group 2: Normal" with depends-on
including "Group 1"; use the actual property names from the parseWishGroups
return shape (e.g., groups[i].name and groups[i].dependsOn or
groups[i].dependencies) to make the assertions concrete so case variations and
dependency parsing are validated.

In `@src/term-commands/team.ts`:
- Around line 145-152: autoDetectTeam() currently only uses GENIE_TEAM or a
single global team; add a CWD-based fallback so when multiple teams exist the
function returns the team matching the current working directory/worktree. After
obtaining teams from teamManager.listTeams(), if teams.length > 1 scan teams for
one whose configured repo root or path (e.g., team.root, team.path or similar
property) is an ancestor of process.cwd() or matches the git repo/worktree of
the CWD and return that team's name; only return null if no match is found. Use
existing symbols autoDetectTeam and teamManager.listTeams() and check each
team's path/root property to perform the ancestor/match test.

---

Outside diff comments:
In `@src/term-commands/dispatch.ts`:
- Around line 55-68: extractGroup is using case-sensitive regexes while
parseWishGroups is case-insensitive, causing mismatches for headings like "###
group 1:"; update extractGroup to use case-insensitive matching by adding the
'i' flag to the RegExp constructed in pattern (where escapeRegExp(groupName) is
used) and make the nextBoundary search regex (/^### Group \d|^---$/m) also
case-insensitive (add the 'i' flag) so both the heading detection and the
next-boundary search will match lowercase/uppercase variants consistently.
- Around line 147-157: The group boundary search is using inconsistent regex
flags: groupPattern is /^### Group (\d+):/gim but nextGroupIdx uses
rest.search(/^### Group \d+:/m), which breaks on mixed-case headings; update the
search to use the same case-insensitive pattern (use the same flags as
groupPattern or at least add the i flag) when computing nextGroupIdx so the loop
and boundary detection are consistent (refer to groupPattern, match, rest,
nextGroupIdx and content to locate and fix).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 647ec916-3d85-44e8-b8e9-6ecb369bfd81

📥 Commits

Reviewing files that changed from the base of the PR and between 9958df1 and ac5390f.

📒 Files selected for processing (23)
  • .github/workflows/version.yml
  • src/genie-commands/update.ts
  • src/genie.ts
  • src/lib/agent-directory.ts
  • src/lib/agent-registry.ts
  • src/lib/file-lock.ts
  • src/lib/mailbox.ts
  • src/lib/protocol-router-spawn.ts
  • src/lib/team-chat.ts
  • src/lib/team-manager.test.ts
  • src/lib/team-manager.ts
  • src/lib/version.ts
  • src/lib/wish-state.test.ts
  • src/lib/wish-state.ts
  • src/term-commands/agents.ts
  • src/term-commands/dispatch.test.ts
  • src/term-commands/dispatch.ts
  • src/term-commands/msg.test.ts
  • src/term-commands/msg.ts
  • src/term-commands/state.ts
  • src/term-commands/team.test.ts
  • src/term-commands/team.ts
  • src/types/genie-config.ts

Comment on lines +84 to +98

- name: Build CLI
run: bun run build

- name: Publish dev release to npm
env:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
NPM_CONFIG_TOKEN: ${{ secrets.NPM_TOKEN }}
HUSKY: "0"
run: |
if [ -z "$NPM_TOKEN" ]; then
echo "⚠️ NPM_TOKEN not set — skipping dev publish"
exit 0
fi
bun publish --access public --tag next

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tag pushed before publish—orphan tags possible on failure.

The workflow pushes the git tag at line 83 before attempting npm publish. If the build or publish step fails, the tag remains in the repo without a corresponding npm release. Consider either:

  1. Moving the tag push after successful publish, or
  2. Adding cleanup logic to delete the tag on publish failure.
Potential fix: reorder tag push after publish
      - name: Commit and tag
        run: |
          VERSION="${{ steps.version.outputs.version }}"

          git add -A '*.json' 'src/lib/version.ts'
          if git diff --cached --quiet; then
            echo "No version changes to commit"
          else
            git commit -m "chore(version): bump to ${VERSION} [skip ci]"
          fi

          git tag "v${VERSION}"
-          git push --atomic origin HEAD:refs/heads/dev "refs/tags/v${VERSION}"
+          git push origin HEAD:refs/heads/dev

      - name: Build CLI
        run: bun run build

      - name: Publish dev release to npm
        env:
          NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
          NPM_CONFIG_TOKEN: ${{ secrets.NPM_TOKEN }}
          HUSKY: "0"
        run: |
          if [ -z "$NPM_TOKEN" ]; then
            echo "⚠️ NPM_TOKEN not set — skipping dev publish"
            exit 0
          fi
          bun publish --access public --tag next

+      - name: Push tag after successful publish
+        run: |
+          VERSION="${{ steps.version.outputs.version }}"
+          git push origin "refs/tags/v${VERSION}"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/version.yml around lines 84 - 98, Workflow pushes a git
tag before running the "Build CLI" and "Publish dev release to npm" steps,
risking orphan tags if the build/publish fails; move the tag push so it occurs
after the "Publish dev release to npm" step (only on successful publish) or add
cleanup logic to delete the pushed tag on publish failure (e.g., in the publish
step's failure handler) so tags are only kept when bun publish --tag next
succeeds; update the job that currently performs the tag push to reference the
new post-publish placement and ensure it runs conditionally based on publish
success.

Comment on lines +289 to +293
async function resolveChannel(options: { next?: boolean; stable?: boolean }): Promise<string> {
// Explicit flags override everything
if (options.next) return 'next';
if (options.stable) return 'latest';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject conflicting channel flags instead of silently preferring --next.

If users pass both --next and --stable, the current logic picks next (Line 291) and persists it. This is ambiguous input and should fail fast.

Proposed fix
 export async function updateCommand(options: { next?: boolean; stable?: boolean } = {}): Promise<void> {
+  if (options.next && options.stable) {
+    error('Choose either --next or --stable, not both.');
+    process.exit(1);
+  }
+
   console.log();
   console.log('\x1b[1m🧞 Genie CLI Update\x1b[0m');
   console.log('\x1b[2m────────────────────────────────────\x1b[0m');
   console.log();

Also applies to: 317-327

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/genie-commands/update.ts` around lines 289 - 293, The resolveChannel
function currently prefers options.next when both options.next and
options.stable are passed; change it to detect the conflicting flags (both true)
and throw a user-facing error (e.g., throw new Error or a CLI-specific exit with
a clear message) instead of silently choosing 'next'; update the same
conflicting-flag logic in the other channel-resolution block in this file (the
second branch handling next/stable around lines 317-327) to perform the same
validation so any input with both --next and --stable fails fast and
consistently.

Comment on lines +307 to +314
async function persistChannel(channel: string): Promise<void> {
try {
const config = await loadGenieConfig();
config.updateChannel = channel as 'latest' | 'next';
await saveGenieConfig(config);
} catch {
// Non-fatal — channel preference lost but update still works
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don’t silently ignore channel persistence failures after explicit switch requests.

When users run genie update --next|--stable, failing to save that preference is meaningful; swallowing the error makes later behavior inconsistent without explanation.

Proposed fix
 async function persistChannel(channel: string): Promise<void> {
   try {
     const config = await loadGenieConfig();
     config.updateChannel = channel as 'latest' | 'next';
     await saveGenieConfig(config);
-  } catch {
-    // Non-fatal — channel preference lost but update still works
+  } catch (err) {
+    const message = err instanceof Error ? err.message : String(err);
+    console.warn(`Warning: failed to persist update channel "${channel}": ${message}`);
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/genie-commands/update.ts` around lines 307 - 314, persistChannel
currently swallows any error from loadGenieConfig()/saveGenieConfig, hiding
failures when users explicitly switch channels; update persistChannel to handle
errors by catching exceptions from loadGenieConfig/saveGenieConfig and either
logging a clear, user-facing error (e.g., via console.error or the existing
logger) that includes the caught error details and the channel attempted, and
then rethrow or return a rejected Promise so callers of persistChannel (the
command handler for update --next/--stable) can surface the failure to the user;
reference the persistChannel function and the loadGenieConfig/saveGenieConfig
calls to locate where to add the error reporting and propagation.

Comment thread src/genie.ts
Comment on lines +97 to +99
.description('Update Genie CLI to the latest version')
.option('--next', 'Switch to dev builds (npm @next tag)')
.option('--stable', 'Switch to stable releases (npm @latest tag)')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update command description is outdated.

Line 97 says “latest version”, but Lines 98-99 now support both stable (latest) and dev (next) channels. Please make the description channel-aware to avoid confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/genie.ts` around lines 97 - 99, Update the .description(...) text for the
Update command to reflect both channel options instead of saying "latest
version": change the string passed to the .description call (adjacent to
.option('--next', ...) and .option('--stable', ...)) to mention that the command
can switch channels (stable/@latest or dev/@next) or to a specific channel like
"Update or switch Genie CLI channel (stable `@latest` or dev `@next`)"; keep the
rest of the command builder intact.

Comment thread src/lib/file-lock.ts
Comment on lines +72 to +78
export async function acquireLock(filePath: string): Promise<() => Promise<void>> {
const lockPath = `${filePath}.lock`;
const deadline = Date.now() + LOCK_TIMEOUT_MS;

while (true) {
const release = await tryCreateLock(lockPath);
if (release) return release;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Create the lock directory before opening ${filePath}.lock.

open(lockPath, 'wx') throws ENOENT when the parent directory does not exist. Callers like src/lib/agent-directory.ts acquire the lock before they call mkdir(), so first-use commands on a clean GENIE_HOME will fail instead of initializing state.

🛠️ Proposed fix
-import { open, stat, unlink } from 'node:fs/promises';
+import { mkdir, open, stat, unlink } from 'node:fs/promises';
+import { dirname } from 'node:path';
@@
 export async function acquireLock(filePath: string): Promise<() => Promise<void>> {
   const lockPath = `${filePath}.lock`;
+  await mkdir(dirname(lockPath), { recursive: true });
   const deadline = Date.now() + LOCK_TIMEOUT_MS;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/file-lock.ts` around lines 72 - 78, The lock acquisition fails if the
lock file's parent directory doesn't exist; in acquireLock/tryCreateLock ensure
the parent directory for lockPath is created before attempting open(lockPath,
'wx'): compute the parent with path.dirname(lockPath) and call
fs.promises.mkdir(parentDir, { recursive: true }) (or equivalent) prior to
opening the lock file, and swallow/ignore EEXIST from mkdir so concurrent
callers don't error; keep existing retry/timeout logic in acquireLock and only
add the directory-creation step before tryCreateLock attempts to open the lock
file.

Comment thread src/lib/wish-state.ts
Comment on lines +119 to +180
function validateGroupRefs(groups: GroupDefinition[]): void {
const groupNames = new Set(groups.map((g) => g.name));

for (const group of groups) {
if (group.dependsOn?.includes(group.name)) {
throw new Error(`Group "${group.name}" depends on itself`);
}
for (const dep of group.dependsOn ?? []) {
if (!groupNames.has(dep)) {
throw new Error(`Group "${group.name}" depends on non-existent group "${dep}"`);
}
}
}
}

/** Detect dependency cycles using Kahn's topological sort algorithm. */
function detectCycles(groups: GroupDefinition[]): void {
const inDegree: Record<string, number> = {};
const adjacency: Record<string, string[]> = {};

for (const group of groups) {
inDegree[group.name] = (group.dependsOn ?? []).length;
adjacency[group.name] = [];
}
for (const group of groups) {
for (const dep of group.dependsOn ?? []) {
adjacency[dep].push(group.name);
}
}

const queue: string[] = Object.entries(inDegree)
.filter(([, deg]) => deg === 0)
.map(([name]) => name);
let processed = 0;

while (queue.length > 0) {
const node = queue.shift();
if (!node) break;
processed++;
for (const neighbor of adjacency[node]) {
inDegree[neighbor]--;
if (inDegree[neighbor] === 0) {
queue.push(neighbor);
}
}
}

if (processed !== groups.length) {
const remaining = Object.entries(inDegree)
.filter(([, deg]) => deg > 0)
.map(([name]) => name);
throw new Error(`Dependency cycle detected among groups: ${remaining.join(', ')}`);
}
}

/**
* Validate group definitions: no self-deps, no dangling deps, no cycles.
* Throws on the first violation found.
*/
function validateGroups(groups: GroupDefinition[]): void {
validateGroupRefs(groups);
detectCycles(groups);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject duplicate group names during validation.

This path still accepts repeated group.name values. Because groupEntries, inDegree, and adjacency are keyed by name, the later definition silently overwrites the earlier one and can mask dependencies or cycles.

🛠️ Proposed fix
 function validateGroupRefs(groups: GroupDefinition[]): void {
-  const groupNames = new Set(groups.map((g) => g.name));
+  const groupNames = new Set<string>();
+
+  for (const group of groups) {
+    if (groupNames.has(group.name)) {
+      throw new Error(`Duplicate group name "${group.name}"`);
+    }
+    groupNames.add(group.name);
+  }
 
   for (const group of groups) {
     if (group.dependsOn?.includes(group.name)) {
       throw new Error(`Group "${group.name}" depends on itself`);
     }

Also applies to: 198-205

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/wish-state.ts` around lines 119 - 180, The validation currently
allows duplicate group.name values which cause later entries to overwrite
earlier ones and mask errors; update validateGroupRefs (called by
validateGroups) to detect duplicates by iterating groups and tracking seen
names, throwing an Error when a duplicate group.name is found (include the
offending group.name in the message); also harden detectCycles initialization
(inDegree and adjacency setup used by detectCycles) to assert or throw if a name
is already present when building those maps to avoid silent overwrites.

Comment on lines +745 to +749
// 2b. Override CWD with team worktree path if available
const teamConfig = await teamManager.getTeam(team);
if (teamConfig?.worktreePath) {
agent = { ...agent, repoPath: teamConfig.worktreePath };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep --cwd and directory-agent spawn roots authoritative.

resolveAgentForSpawn() already gives options.cwd precedence, but this block overwrites it for every team spawn. It also ignores directory entries whose dir is the documented spawn CWD, so custom agents can start in the wrong folder as soon as the team has a worktree.

🛠️ Proposed fix
-  if (teamConfig?.worktreePath) {
+  if (teamConfig?.worktreePath && !options.cwd && !agent.entry.dir) {
     agent = { ...agent, repoPath: teamConfig.worktreePath };
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/term-commands/agents.ts` around lines 745 - 749, The team worktree
override is clobbering authoritative spawn roots: change the block that sets
agent = { ...agent, repoPath: teamConfig.worktreePath } so it only applies when
no explicit spawn cwd or directory-agent root exists; specifically, in
resolveAgentForSpawn (and the code using teamManager.getTeam), check
options?.cwd first and if present do not override agent.repoPath, and also
detect directory-agent roots (e.g., agent.type === 'directory' or agent.dir
present) and skip applying teamConfig.worktreePath for those agents—only set
agent.repoPath to teamConfig.worktreePath when neither options.cwd nor a
directory-agent root is defined.

Comment on lines +510 to +545
describe('parseWishGroups()', () => {
it('should parse standard Group headings', () => {
const groups = parseWishGroups(SAMPLE_WISH);
expect(groups.length).toBe(3);
expect(groups[0].name).toBe('1');
expect(groups[1].name).toBe('2');
expect(groups[2].name).toBe('3');
});

it('should parse lowercase group headings (case-insensitive)', () => {
const content = '### group 1: Test\n**depends-on:** none\n\n### group 2: Next\n**depends-on:** Group 1';
const groups = parseWishGroups(content);
expect(groups.length).toBe(2);
expect(groups[0].name).toBe('1');
expect(groups[0].dependsOn).toEqual([]);
expect(groups[1].name).toBe('2');
expect(groups[1].dependsOn).toEqual(['1']);
});

it('should parse mixed case group headings', () => {
const content = '### GROUP 1: Loud\n**depends-on:** none\n\n### Group 2: Normal\n**depends-on:** Group 1';
const groups = parseWishGroups(content);
expect(groups.length).toBe(2);
});

it('should parse depends-on with Group prefix', () => {
const groups = parseWishGroups(SAMPLE_WISH);
expect(groups[1].dependsOn).toEqual(['1']);
expect(groups[2].dependsOn).toEqual(['2']);
});

it('should handle depends-on: none', () => {
const groups = parseWishGroups(SAMPLE_WISH);
expect(groups[0].dependsOn).toEqual([]);
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Tests for extractGroup with case-insensitive headings are missing.

Given the change to make parseWishGroups case-insensitive, there should be corresponding tests for extractGroup to verify it handles lowercase/mixed-case headings. Currently, if someone uses ### group 1:, parseWishGroups will find it but extractGroup will return null.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/term-commands/dispatch.test.ts` around lines 510 - 545, Add unit tests
that exercise extractGroup directly to assert it is case-insensitive (matching
the behavior of parseWishGroups): create test cases in dispatch.test.ts that
call extractGroup with headings like '### group 1: Foo' and '### Group 2: Bar'
and assert the function returns the expected group object (not null) and
extracts name '1'/'2' and proper dependsOn parsing; also include a mixed-case
heading test to ensure extractGroup('### GROUP 3: Baz') behaves the same as
parseWishGroups for group detection and dependency parsing. Ensure tests
reference the extractGroup function by name so regressions are caught.

Comment on lines +529 to +533
it('should parse mixed case group headings', () => {
const content = '### GROUP 1: Loud\n**depends-on:** none\n\n### Group 2: Normal\n**depends-on:** Group 1';
const groups = parseWishGroups(content);
expect(groups.length).toBe(2);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Mixed-case test could be more thorough.

This test only verifies groups.length. Consider also asserting the group names and dependencies to ensure both headings are parsed correctly with their respective cases.

💡 Suggested enhancement
   it('should parse mixed case group headings', () => {
     const content = '### GROUP 1: Loud\n**depends-on:** none\n\n### Group 2: Normal\n**depends-on:** Group 1';
     const groups = parseWishGroups(content);
     expect(groups.length).toBe(2);
+    expect(groups[0].name).toBe('1');
+    expect(groups[0].dependsOn).toEqual([]);
+    expect(groups[1].name).toBe('2');
+    expect(groups[1].dependsOn).toEqual(['1']);
   });
📝 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.

Suggested change
it('should parse mixed case group headings', () => {
const content = '### GROUP 1: Loud\n**depends-on:** none\n\n### Group 2: Normal\n**depends-on:** Group 1';
const groups = parseWishGroups(content);
expect(groups.length).toBe(2);
});
it('should parse mixed case group headings', () => {
const content = '### GROUP 1: Loud\n**depends-on:** none\n\n### Group 2: Normal\n**depends-on:** Group 1';
const groups = parseWishGroups(content);
expect(groups.length).toBe(2);
expect(groups[0].name).toBe('1');
expect(groups[0].dependsOn).toEqual([]);
expect(groups[1].name).toBe('2');
expect(groups[1].dependsOn).toEqual(['1']);
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/term-commands/dispatch.test.ts` around lines 529 - 533, Update the test
for parseWishGroups to assert not just groups.length but also that the parsed
group objects contain the correct names and dependencies: after calling
parseWishGroups(content) check that one group has name "GROUP 1: Loud" with
depends-on equal to "none" (or empty array as your parser returns) and the other
has name "Group 2: Normal" with depends-on including "Group 1"; use the actual
property names from the parseWishGroups return shape (e.g., groups[i].name and
groups[i].dependsOn or groups[i].dependencies) to make the assertions concrete
so case variations and dependency parsing are validated.

Comment thread src/term-commands/team.ts
Comment on lines +145 to 152
async function autoDetectTeam(): Promise<string | null> {
const envTeam = process.env.GENIE_TEAM;
if (envTeam) return envTeam;

const teams = await teamManager.listTeams(repoPath);
const teams = await teamManager.listTeams();
if (teams.length === 1) return teams[0].name;

return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add CWD-based fallback in autoDetectTeam() to preserve context auto-detection.

This implementation only resolves by GENIE_TEAM or “single team globally.” In multi-team environments, commands can fail with “Could not detect team” even when run inside a team worktree.

Proposed fix
 async function autoDetectTeam(): Promise<string | null> {
   const envTeam = process.env.GENIE_TEAM;
   if (envTeam) return envTeam;
 
   const teams = await teamManager.listTeams();
   if (teams.length === 1) return teams[0].name;
+
+  // Fallback: infer from current working directory when inside a team worktree.
+  const cwd = process.cwd();
+  const matches = teams.filter((t) => cwd === t.worktreePath || cwd.startsWith(`${t.worktreePath}/`));
+  if (matches.length === 1) return matches[0].name;
 
   return null;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/term-commands/team.ts` around lines 145 - 152, autoDetectTeam() currently
only uses GENIE_TEAM or a single global team; add a CWD-based fallback so when
multiple teams exist the function returns the team matching the current working
directory/worktree. After obtaining teams from teamManager.listTeams(), if
teams.length > 1 scan teams for one whose configured repo root or path (e.g.,
team.root, team.path or similar property) is an ancestor of process.cwd() or
matches the git repo/worktree of the CWD and return that team's name; only
return null if no match is found. Use existing symbols autoDetectTeam and
teamManager.listTeams() and check each team's path/root property to perform the
ancestor/match test.

Test User and others added 6 commits March 14, 2026 15:44
After bun/npm install, copies the plugin from the installed package
to ~/.claude/plugins/cache/automagik/genie/<version>/ and updates
installed_plugins.json so Claude Code loads the new skills/hooks
without manual reinstall.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
- Pass installType to syncPlugin() so bun updates resolve bun path
  and npm updates resolve npm path (no stale cross-resolution)
- Use `npm root -g` for dynamic npm global dir resolution
  (supports nvm/fnm/volta managed installs)
- Keep fallback chain for edge cases

Co-Authored-By: Paperclip <noreply@paperclip.ing>
feat: genie update syncs plugin + --next/--stable channels + @next npm publish

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/genie-commands/update.ts`:
- Around line 375-394: The read-modify-write of installed_plugins.json via
registryPath is not atomic and can race; change the update in the registry
update block to perform an atomic write (write JSON to a temporary file in the
same directory, fsync the temp file, then rename/move it to overwrite
installed_plugins.json) and consider creating a timestamped backup before
replacing; ensure you still parse the file (readFileSync/JSON.parse) and update
the same entries logic (registry.plugins?.['genie@automagik'], entry.scope ===
'user', set installPath/version/lastUpdated) but replace
writeFileSync(registryPath, ...) with the temp-file-write+fsync+rename (or use
an advisory lock around the read-modify-write if available in your environment)
so the operation is atomic and recoverable.
- Around line 293-304: The current copyDirSync follows symlinks because it
treats non-directory Dirent entries the same and calls copyFileSync, which will
fail for symlinks to directories; update copyDirSync to explicitly detect
symlinks (use entry.isSymbolicLink() and/or lstat) before copying: when a
symlink is found, call readlink to get its target and create an equivalent
symlink at dest (using symlink), rather than calling copyFileSync; keep the
existing recursion for real directories (entry.isDirectory()) and use
copyFileSync only for regular files—refer to the copyDirSync function,
entry.isDirectory(), entry.isSymbolicLink(), copyFileSync, readdirSync, readlink
and symlink handling to locate where to add this logic.
- Around line 350-358: The code reads package.version into the variable version
and then uses it to build cacheDir and later calls rmSync, which is unsafe if
version contains path separators; sanitize or validate version before use (e.g.,
allow only a strict semver-like pattern or escape/remove path separators) and
construct cacheDir from the sanitized value; update the logic where cacheDir is
computed (references: version, cacheDir, globalPkgDir, join) and ensure rmSync
is only called on the validated/sanitized path to prevent directory traversal or
accidental deletes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4c283b4a-16a0-48c0-9ff4-59e02faa920d

📥 Commits

Reviewing files that changed from the base of the PR and between ac5390f and afd9bad.

📒 Files selected for processing (6)
  • .claude-plugin/marketplace.json
  • openclaw.plugin.json
  • package.json
  • plugins/genie/.claude-plugin/plugin.json
  • plugins/genie/package.json
  • src/genie-commands/update.ts

Comment on lines +293 to +304
function copyDirSync(src: string, dest: string): void {
mkdirSync(dest, { recursive: true });
for (const entry of readdirSync(src, { withFileTypes: true })) {
const srcPath = join(src, entry.name);
const destPath = join(dest, entry.name);
if (entry.isDirectory()) {
copyDirSync(srcPath, destPath);
} else {
copyFileSync(srcPath, destPath);
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Symlinks are followed but not explicitly handled.

copyFileSync follows symlinks by default. If the plugin source contains symlinks to directories, entry.isDirectory() returns false for the symlink entry, causing copyFileSync to fail. This is unlikely for typical npm packages but worth noting.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/genie-commands/update.ts` around lines 293 - 304, The current copyDirSync
follows symlinks because it treats non-directory Dirent entries the same and
calls copyFileSync, which will fail for symlinks to directories; update
copyDirSync to explicitly detect symlinks (use entry.isSymbolicLink() and/or
lstat) before copying: when a symlink is found, call readlink to get its target
and create an equivalent symlink at dest (using symlink), rather than calling
copyFileSync; keep the existing recursion for real directories
(entry.isDirectory()) and use copyFileSync only for regular files—refer to the
copyDirSync function, entry.isDirectory(), entry.isSymbolicLink(), copyFileSync,
readdirSync, readlink and symlink handling to locate where to add this logic.

Comment on lines +350 to +358
// Read version from installed package
let version: string;
try {
const pkg = JSON.parse(readFileSync(join(globalPkgDir, 'package.json'), 'utf-8'));
version = pkg.version;
} catch {
log('Could not read package version — skipping plugin sync');
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Version string used in path construction without sanitization.

The version from package.json is directly used in cacheDir path construction (Line 362). A malicious version string containing path separators (e.g., ../..) could cause rmSync to delete unintended directories. While the package is self-published and low risk in practice, defensive sanitization is advisable.

Proposed fix
   try {
     const pkg = JSON.parse(readFileSync(join(globalPkgDir, 'package.json'), 'utf-8'));
-    version = pkg.version;
+    version = pkg.version?.replace(/[/\\]/g, '_');
+    if (!version || !/^\d+\.\d+/.test(version)) {
+      log('Invalid package version — skipping plugin sync');
+      return;
+    }
   } catch {
📝 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.

Suggested change
// Read version from installed package
let version: string;
try {
const pkg = JSON.parse(readFileSync(join(globalPkgDir, 'package.json'), 'utf-8'));
version = pkg.version;
} catch {
log('Could not read package version — skipping plugin sync');
return;
}
// Read version from installed package
let version: string | undefined;
try {
const pkg = JSON.parse(readFileSync(join(globalPkgDir, 'package.json'), 'utf-8'));
version = pkg.version?.replace(/[/\\]/g, '_');
if (!version || !/^\d+\.\d+/.test(version)) {
log('Invalid package version — skipping plugin sync');
return;
}
} catch {
log('Could not read package version — skipping plugin sync');
return;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/genie-commands/update.ts` around lines 350 - 358, The code reads
package.version into the variable version and then uses it to build cacheDir and
later calls rmSync, which is unsafe if version contains path separators;
sanitize or validate version before use (e.g., allow only a strict semver-like
pattern or escape/remove path separators) and construct cacheDir from the
sanitized value; update the logic where cacheDir is computed (references:
version, cacheDir, globalPkgDir, join) and ensure rmSync is only called on the
validated/sanitized path to prevent directory traversal or accidental deletes.

Comment on lines +375 to +394
// Update installed_plugins.json registry
const registryPath = join(claudePlugins, 'installed_plugins.json');
try {
if (existsSync(registryPath)) {
const registry = JSON.parse(readFileSync(registryPath, 'utf-8'));
const entries = registry.plugins?.['genie@automagik'];
if (Array.isArray(entries)) {
for (const entry of entries) {
if (entry.scope === 'user') {
entry.installPath = cacheDir;
entry.version = version;
entry.lastUpdated = new Date().toISOString();
}
}
writeFileSync(registryPath, JSON.stringify(registry, null, 2));
}
}
} catch (err) {
log(`Registry update failed (non-fatal): ${err}`);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Registry update is not atomic.

Read-modify-write on installed_plugins.json without locking could race with Claude Code. Probability is low (manual update command), but consider a backup or temp-file-rename pattern if this causes issues in practice.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/genie-commands/update.ts` around lines 375 - 394, The read-modify-write
of installed_plugins.json via registryPath is not atomic and can race; change
the update in the registry update block to perform an atomic write (write JSON
to a temporary file in the same directory, fsync the temp file, then rename/move
it to overwrite installed_plugins.json) and consider creating a timestamped
backup before replacing; ensure you still parse the file
(readFileSync/JSON.parse) and update the same entries logic
(registry.plugins?.['genie@automagik'], entry.scope === 'user', set
installPath/version/lastUpdated) but replace writeFileSync(registryPath, ...)
with the temp-file-write+fsync+rename (or use an advisory lock around the
read-modify-write if available in your environment) so the operation is atomic
and recoverable.

Test User and others added 4 commits March 14, 2026 16:33
The worktree CWD override in handleWorkerSpawn set ctx.cwd correctly
but tmux split-window inherited the parent pane's CWD instead.
Adding -c flag ensures the spawned pane starts in the team's worktree.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
fix(spawn): pass CWD to tmux split-window via -c flag
@namastex888
namastex888 merged commit 2b8f17f into main Mar 14, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant