chore: promote dev → main (genie-mcp + omni approval UX) - #2510
Conversation
…aude Code consume genie state Richer Warp integration, grounded correctly: Warp's plugin surface IS MCP. A genie mcp stdio server exposes genie.db read-only (board, wish_status, worktree_context by branch, task, active), auto-registered via project .warp/.mcp.json + .mcp.json by init/launch. Spike-first on hand-rolled-vs-SDK + the real Warp config schema. Codex deferred (TOML, not .mcp.json); read-only open is net-new; write tools later. Design reviewed and validated (WRS 100/100).
…sume genie state via MCP genie mcp stdio server exposes genie.db read-only (board, wish_status, worktree_context-by-branch, task, active), auto-registered into project .warp/.mcp.json + .mcp.json by init/launch. Spike-first (hand-rolled MCP vs SDK + real Warp/CC config schema). Read-only open is net-new; Codex deferred; write tools later. Plan reviewed: SHIP (validation blocks fixed).
hand-rolled stdio server satisfies real claude code; contract pinned for g2
Group 3 of genie-mcp. genie init (and launch per worktree) JSON-merge-register the genie mcp server into project .mcp.json + .warp/.mcp.json under mcpServers: preserve all other servers + top-level keys, byte-identical rerun, malformed- JSON-safe (throws, no clobber), alt-wrapper-key aware. Command uses the absolute process.execPath (correct for the shipped compiled binary; bare genie is not on PATH). README documents the pull-not-push tab-info limitation. Reviewed: SHIP. 14 init tests, 21 launch tests.
Group 2 of genie-mcp. Hand-rolled newline-delimited JSON-RPC 2.0 stdio server (no MCP SDK, genie stays 4-deps-lean) exposing genie.db READ-ONLY via 5 tools: genie_board, genie_wish_status, genie_worktree_context (resolves branch wish/<slug>-<group>), genie_task, genie_active. Net-new new Database(readonly) open (not openSqlite) degrades to empty board on absent db; mcp-tools is await-import'd so non-mcp paths never load bun:sqlite (graph-walk probe). mcp added to WORKSPACE_EXEMPT (v5 self-resolving, like task/board/launch/omni). Reviewed: SHIP. 14 mcp tests, 599 full suite, dist protocol verified.
feat: genie mcp server — Warp + Claude Code consume genie state
Rebased onto current dev (was based on a pre-omni-hardening commit). Wish:
correlated identity, reaction approve/deny, two-state ⏳→✅ acks; sequential
G1->G2->G3. SPIKE.md (0 live msgs, source-proven): reactions arrive on
omni.message.{instance}.{chatId} (omni.event.> has zero publishers), send
returns the stanza id, outbound set-reaction GO, text quoted-id unavailable
(bare text stays oldest fallback).
Group 2 of omni-approval-ux. announce() now sends via an injectable OmniSend seam (default: signed POST to omni /api/v2/messages) and stores the REAL WhatsApp stanza id via attachOmniMessageId — retiring the self-referential genId() ref that matched nothing inbound. Inbound reactions parse off omni.message.* as [Reaction: <emoji> on message <id>], correlate 👍/👎 to the exact approval by omni_message_id (oldest only when no id), and the dual-emit bare-emoji echo drops structurally (reaction-vs-text split). Retired the dead omni.event.> path/handleEvent. Bare text stays oldest fallback; PR#2507 instance-scope guard kept. Reviewed: SHIP. 28 runner/queue tests + typecheck.
Group 3 (final) of omni-approval-ux. genie sets ⏳ on the approval message the moment it's sent (the G2 stanza id), swapping in place to ✅ (approved) / ❌ (denied/expired) — via an injectable OmniSetReaction seam (default: signed omni --reaction POST; fallback message-edit/status-reply is a one-seam swap). A tick reconciliation pass (reconcileStatusAcks) makes the runner the authoritative acker regardless of which process expired the row, curing a stuck-⏳ hook-fork race AND transport-dropped swaps. Reactions targeting an unknown id no-op (not resolveOldest). Adds genie omni test-approval (fake default, --live) + a doctor hook-timeout guardrail. New last_status_glyph column (additive, no user_version bump; one-time migration backfill + 24h recency cap so upgrade never sweeps history). Reviewed: SHIP (2 HIGHs found+fixed). 77 omni tests.
feat(omni): approval UX — correlated identity, reactions, ⏳→✅ acks
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a read-only ChangesGenie MCP Server
Estimated code review effort: 4 (Complex) | ~60 minutes Omni Approval UX
Estimated code review effort: 4 (Complex) | ~55 minutes Version Bumps
Estimated code review effort: 1 (Trivial) | ~2 minutes Sequence Diagram(s)sequenceDiagram
participant Client as Warp/Claude Code
participant MCPServer as genie mcp
participant MCPTools as mcp-tools.ts
participant DB as genie.db
Client->>MCPServer: initialize
MCPServer-->>Client: protocolVersion + capabilities
Client->>MCPServer: tools/call genie_board
MCPServer->>MCPTools: dynamic import + dispatch
MCPTools->>DB: openReadonlyDb()
DB-->>MCPTools: rows or null
MCPTools-->>MCPServer: counts + task summaries
MCPServer-->>Client: tool result payload
sequenceDiagram
participant User as Omni user
participant Runner as omni-runner
participant Queue as omni-queue
participant Transport as sendApproval / setReaction
Runner->>Transport: sendApproval()
Transport-->>Runner: real stanza id
Runner->>Queue: enqueueApproval(omni_message_id)
User->>Runner: reaction on omni.message.*
Runner->>Queue: recordStatusGlyph()
Runner->>Transport: setReaction(✅ / ❌)
Queue->>Queue: listApprovalsNeedingStatusAck()
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a read-only, zero-dependency stdio MCP server (genie mcp) that integrates with Warp and Claude Code to expose board and wish state. It also enhances the Omni approval UX by correlating approval identity with real message IDs, supporting reaction-based decisions, and implementing a two-state status-reaction lifecycle (⏳→✅/❌). Additionally, a new test-approval command and a doctor timeout guardrail are added. A critical issue was identified in the MCP server's line handler where parsing non-object JSON (such as null) can lead to an unhandled TypeError and crash the server process.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function handleLine(line: string): void { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed) return; | ||
| let req: JsonRpcRequest; | ||
| try { | ||
| req = JSON.parse(trimmed) as JsonRpcRequest; | ||
| } catch { | ||
| // Unparseable line → cannot attribute an id; drop it (no id to reply to). | ||
| return; | ||
| } | ||
| try { | ||
| dispatch(req); | ||
| } catch (e) { | ||
| const id = req.id ?? null; | ||
| if (id !== null) err(id, INTERNAL_ERROR, e instanceof Error ? e.message : String(e)); | ||
| } | ||
| } |
There was a problem hiding this comment.
If JSON.parse(trimmed) returns null (which is valid JSON for the string "null"), req will be null. In this case, dispatch(req) will throw a TypeError when trying to access req.id. The catch (e) block will then attempt to access req.id again to extract the ID, which throws another TypeError that is unhandled, crashing the entire MCP server process. To prevent this, validate that req is a non-null object before dispatching.
| function handleLine(line: string): void { | |
| const trimmed = line.trim(); | |
| if (!trimmed) return; | |
| let req: JsonRpcRequest; | |
| try { | |
| req = JSON.parse(trimmed) as JsonRpcRequest; | |
| } catch { | |
| // Unparseable line → cannot attribute an id; drop it (no id to reply to). | |
| return; | |
| } | |
| try { | |
| dispatch(req); | |
| } catch (e) { | |
| const id = req.id ?? null; | |
| if (id !== null) err(id, INTERNAL_ERROR, e instanceof Error ? e.message : String(e)); | |
| } | |
| } | |
| function handleLine(line: string): void { | |
| const trimmed = line.trim(); | |
| if (!trimmed) return; | |
| let req: JsonRpcRequest; | |
| try { | |
| req = JSON.parse(trimmed) as JsonRpcRequest; | |
| if (typeof req !== 'object' || req === null || Array.isArray(req)) { | |
| return; | |
| } | |
| } catch { | |
| // Unparseable line → cannot attribute an id; drop it (no id to reply to). | |
| return; | |
| } | |
| try { | |
| dispatch(req); | |
| } catch (e) { | |
| const id = req.id ?? null; | |
| if (id !== null) err(id, INTERNAL_ERROR, e instanceof Error ? e.message : String(e)); | |
| } | |
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb7cd7cc26
ℹ️ 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".
| const json = (await res.json()) as { messageId?: string; data?: { messageId?: string } }; | ||
| const messageId = json.messageId ?? json.data?.messageId; | ||
| return { success: Boolean(messageId), messageId }; |
There was a problem hiding this comment.
Store the external stanza id from live Omni sends
In the live path this posts to /api/v2/messages and then stores messageId, but the spike added in this commit documents that the persisted REST route returns the Omni row UUID while inbound reactions reference the WhatsApp stanza/external id (.genie/wishes/omni-approval-ux/SPIKE.md lines 64-68). With real Omni configured, attachOmniMessageId will therefore persist an id that resolveReaction() never matches, so 👍/👎 reactions on approval messages are ignored even though the fake transport tests pass; use an id-returning channel send or extract/translate the external id before returning it here.
Useful? React with 👍 / 👎.
| const dash = rest.lastIndexOf('-'); | ||
| if (dash <= 0 || dash === rest.length - 1) return null; | ||
| return { wish: rest.slice(0, dash), group: rest.slice(dash + 1) }; |
There was a problem hiding this comment.
Resolve worktree branches with hyphenated group names
This split assumes the group is the final - segment, but genie launch explicitly allows hyphens in group names (GROUP_NAME_PATTERN includes -) and creates branches as wish/<slug>-<group>. For a valid launched group such as wish ui-refactor / group api-tests, genie_worktree_context parses wish/ui-refactor-api-tests as wish ui-refactor-api and group tests, so the pane's MCP context returns no matching tasks for its actual group; resolve against known wish/group rows or otherwise encode the separator unambiguously.
Useful? React with 👍 / 👎.
| const db = openReadonlyDb(cwd); | ||
| const ctx: ToolContext = { db, cwd }; |
There was a problem hiding this comment.
Reopen the MCP database after a late first write
When the MCP server is started in a freshly initialized repo before .genie/genie.db exists, this captures ctx.db as null for the lifetime of the stdio server. If the same Claude Code/Warp session then creates the first wish or task, every tool keeps returning the empty-board fallback until the MCP process is restarted, which breaks the advertised live board state in the common init-first workflow; retry opening when ctx.db is null or open per tool call.
Useful? React with 👍 / 👎.
… the server
Gemini PR review (HIGH): JSON.parse('null') is valid JSON returning null, so
dispatch(null) throws on null.id (mcp.ts:105) and the catch handler's own
req.id throws again → uncaught → the stdio server crashes on a single 'null'
line. Guard: after parse, drop any non-object (null/primitive) like an
unparseable line — it carries no id to attribute. Regression test drives raw
null/primitive lines and asserts the server survives + answers initialize.
fix(mcp): a bare 'null' JSON-RPC line must not crash the server
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 @.genie/INDEX.md:
- Line 17: The genie-mcp index entry overstates the validated scope by
mentioning Codex and auto-serving behavior that was not part of the spike.
Update the entry in INDEX.md so it only reflects the confirmed Warp + Claude
Code scope, and keep the wording aligned with the genie-mcp DESIGN and WISH docs
by removing any Codex or broader auto-registration claims.
In @.genie/wishes/genie-mcp/spike/server.mjs:
- Around line 34-35: The handle(msg) logic destructures id, method, and params
immediately after JSON.parse() output, which can crash on non-object decoded
values. Add a guard in handle (or right after parsing in the message-processing
path) to verify the decoded message is a non-null object before destructuring,
and return/send a protocol error for malformed input instead of throwing. Keep
the fix localized to the handle(msg) flow so invalid lines are handled safely
without taking down the spike server.
- Around line 40-45: The initialize handler in server.mjs is returning a tools
capability shape that diverges from the MCP contract used elsewhere in the repo.
Update the reply payload in the initialize case so it matches the expected
handshake shape with capabilities.tools as an empty object, and keep the change
localized to the initialize branch in the server message handler.
In `@src/genie-commands/doctor.test.ts`:
- Around line 79-123: Add a boundary test in doctor.test.ts for the exact
equality case where the hook timeout matches pollBudgetMs. Extend the existing
evaluateOmniHookTimeout coverage to assert that enabled=true with
pollBudgetMs=110_000 and timeoutSec=110 returns a warn status, since the strict
“below” contract in findDispatchHookTimeoutSec/evaluateOmniHookTimeout should
warn when timeoutMs equals pollBudgetMs. Keep the test alongside the current
omni hook-timeout guardrail cases so it clearly validates the off-by-one
behavior.
In `@src/genie-commands/doctor.ts`:
- Around line 232-258: In evaluateOmniHookTimeout, the hook timeout check is
using a non-strict comparison and incorrectly passes when timeoutMs exactly
equals pollBudgetMs; change the validation so only timeoutMs strictly greater
than pollBudgetMs is considered pass, and update the warning/suggestion logic to
require a timeout above the boundary rather than matching it. Also adjust the
needSec calculation and suggestion text so it cannot recommend an exact-boundary
value, and clean up the pass detail to compare values in the same units or
otherwise avoid mixing seconds and milliseconds.
In `@src/lib/v5/global-db.ts`:
- Around line 126-149: The approval-column migration in ensureApprovalColumns is
vulnerable to a race when two first opens both try to add last_status_glyph.
Update ensureApprovalColumns to handle the ALTER TABLE path safely by catching
and ignoring the duplicate column name case, or by serializing the PRAGMA
table_info check, ALTER TABLE add, and backfill as one locked operation so
concurrent upgrades do not throw MalformedDbError. Keep the idempotent behavior
in ensureSchema and ensureApprovalColumns intact.
In `@src/lib/v5/mcp-tools.ts`:
- Around line 144-163: The genieBoard tool’s default behavior is inconsistent
with its description because omitting board leaves the query unscoped and
returns tasks from all boards. Update genieBoard in mcp-tools.ts so it either
applies a canonical default board when boardArg is missing, or adjust the
tool/schema description to explicitly state that no board means a cross-board
list; make the behavior and docs match for boardArg, getBoardByName, and
filter.boardId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4b99242c-aebd-4ce3-ad3f-878113b4e50e
⛔ Files ignored due to path filters (1)
README.mdis excluded by!*.md
📒 Files selected for processing (32)
.claude-plugin/marketplace.json.genie/INDEX.md.genie/brainstorms/genie-mcp/DESIGN.md.genie/brainstorms/genie-mcp/DRAFT.md.genie/wishes/genie-mcp/SPIKE.md.genie/wishes/genie-mcp/WISH.md.genie/wishes/genie-mcp/spike/server.mjs.genie/wishes/omni-approval-ux/SPIKE.md.genie/wishes/omni-approval-ux/WISH.mdpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/package.jsonsrc/genie-commands/doctor.test.tssrc/genie-commands/doctor.tssrc/genie.tssrc/lib/interactivity.tssrc/lib/omni-config.tssrc/lib/omni-runner.test.tssrc/lib/omni-runner.tssrc/lib/v5/global-db.test.tssrc/lib/v5/global-db.tssrc/lib/v5/mcp-tools.tssrc/lib/v5/omni-queue.test.tssrc/lib/v5/omni-queue.tssrc/term-commands/init.test.tssrc/term-commands/init.tssrc/term-commands/launch.tssrc/term-commands/mcp.test.tssrc/term-commands/mcp.tssrc/term-commands/omni.test.tssrc/term-commands/omni.tssrc/types/genie-config.ts
| - [WISH: warp-integration](wishes/warp-integration/WISH.md) — umbrella Group 3: genie init, Warp launch-config emitter, genie launch, /work multi-session opt-in (drafted 2026-07-02) | ||
|
|
||
| ## Poured | ||
| - [genie-mcp](brainstorms/genie-mcp/DESIGN.md) · [WISH](wishes/genie-mcp/WISH.md) — genie MCP server: Warp/Claude Code/Codex consume genie.db state read-only; stdio, auto-registered; spike-first (2026-07-03) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the scope to Warp + Claude Code here.
This spike is the evidence for the release, so saying it “auto-serves Claude Code + Codex” overstates what was actually validated and conflicts with the rest of the genie-mcp docs and PR scope.
🤖 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 @.genie/INDEX.md at line 17, The genie-mcp index entry overstates the
validated scope by mentioning Codex and auto-serving behavior that was not part
of the spike. Update the entry in INDEX.md so it only reflects the confirmed
Warp + Claude Code scope, and keep the wording aligned with the genie-mcp DESIGN
and WISH docs by removing any Codex or broader auto-registration claims.
| function handle(msg) { | ||
| const { id, method, params } = msg; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the decoded message before destructuring.
JSON.parse() can yield a non-object value, and this destructure will throw instead of returning a protocol error. A single malformed line can crash the spike server.
🛠️ Proposed fix
function handle(msg) {
+ if (!msg || typeof msg !== 'object' || Array.isArray(msg)) {
+ replyError(null, -32600, 'Invalid Request');
+ return;
+ }
const { id, method, params } = msg;📝 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 handle(msg) { | |
| const { id, method, params } = msg; | |
| function handle(msg) { | |
| if (!msg || typeof msg !== 'object' || Array.isArray(msg)) { | |
| replyError(null, -32600, 'Invalid Request'); | |
| return; | |
| } | |
| const { id, method, params } = msg; |
🤖 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 @.genie/wishes/genie-mcp/spike/server.mjs around lines 34 - 35, The
handle(msg) logic destructures id, method, and params immediately after
JSON.parse() output, which can crash on non-object decoded values. Add a guard
in handle (or right after parsing in the message-processing path) to verify the
decoded message is a non-null object before destructuring, and return/send a
protocol error for malformed input instead of throwing. Keep the fix localized
to the handle(msg) flow so invalid lines are handled safely without taking down
the spike server.
| case 'initialize': | ||
| reply(id, { | ||
| protocolVersion: PROTOCOL_VERSION, | ||
| capabilities: { tools: { listChanged: false } }, | ||
| serverInfo: SERVER_INFO, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files '.genie/wishes/genie-mcp/spike/server.mjs' 'src/term-commands/mcp.ts' 'README.md' | cat
printf '\n== Outline: spike server ==\n'
ast-grep outline .genie/wishes/genie-mcp/spike/server.mjs --view expanded || true
printf '\n== Outline: mcp.ts ==\n'
ast-grep outline src/term-commands/mcp.ts --view expanded || true
printf '\n== Relevant snippets ==\n'
sed -n '1,140p' .genie/wishes/genie-mcp/spike/server.mjs
printf '\n--- src/term-commands/mcp.ts ---\n'
sed -n '1,220p' src/term-commands/mcp.ts
printf '\n== Search for listChanged and initialize shapes ==\n'
rg -n "listChanged|capabilities:\s*\{\s*tools|initialize" .genie/wishes/genie-mcp/spike/server.mjs src/term-commands/mcp.ts . || true
printf '\n== Search for MCP docs/spec references in repo ==\n'
rg -n "MCP|Model Context Protocol|listChanged|tools:\s*\{\s*\}" . --glob '!**/node_modules/**' || trueRepository: automagik-dev/genie
Length of output: 17541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== mcp.test.ts around initialize ==\n'
sed -n '1,180p' src/term-commands/mcp.test.ts
printf '\n== Any spike-specific docs ==\n'
rg -n "genie-spike|listChanged|tools: \{\}" .genie src README.md --glob '!**/node_modules/**' || trueRepository: automagik-dev/genie
Length of output: 9282
🌐 Web query:
MCP initialize capabilities tools listChanged 2024-11-05 spec
💡 Result:
In the Model Context Protocol (MCP) specification dated 2024-11-05, the listChanged capability is a sub-capability used to indicate that a server will emit notifications when its list of available tools changes [1][2]. When a server supports tools, it must declare this capability during the initialization handshake to inform the client of its ability to send these updates [1][2]. The declaration format is as follows: { "capabilities": { "tools": { "listChanged": true } } } If a server has declared this capability and the list of available tools changes, it should send a notifications/tools/list_changed notification to the client [1][2]: { "jsonrpc": "2.0", "method": "notifications/tools/list_changed" } Upon receiving this notification, the client is typically expected to perform a new tools/list request to discover the updated list of tools [3][4]. This mechanism allows clients to stay synchronized with dynamic tool environments [1][2].
Citations:
- 1: https://modelcontextprotocol.io/specification/2024-11-05/server/tools
- 2: https://startai.mintlify.app/specification/2024-11-05/server/tools
- 3: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/ff960c9e/docs/specification/draft/server/tools.mdx
- 4: https://mcpindotnet.github.io/docs/concepts/server-concepts/tools/
Match the MCP handshake shape — initialize should return capabilities: { tools: {} } here too; listChanged: false diverges from the repo’s MCP contract and tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.genie/wishes/genie-mcp/spike/server.mjs around lines 40 - 45, The
initialize handler in server.mjs is returning a tools capability shape that
diverges from the MCP contract used elsewhere in the repo. Update the reply
payload in the initialize case so it matches the expected handshake shape with
capabilities.tools as an empty object, and keep the change localized to the
initialize branch in the server message handler.
| export function ensureSchema(db: Database): void { | ||
| db.exec(SCHEMA_SQL); | ||
| ensureApprovalColumns(db); | ||
| } | ||
|
|
||
| /** | ||
| * Additive, in-place column backfill for `approvals`. `CREATE TABLE IF NOT | ||
| * EXISTS` never alters an existing table, so a DB stamped by an earlier build | ||
| * (which lacked `last_status_glyph`) needs the column added. It is nullable, so | ||
| * this stays within `user_version = 1` — no destructive migration, no version | ||
| * bump (mirrors genie-db.ts's ensureTaskColumns). Idempotent: a table that | ||
| * already has the column is left untouched. | ||
| */ | ||
| function ensureApprovalColumns(db: Database): void { | ||
| const cols = new Set((db.query('PRAGMA table_info(approvals)').all() as Array<{ name: string }>).map((c) => c.name)); | ||
| if (!cols.has('last_status_glyph')) { | ||
| db.exec('ALTER TABLE approvals ADD COLUMN last_status_glyph TEXT'); | ||
| // One-time upgrade backfill: stamp every already-closed approval as MIGRATED | ||
| // so the runner's reconciliation pass never re-acks pre-upgrade history | ||
| // (whose omni_message_id may be a bogus self-ref, or a real days-old stanza | ||
| // id we must not spam). Runs ONLY when the column is first added. | ||
| db.query(`UPDATE approvals SET last_status_glyph = ? WHERE status != 'pending'`).run(MIGRATED_STATUS_SENTINEL); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if a similar additive-column helper elsewhere already guards against
# concurrent duplicate-column races, and whether openSqlite/ensureSchema callers
# retry on SQLite errors.
rg -n "ensureTaskColumns" -A 15 src/lib/v5/genie-db.ts
rg -n "function openSqlite" -A 30 src/lib/v5/*.ts
rg -n "duplicate column" src -nRepository: automagik-dev/genie
Length of output: 3532
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the schema/open path around ensureSchema and any transaction or retry handling.
sed -n '1,260p' src/lib/v5/global-db.ts
printf '\n--- sqlite-open ---\n'
sed -n '120,260p' src/lib/v5/sqlite-open.ts
printf '\n--- search for schema init/retry helpers ---\n'
rg -n "ensureSchema|initWithBusyRetry|busy retry|transaction\\(" src/lib/v5 -A 20 -B 10Repository: automagik-dev/genie
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how SQLite behaves if the same migration is attempted twice in a row.
python3 - <<'PY'
import sqlite3, tempfile, os
path = tempfile.mktemp(suffix='.db')
conn = sqlite3.connect(path)
cur = conn.cursor()
cur.execute('CREATE TABLE approvals (status TEXT)')
conn.commit()
# Simulate first opener.
cur.execute('PRAGMA table_info(approvals)')
cols = {row[1] for row in cur.fetchall()}
if 'last_status_glyph' not in cols:
cur.execute('ALTER TABLE approvals ADD COLUMN last_status_glyph TEXT')
cur.execute("UPDATE approvals SET last_status_glyph = ? WHERE status != 'pending'", ('MIGRATED',))
conn.commit()
# Simulate second opener seeing stale state and retrying the same ALTER.
try:
cur.execute('ALTER TABLE approvals ADD COLUMN last_status_glyph TEXT')
conn.commit()
print('second ALTER unexpectedly succeeded')
except Exception as e:
print(type(e).__name__, str(e))
conn.close()
os.unlink(path)
PYRepository: automagik-dev/genie
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' src/lib/v5/global-db.ts
printf '\n--- sqlite-open ---\n'
sed -n '120,260p' src/lib/v5/sqlite-open.ts
printf '\n--- search for schema init/retry helpers ---\n'
rg -n "ensureSchema|initWithBusyRetry|busy retry|transaction\\(" src/lib/v5 -A 20 -B 10Repository: automagik-dev/genie
Length of output: 50375
Make the approval-column migration race-tolerant. Two concurrent first opens can both miss last_status_glyph and race into ALTER TABLE ... ADD COLUMN, turning a transient upgrade collision into MalformedDbError. Catch and ignore duplicate column name here, or serialize the check/add/backfill under one lock.
🤖 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/global-db.ts` around lines 126 - 149, The approval-column
migration in ensureApprovalColumns is vulnerable to a race when two first opens
both try to add last_status_glyph. Update ensureApprovalColumns to handle the
ALTER TABLE path safely by catching and ignoring the duplicate column name case,
or by serializing the PRAGMA table_info check, ALTER TABLE add, and backfill as
one locked operation so concurrent upgrades do not throw MalformedDbError. Keep
the idempotent behavior in ensureSchema and ensureApprovalColumns intact.
Source: Path instructions
| function genieBoard(ctx: ToolContext, args: Record<string, unknown>): BoardPayload { | ||
| const emptyCounts: StatusCounts = { blocked: 0, ready: 0, in_progress: 0, done: 0, total: 0 }; | ||
| const boardArg = argString(args, 'board'); | ||
| const wishArg = argString(args, 'wish'); | ||
| if (!ctx.db) return { board: boardArg ?? null, counts: emptyCounts, tasks: [] }; | ||
|
|
||
| const filter: TaskFilter = {}; | ||
| let boardName: string | null = null; | ||
| if (boardArg) { | ||
| const board = getBoardByName(ctx.db, boardArg); | ||
| // Unknown board name → empty projection (read-only; never throws at caller). | ||
| if (!board) return { board: boardArg, counts: emptyCounts, tasks: [] }; | ||
| boardName = board.name; | ||
| filter.boardId = board.id; | ||
| } | ||
| if (wishArg) filter.wish = wishArg; | ||
|
|
||
| const tasks = listTasks(ctx.db, filter); | ||
| return { board: boardName, counts: tally(tasks), tasks: tasks.map(toSummary) }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether a canonical 'repo' default board is used elsewhere (e.g. genie board/init commands)
rg -n "'repo'" src/term-commands src/lib/v5 -g '*.ts' -C2
rg -n "getBoardByName\(" src -g '*.ts' -C3Repository: automagik-dev/genie
Length of output: 4127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the MCP tool definition around genieBoard and its schema/docs.
sed -n '1,240p' src/lib/v5/mcp-tools.ts
# Inspect the MCP tests that seed boards and exercise board scoping.
sed -n '1,220p' src/term-commands/mcp.test.ts
# Look for any explicit documentation of the default board behavior.
rg -n "default repo board|board name; default|genie_board|genieBoard|boardArg" src -g '*.ts' -C2Repository: automagik-dev/genie
Length of output: 21373
genie_board's default board description is wrong.
When board is omitted, filter.boardId stays unset, so the tool returns tasks from every board, not a repo board. Either apply a canonical default here or change the schema text to say omission returns an unscoped cross-board list.
🤖 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/mcp-tools.ts` around lines 144 - 163, The genieBoard tool’s
default behavior is inconsistent with its description because omitting board
leaves the query unscoped and returns tasks from all boards. Update genieBoard
in mcp-tools.ts so it either applies a canonical default board when boardArg is
missing, or adjust the tool/schema description to explicitly state that no board
means a cross-board list; make the behavior and docs match for boardArg,
getBoardByName, and filter.boardId.
Two P2s from the #2510/#2509 PR review (Codex): - Worktree branch parsing split on the last '-', so a top-level 'wish/genie-mcp' mis-resolved to a 'genie' wish with an 'mcp' group. Now disambiguated against the KNOWN wish slugs (new listWishSlugs, longest-first): exact slug → top-level (group null); else longest known prefix + '-<group>'; heuristic only when the wish isn't in the db yet. A top-level branch now lists all the wish's tasks. - The MCP server opened the db once at startup; a db created mid-session (fresh genie init) was never picked up (empty forever). Reopen the read-only handle per tool call when null. Existing dbs already saw writes via WAL; only the absent→created case was stale. Tests: hyphenated-slug top-level + group branches resolve correctly; a db created after the server started is picked up. check 631, build green.
…lution Gemini HIGH: the per-call db reopen wrote to ctx.db while close() closed a separate startup 'db' local — a mid-session reopen leaked its handle. Drop the stale local; ctx.db is the single source of truth (close() closes the current handle). Codex P2: prefer a launch-worktree interpretation whose <group> is a REAL group of the prefix wish over a same-named top-level slug (disambiguates the genie-mcp / genie+mcp collision). Gemini MEDIUM: drop redundant GROUP BY (UNION already dedups). Codex P2: the reopen test now waits for the initialize reply (startup open ran) before seeding, so it can't pass without exercising the reopen. + collision test. check 632.
…d-db-reopen fix(mcp): db-backed branch resolution + reopen db created mid-session (2 PR-review P2s)
…dary CodeRabbit (#2510): the omni hook-timeout check warned only when timeoutMs < pollBudgetMs, but genie-config.ts's contract is pollBudgetMs MUST stay STRICTLY below the hook timeout — at timeoutMs === pollBudgetMs there is zero margin (CC can kill the hook the instant the budget expires), so it now warns on equal too. needSec = floor(pollBudgetMs/1000)+1 so the suggested timeout strictly exceeds the budget. Added the exact-boundary test.
…trict-boundary fix(doctor): enforce strictly-below hook-timeout contract at the boundary
Rolling promotion of
dev→main(stable release).Release payload
genie mcpstdio server exposing genie.db read-only (board, wish_status, worktree_context, task, active), auto-registered into.warp/.mcp.json+.mcp.jsonbygenie init/launch. Warp + Claude Code (any MCP client) can read genie's live state. Dogfood-proven live; 4 runtime deps unchanged (hand-rolled, no MCP SDK).genie omni test-approval+ agenie doctorhook-timeout guardrail. Both G3 review HIGHs found+fixed; additive schema (nouser_versionbump).Merge via "Create a merge commit" (NOT squash/rebase) — genie's
version.ymlfires the stable release only when the main tip isMerge pull request … from automagik-dev/dev(starts with "Merge pull request" + contains/dev). A squash/rebase would silently skip the release.Post-merge watch
After merging, confirm the Version → Release → Release Publish chain runs and the GitHub Release + npm publish land for the new
5.260703.x. (If genie shares omni'sGITHUB_TOKEN-can't-dispatch pattern, the release may need a manual nudge — I'll watch and flag.)Merge decision: Felipe.
Summary by CodeRabbit
genie mcp(stdio MCP server) with automatic MCP config registration duringgenie init, enabling read-only board/task/wish viewing by supported clients.genie omni test-approvalwith an optional live mode to run approval workflow verification.genie doctornow warns when the Omni hook timeout may be too short for the configured poll budget.5.260703.5.