chore: rolling promotion dev -> main - #625
Conversation
genie work auto-initializes state, so checking genie status before the first dispatch is a wasted step that errors. Added explicit instruction: "do NOT run genie status before your first dispatch."
…1, 2) parseWishGroups() regex was \d+ (digits only), so wishes with Group A / Group B were parsed as 0 groups. Changed to [A-Za-z0-9]+ to accept both styles. Also fixed the error message group listing.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPatch version bump to 3.260317.3 across manifests and packages; docs update to clarify genie auto-initialization and status timing; dispatch parsing extended to accept alphanumeric group IDs; team-manager worker termination scoped by team name. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Summary of ChangesHello, 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 a routine rolling promotion from the Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a rolling promotion from dev to main. The main functional change appears to be an update to allow alphanumeric group identifiers in WISH.md files, alongside several version bumps and documentation updates. While the intent to support more flexible group IDs is good, the implementation in src/term-commands/dispatch.ts has a couple of issues related to case-sensitivity in regular expressions. I've added comments with suggestions to fix these bugs, which could otherwise lead to incorrect parsing and confusing error messages.
| // 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 [A-Za-z0-9]+:/m); |
There was a problem hiding this comment.
The regex used to find the next group heading is missing the case-insensitive (i) flag. The main groupPattern at line 148 is case-insensitive, so this can lead to incorrect parsing of group sections if group headings in WISH.md have mixed casing (e.g., ### Group 1 followed by ### group 2). This will cause the parser to treat the content of all subsequent groups as part of the first group's section.
| const nextGroupIdx = rest.search(/^### Group [A-Za-z0-9]+:/m); | |
| const nextGroupIdx = rest.search(/^### Group [A-Za-z0-9]+:/im); |
| console.error(`❌ Group "${group}" not found in ${wishPath}`); | ||
| console.error(' Available groups:'); | ||
| const groups = content.match(/^### Group \d+:.*$/gm); | ||
| const groups = content.match(/^### Group [A-Za-z0-9]+:.*$/gm); |
There was a problem hiding this comment.
The regex used to find available groups for the error message is missing the case-insensitive (i) flag. This means if a WISH.md file uses lowercase group headings (e.g., ### group 1: ...), they won't be listed as available groups in the error message when a group is not found, which could be confusing for the user.
| const groups = content.match(/^### Group [A-Za-z0-9]+:.*$/gm); | |
| const groups = content.match(/^### Group [A-Za-z0-9]+:.*$/gim); |
![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: f142f0c956
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function parseWishGroups(content: string): GroupDefinition[] { | ||
| const groups: GroupDefinition[] = []; | ||
| const groupPattern = /^### Group (\d+):/gim; | ||
| const groupPattern = /^### Group ([A-Za-z0-9]+):/gim; |
There was a problem hiding this comment.
By broadening parseWishGroups to accept alphanumeric group IDs, this commit makes lettered groups (Group A, Group B, etc.) look supported, but extractGroup still searches for the next heading with ^### Group \d and therefore cannot stop at the next lettered heading. In WISH files that omit --- separators between groups, dispatching #A will include Group B and later sections in the assigned context, which can misroute implementation/review work for that group.
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: 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 `@plugins/genie/agents/team-lead.md`:
- Line 52: Update the contradictory guidance in team-lead.md so the heartbeat
wording matches the status-timing fix: change the unconditional instruction
about running `genie status` in the heartbeat section to state that `genie work`
auto-initializes state on first call and you should NOT run `genie status`
before the first dispatch; ensure both occurrences referencing heartbeat/status
timing (the paragraph that starts "Dispatch groups whose dependencies are
satisfied..." and the other similar line) are edited to mirror this behavior and
remove any implication of an unconditional status check before the first
dispatch.
In `@plugins/genie/agents/team-lead/AGENTS.md`:
- Line 52: Update the heartbeat checklist so it no longer instructs running
`genie status` on every loop before `genie work` has auto-initialized state;
instead, make the heartbeat description conditional or explicit: skip the `genie
status` step until after the first dispatch/initialization, or change wording to
run `genie status` only after `genie work` has been invoked at least once.
Locate the “heartbeat checklist” section and the lines referencing `genie work`
/ `genie status` (the dispatch guidance) and edit them to remove the
contradictory pre-dispatch `genie status` instruction so it matches the “do NOT
run `genie status` before your first dispatch” guidance.
In `@src/term-commands/dispatch.ts`:
- Line 148: parseWishGroups was updated to accept alphanumeric IDs but
extractGroup still looks for the next numeric-only heading, causing sections to
bleed; update the heading-matching regex used in extractGroup (and any other
place using /^### Group \d/) to the same alphanumeric pattern used above (e.g.,
/^### Group ([A-Za-z0-9]+):/i) and ensure the extraction boundary logic stops at
that next alphanumeric group heading or EOF so group slices align with
parseWishGroups; reference functions: parseWishGroups and extractGroup and the
group heading regex variable/groupPattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b0c7761c-8eff-4ea6-a563-5f58dcbc9b48
📒 Files selected for processing (8)
.claude-plugin/marketplace.jsonopenclaw.plugin.jsonpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/agents/team-lead.mdplugins/genie/agents/team-lead/AGENTS.mdplugins/genie/package.jsonsrc/term-commands/dispatch.ts
|
|
||
| ## Phase 2 — Execute Groups | ||
| Dispatch groups whose dependencies are satisfied. Run independent groups in parallel. Never start a group before its dependencies complete. | ||
| Dispatch groups whose dependencies are satisfied. `genie work` auto-initializes state on first call — do NOT run `genie status` before your first dispatch. Just dispatch immediately. |
There was a problem hiding this comment.
Mirror the same status-timing fix here to avoid first-loop failures.
This file has the same contradiction: “do not run status before first dispatch” vs heartbeat’s unconditional status check. Update heartbeat wording here as well.
Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/genie/agents/team-lead.md` at line 52, Update the contradictory
guidance in team-lead.md so the heartbeat wording matches the status-timing fix:
change the unconditional instruction about running `genie status` in the
heartbeat section to state that `genie work` auto-initializes state on first
call and you should NOT run `genie status` before the first dispatch; ensure
both occurrences referencing heartbeat/status timing (the paragraph that starts
"Dispatch groups whose dependencies are satisfied..." and the other similar
line) are edited to mirror this behavior and remove any implication of an
unconditional status check before the first dispatch.
|
|
||
| ## Phase 2 — Execute Groups | ||
| Dispatch groups whose dependencies are satisfied. Run independent groups in parallel. Never start a group before its dependencies complete. | ||
| Dispatch groups whose dependencies are satisfied. `genie work` auto-initializes state on first call — do NOT run `genie status` before your first dispatch. Just dispatch immediately. |
There was a problem hiding this comment.
Status timing guidance conflicts with the heartbeat checklist.
These lines correctly say not to run genie status before first dispatch, but the heartbeat still instructs status every loop. That contradiction can cause immediate pre-dispatch failure.
Suggested fix
-2. **Wish status** — `genie status <slug>` — which groups are done, in-progress, or blocked?
+2. **Wish status** — after first dispatch, run `genie status <slug>` — which groups are done, in-progress, or blocked?Also applies to: 58-58
🤖 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 52, Update the heartbeat
checklist so it no longer instructs running `genie status` on every loop before
`genie work` has auto-initialized state; instead, make the heartbeat description
conditional or explicit: skip the `genie status` step until after the first
dispatch/initialization, or change wording to run `genie status` only after
`genie work` has been invoked at least once. Locate the “heartbeat checklist”
section and the lines referencing `genie work` / `genie status` (the dispatch
guidance) and edit them to remove the contradictory pre-dispatch `genie status`
instruction so it matches the “do NOT run `genie status` before your first
dispatch” guidance.
| export function parseWishGroups(content: string): GroupDefinition[] { | ||
| const groups: GroupDefinition[] = []; | ||
| const groupPattern = /^### Group (\d+):/gim; | ||
| const groupPattern = /^### Group ([A-Za-z0-9]+):/gim; |
There was a problem hiding this comment.
Alphanumeric group support is incomplete without updating extraction boundaries.
parseWishGroups now accepts alphanumeric IDs, but extractGroup still finds the next heading with numeric-only matching (^### Group \d). For lettered groups, this can cause section extraction to bleed into later groups and dispatch incorrect scope.
Suggested fix
- const nextBoundary = afterHeading.slice(1).search(/^### Group \d|^---$/m);
+ const nextBoundary = afterHeading.slice(1).search(/^### Group [A-Za-z0-9]+:|^---$/m);Also applies to: 157-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/term-commands/dispatch.ts` at line 148, parseWishGroups was updated to
accept alphanumeric IDs but extractGroup still looks for the next numeric-only
heading, causing sections to bleed; update the heading-matching regex used in
extractGroup (and any other place using /^### Group \d/) to the same
alphanumeric pattern used above (e.g., /^### Group ([A-Za-z0-9]+):/i) and ensure
the extraction boundary logic stops at that next alphanumeric group heading or
EOF so group slices align with parseWishGroups; reference functions:
parseWishGroups and extractGroup and the group heading regex
variable/groupPattern.
…workers killWorkersByName filtered by role name only (e.g., "engineer"), which is shared across all teams. When one team called genie team done, it killed engineers from ALL teams. Now accepts a teamName parameter and filters by both role AND team. Closes #626
fix: scope killWorkersByName by team — prevents killing other teams' workers
1 participant
Rolling Promotion PR
Auto-maintained rolling promotion PR from
devtomain.Process:
ready-to-mergeadded when all checks passSummary by CodeRabbit
New Features
Documentation
Chores