Roadmap lanes follow wish status - #2751
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds task-to-wish linking, JSON board wish-status reconciliation, read-only MCP integrity coverage, and coordinated package and plugin version updates. ChangesTask-to-wish linking
Board wish-status reconciliation
Manifest version alignment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Board as genie_board
participant Wishes as WISH.md
participant State as moveTask
participant SQLite as SQLite database
Board->>Wishes: Read selected task status
Wishes-->>Board: Return status or read miss
Board->>State: Move task for recognized status
State->>SQLite: Persist lane and sync event
SQLite-->>Board: Return updated board state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/v5/task-state.ts`:
- Around line 524-532: Update the transaction flow surrounding link.immediate()
so getTask(db, taskId) executes and its result is captured inside the immediate
transaction before the write lock is released. Return that captured task after
link.immediate(), ensuring handleLink receives the association written by this
call rather than a concurrent update.
In `@src/term-commands/v5-board.ts`:
- Around line 145-172: Update laneForWishStatus to recognize the FIX-FIRST
status prefix and map it to the Review lane, consistent with
returned-from-review semantics. Add or update the corresponding FIX-FIRST
mapping case in the laneForWishStatus tests.
- Around line 224-245: Update reconcileWishLanes to create a per-invocation Map
keyed by wish slug before iterating boards, and reuse each cached result when
resolving readWishStatus. Populate the cache only on the first encounter for
each slug, including null or missing statuses, so repeated tasks and boards
avoid duplicate reads while preserving one consistent status throughout the
reconciliation pass.
🪄 Autofix
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 Plus
Run ID: e69b8bd2-c7e8-4398-ba43-6935da8fa5d6
📒 Files selected for processing (14)
.claude-plugin/marketplace.jsonpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/.codex-plugin/plugin.jsonplugins/genie/package.jsonplugins/hermes-genie/plugin.yamlplugins/pi-genie/package.jsonsrc/lib/v5/task-state.test.tssrc/lib/v5/task-state.tssrc/term-commands/mcp.test.tssrc/term-commands/v5-board.test.tssrc/term-commands/v5-board.tssrc/term-commands/v5-task.test.tssrc/term-commands/v5-task.ts
| const link = db.transaction(() => { | ||
| requireTask(db, taskId); | ||
| db.query( | ||
| `UPDATE tasks SET wish = ?, group_name = ?, updated_at = ? | ||
| WHERE id = ? AND (wish IS NOT ? OR group_name IS NOT ?)`, | ||
| ).run(wish, normalizedGroup, now, taskId, wish, normalizedGroup); | ||
| }); | ||
| link.immediate(); | ||
| return getTask(db, taskId) as TaskRow; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Return the linked task inside the transaction.
Line 532 runs after link.immediate() releases the write lock. A concurrent linkTaskToWish call can update the same task in that gap. This call can then return the other call's wish and group, and handleLink can report an association that this command did not write.
Read and return the task inside the immediate transaction.
Proposed fix
const normalizedGroup = group ?? null;
const link = db.transaction(() => {
requireTask(db, taskId);
db.query(
`UPDATE tasks SET wish = ?, group_name = ?, updated_at = ?
WHERE id = ? AND (wish IS NOT ? OR group_name IS NOT ?)`,
).run(wish, normalizedGroup, now, taskId, wish, normalizedGroup);
+ return getTask(db, taskId) as TaskRow;
});
- link.immediate();
- return getTask(db, taskId) as TaskRow;
+ return link.immediate();
}📝 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.
| const link = db.transaction(() => { | |
| requireTask(db, taskId); | |
| db.query( | |
| `UPDATE tasks SET wish = ?, group_name = ?, updated_at = ? | |
| WHERE id = ? AND (wish IS NOT ? OR group_name IS NOT ?)`, | |
| ).run(wish, normalizedGroup, now, taskId, wish, normalizedGroup); | |
| }); | |
| link.immediate(); | |
| return getTask(db, taskId) as TaskRow; | |
| const link = db.transaction(() => { | |
| requireTask(db, taskId); | |
| db.query( | |
| `UPDATE tasks SET wish = ?, group_name = ?, updated_at = ? | |
| WHERE id = ? AND (wish IS NOT ? OR group_name IS NOT ?)`, | |
| ).run(wish, normalizedGroup, now, taskId, wish, normalizedGroup); | |
| return getTask(db, taskId) as TaskRow; | |
| }); | |
| return link.immediate(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/v5/task-state.ts` around lines 524 - 532, Update the transaction flow
surrounding link.immediate() so getTask(db, taskId) executes and its result is
captured inside the immediate transaction before the write lock is released.
Return that captured task after link.immediate(), ensuring handleLink receives
the association written by this call rather than a concurrent update.
| function laneForWishStatus(status: string): WishLane | null { | ||
| const key = status.toUpperCase(); | ||
| if (key.startsWith('DRAFT') || key.startsWith('ROADMAP')) return 'Idea'; | ||
| if (key.startsWith('BLOCK') || key.startsWith('ON-HOLD')) return 'Work'; | ||
| if (key.startsWith('EXECUTED') || key.startsWith('REVIEWED') || key.startsWith('PLAN-REVIEWED')) return 'Review'; | ||
| if (key.startsWith('IN') || key.startsWith('EXECUT') || key.startsWith('WAVE')) return 'Work'; | ||
| if ( | ||
| key.startsWith('READY') || | ||
| key.startsWith('APPROVED') || | ||
| key.startsWith('PLAN-') || | ||
| key.startsWith('SHIP-') || | ||
| key.startsWith('STAGED') | ||
| ) { | ||
| return 'Wish'; | ||
| } | ||
| if ( | ||
| key.startsWith('DONE') || | ||
| key.startsWith('SHIP') || | ||
| key.startsWith('MERGED') || | ||
| key.startsWith('COMPLET') || | ||
| key.startsWith('DELIVER') || | ||
| key.startsWith('PUBLISH') || | ||
| key.startsWith('CONCLU') | ||
| ) { | ||
| return 'Done'; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
FIX-FIRST is a durable wish status but maps to no lane.
The coding guidelines list FIX-FIRST as a durable wish status. laneForWishStatus matches no prefix for it, so it returns null and the card never reconciles. FIX-FIRST means a wish returned from review, so Review is the consistent destination. Add the prefix, or state in the docstring that FIX-FIRST stays hand-owned on purpose.
As per coding guidelines: "durable statuses are DRAFT, FIX-FIRST, APPROVED, IN_PROGRESS, BLOCKED, and SHIPPED."
🐛 Proposed mapping for `FIX-FIRST`
- if (key.startsWith('EXECUTED') || key.startsWith('REVIEWED') || key.startsWith('PLAN-REVIEWED')) return 'Review';
+ if (
+ key.startsWith('EXECUTED') ||
+ key.startsWith('REVIEWED') ||
+ key.startsWith('PLAN-REVIEWED') ||
+ key.startsWith('FIX-FIRST')
+ ) {
+ return 'Review';
+ }Add the matching case to the prefix table in src/term-commands/v5-board.test.ts as well.
📝 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.
| function laneForWishStatus(status: string): WishLane | null { | |
| const key = status.toUpperCase(); | |
| if (key.startsWith('DRAFT') || key.startsWith('ROADMAP')) return 'Idea'; | |
| if (key.startsWith('BLOCK') || key.startsWith('ON-HOLD')) return 'Work'; | |
| if (key.startsWith('EXECUTED') || key.startsWith('REVIEWED') || key.startsWith('PLAN-REVIEWED')) return 'Review'; | |
| if (key.startsWith('IN') || key.startsWith('EXECUT') || key.startsWith('WAVE')) return 'Work'; | |
| if ( | |
| key.startsWith('READY') || | |
| key.startsWith('APPROVED') || | |
| key.startsWith('PLAN-') || | |
| key.startsWith('SHIP-') || | |
| key.startsWith('STAGED') | |
| ) { | |
| return 'Wish'; | |
| } | |
| if ( | |
| key.startsWith('DONE') || | |
| key.startsWith('SHIP') || | |
| key.startsWith('MERGED') || | |
| key.startsWith('COMPLET') || | |
| key.startsWith('DELIVER') || | |
| key.startsWith('PUBLISH') || | |
| key.startsWith('CONCLU') | |
| ) { | |
| return 'Done'; | |
| } | |
| return null; | |
| } | |
| function laneForWishStatus(status: string): WishLane | null { | |
| const key = status.toUpperCase(); | |
| if (key.startsWith('DRAFT') || key.startsWith('ROADMAP')) return 'Idea'; | |
| if (key.startsWith('BLOCK') || key.startsWith('ON-HOLD')) return 'Work'; | |
| if ( | |
| key.startsWith('EXECUTED') || | |
| key.startsWith('REVIEWED') || | |
| key.startsWith('PLAN-REVIEWED') || | |
| key.startsWith('FIX-FIRST') | |
| ) { | |
| return 'Review'; | |
| } | |
| if (key.startsWith('IN') || key.startsWith('EXECUT') || key.startsWith('WAVE')) return 'Work'; | |
| if ( | |
| key.startsWith('READY') || | |
| key.startsWith('APPROVED') || | |
| key.startsWith('PLAN-') || | |
| key.startsWith('SHIP-') || | |
| key.startsWith('STAGED') | |
| ) { | |
| return 'Wish'; | |
| } | |
| if ( | |
| key.startsWith('DONE') || | |
| key.startsWith('SHIP') || | |
| key.startsWith('MERGED') || | |
| key.startsWith('COMPLET') || | |
| key.startsWith('DELIVER') || | |
| key.startsWith('PUBLISH') || | |
| key.startsWith('CONCLU') | |
| ) { | |
| return 'Done'; | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/term-commands/v5-board.ts` around lines 145 - 172, Update
laneForWishStatus to recognize the FIX-FIRST status prefix and map it to the
Review lane, consistent with returned-from-review semantics. Add or update the
corresponding FIX-FIRST mapping case in the laneForWishStatus tests.
Source: Coding guidelines
| function reconcileWishLanes(db: Database, filter: TaskFilter, selectedBoard: BoardRow | null): void { | ||
| const repoRoot = resolveRepoRoot(); | ||
| const boards = selectedBoard ? [selectedBoard] : listBoards(db); | ||
| for (const board of boards) { | ||
| const lanes = board.lanes; | ||
| if (!lanes || lanes.length === 0) continue; | ||
| const laneNames = new Set(lanes.map((lane) => lane.name)); | ||
| const enclosingLane = lanes[0].name; | ||
| const boardFilter: TaskFilter = { ...filter, boardId: board.id }; | ||
| for (const task of listTasksWithLane(db, boardFilter)) { | ||
| if (!task.wish) continue; | ||
| const status = readWishStatus(repoRoot, task.wish); | ||
| const destination = status ? laneForWishStatus(status) : null; | ||
| if (!destination || !laneNames.has(destination)) continue; | ||
| const currentLane = task.lane ?? enclosingLane; | ||
| if (currentLane === destination) continue; | ||
| try { | ||
| moveTask(db, task.id, destination, { author: 'wish-status-sync', authorKind: 'genie' }); | ||
| } catch { | ||
| // Best-effort reconciliation: render the durable lane that remains. | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Cache wish status per slug for the duration of one reconciliation pass.
readWishStatus runs once per task. Tasks that belong to the same wish repeat the same three lstatSync calls plus an open and a full read of the same WISH.md. When no board is selected, the loop also repeats across boards. A local Map keyed by slug removes the duplicate I/O and keeps one consistent status for the whole pass.
♻️ Proposed per-pass cache
function reconcileWishLanes(db: Database, filter: TaskFilter, selectedBoard: BoardRow | null): void {
const repoRoot = resolveRepoRoot();
const boards = selectedBoard ? [selectedBoard] : listBoards(db);
+ // One filesystem read per slug: a wish commonly owns many cards.
+ const statusBySlug = new Map<string, string | null>();
+ const statusFor = (wish: string): string | null => {
+ if (!statusBySlug.has(wish)) statusBySlug.set(wish, readWishStatus(repoRoot, wish));
+ return statusBySlug.get(wish) ?? null;
+ };
for (const board of boards) {
const lanes = board.lanes;
if (!lanes || lanes.length === 0) continue;
const laneNames = new Set(lanes.map((lane) => lane.name));
const enclosingLane = lanes[0].name;
const boardFilter: TaskFilter = { ...filter, boardId: board.id };
for (const task of listTasksWithLane(db, boardFilter)) {
if (!task.wish) continue;
- const status = readWishStatus(repoRoot, task.wish);
+ const status = statusFor(task.wish);
const destination = status ? laneForWishStatus(status) : null;📝 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.
| function reconcileWishLanes(db: Database, filter: TaskFilter, selectedBoard: BoardRow | null): void { | |
| const repoRoot = resolveRepoRoot(); | |
| const boards = selectedBoard ? [selectedBoard] : listBoards(db); | |
| for (const board of boards) { | |
| const lanes = board.lanes; | |
| if (!lanes || lanes.length === 0) continue; | |
| const laneNames = new Set(lanes.map((lane) => lane.name)); | |
| const enclosingLane = lanes[0].name; | |
| const boardFilter: TaskFilter = { ...filter, boardId: board.id }; | |
| for (const task of listTasksWithLane(db, boardFilter)) { | |
| if (!task.wish) continue; | |
| const status = readWishStatus(repoRoot, task.wish); | |
| const destination = status ? laneForWishStatus(status) : null; | |
| if (!destination || !laneNames.has(destination)) continue; | |
| const currentLane = task.lane ?? enclosingLane; | |
| if (currentLane === destination) continue; | |
| try { | |
| moveTask(db, task.id, destination, { author: 'wish-status-sync', authorKind: 'genie' }); | |
| } catch { | |
| // Best-effort reconciliation: render the durable lane that remains. | |
| } | |
| } | |
| function reconcileWishLanes(db: Database, filter: TaskFilter, selectedBoard: BoardRow | null): void { | |
| const repoRoot = resolveRepoRoot(); | |
| const boards = selectedBoard ? [selectedBoard] : listBoards(db); | |
| // One filesystem read per slug: a wish commonly owns many cards. | |
| const statusBySlug = new Map<string, string | null>(); | |
| const statusFor = (wish: string): string | null => { | |
| if (!statusBySlug.has(wish)) statusBySlug.set(wish, readWishStatus(repoRoot, wish)); | |
| return statusBySlug.get(wish) ?? null; | |
| }; | |
| for (const board of boards) { | |
| const lanes = board.lanes; | |
| if (!lanes || lanes.length === 0) continue; | |
| const laneNames = new Set(lanes.map((lane) => lane.name)); | |
| const enclosingLane = lanes[0].name; | |
| const boardFilter: TaskFilter = { ...filter, boardId: board.id }; | |
| for (const task of listTasksWithLane(db, boardFilter)) { | |
| if (!task.wish) continue; | |
| const status = statusFor(task.wish); | |
| const destination = status ? laneForWishStatus(status) : null; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/term-commands/v5-board.ts` around lines 224 - 245, Update
reconcileWishLanes to create a per-invocation Map keyed by wish slug before
iterating boards, and reuse each cached result when resolving readWishStatus.
Populate the cache only on the first encounter for each slug, including null or
missing statuses, so repeated tasks and boards avoid duplicate reads while
preserving one consistent status throughout the reconciliation pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bf066eff1
ℹ️ 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".
| // Deliberately CLI-only. MCP queries call their shared read projection and | ||
| // never enter this verb handler, so they remain read-only. | ||
| if (opts.json) reconcileWishLanes(db, filter, board); |
There was a problem hiding this comment.
Keep JSON board reads non-mutating for plugin callers
When Pi's genie_board tool, Hermes's legacy genie_board/genie_wish_status tools, or the Hermes session-context hook invokes genie board --json, this unconditional reconciliation calls moveTask and writes lanes plus timeline events. Those callers explicitly advertise themselves as read-only (and the context hook can run automatically), so merely inspecting status can silently undo manual lane placement; reconciliation needs an explicit mutating surface or flag while ordinary --json remains read-only.
Useful? React with 👍 / 👎.
| function laneForWishStatus(status: string): WishLane | null { | ||
| const key = status.toUpperCase(); | ||
| if (key.startsWith('DRAFT') || key.startsWith('ROADMAP')) return 'Idea'; | ||
| if (key.startsWith('BLOCK') || key.startsWith('ON-HOLD')) return 'Work'; | ||
| if (key.startsWith('EXECUTED') || key.startsWith('REVIEWED') || key.startsWith('PLAN-REVIEWED')) return 'Review'; | ||
| if (key.startsWith('IN') || key.startsWith('EXECUT') || key.startsWith('WAVE')) return 'Work'; |
There was a problem hiding this comment.
Map the canonical FIX-FIRST status to its lifecycle lane
FIX-FIRST is a canonical persisted WISH status (scripts/wishes-lint.ts) for a plan that must be fixed and re-reviewed, but none of these prefixes recognizes it. Consequently, after a linked wish transitions to FIX-FIRST, every JSON reconciliation leaves its card in the previous lane, defeating the new status-following behavior for a normal lifecycle transition; handle FIX-FIRST explicitly in the plan-stage lane and cover it in the mapping test.
Useful? React with 👍 / 👎.
deaf4ce to
40ad3d0
Compare
… its branch The wish documents were authored in the remotty repo by the originating session and recovered on 2026-08-06. The ledger carries the 2026-08-05 execution record with a dated amendment (promote reverted), the FIX-FIRST post-relocation execution review, and the fix record resolving it. Wish: roadmap-truth
Fix round 1 closed all eight FIX-FIRST gaps; deferred live oracles are owned by follow-up task t_msi2nv2gcf9c97af. Wish: roadmap-truth
Summary
Validation
Summary by CodeRabbit
New Features
task linkto associate existing tasks with wishes and optional groups.Bug Fixes
Chores
5.260805.1.Review disclosures (2026-08-06 post-relocation execution re-review)
An independent execution re-review (FIX-FIRST, resolved — full ledger in
.genie/wishes/roadmap-truth/WISH.mdon this branch) surfaced items this body must declare:.well-knownchannel manifests are edited by40ad3d050(dev.json,latest.json). Net-nil: both are pinned toorigin/main's exact values so this branch's tree matches main's release surface. They were outside the wish's file list, hence declared here.dev@6d252e65c, notorigin/mainas the wish's Group 0 guard wrote. Benign — the tree differs fromorigin/mainonly in the.well-knownfiles above, which the tip commit pins to main's values.5.260805.1(dev is behind at5.260803.6); the merge moves forward and[auto-version]CI owns the final stamp.21350f7f6commits the wish plan, design, and full review ledger onto this branch.