From 1d777c6627366589328fdcc6e4d76dda81d73643 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 00:12:15 -0300 Subject: [PATCH 01/18] =?UTF-8?q?docs(brainstorm):=20genie=20MCP=20server?= =?UTF-8?q?=20design=20(SHIP-reviewed)=20=E2=80=94=20Warp=20+=20Claude=20C?= =?UTF-8?q?ode=20consume=20genie=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .genie/INDEX.md | 2 + .genie/brainstorms/genie-mcp/DESIGN.md | 64 ++++++++++++++++++++++++++ .genie/brainstorms/genie-mcp/DRAFT.md | 21 +++++++++ 3 files changed, 87 insertions(+) create mode 100644 .genie/brainstorms/genie-mcp/DESIGN.md create mode 100644 .genie/brainstorms/genie-mcp/DRAFT.md diff --git a/.genie/INDEX.md b/.genie/INDEX.md index 7027bce36..64b56b250 100644 --- a/.genie/INDEX.md +++ b/.genie/INDEX.md @@ -9,10 +9,12 @@ - [WISH: v5-completion](wishes/v5-completion/WISH.md) — CLAUDE.md-for-v5 rewrite ∥ Codex launch target + Hermes decision ∥ distribution 5.x (drafted 2026-07-02) - [WISH: dispatch-inproc-default](wishes/dispatch-inproc-default/WISH.md) — HIGH discovered defect: v5 hooks fall open by default (daemon deleted in demolition); re-arm branch-guard + unblock omni approvals (drafted 2026-07-02) +- [WISH: omni-approval-ux](wishes/omni-approval-ux/WISH.md) — correlated approval identity, reaction approve/deny, anti-spam feedback; grounded in the 2026-07-03 live WhatsApp QA (drafted 2026-07-03) - [WISH: omni-runner-port](wishes/omni-runner-port/WISH.md) — umbrella Group 5: approval-capture spike, global genie.db queue, genie omni serve, inbound one-shot (drafted 2026-07-02) - [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) — genie MCP server: Warp/Claude Code/Codex consume genie.db state read-only; stdio, auto-registered; spike-first (2026-07-03) - [Genie v5 — lightweight body](brainstorms/genie-v5-lightweight-body/DESIGN.md) — skills+files+stock Warp replace the v4 harness; CC/Codex/Hermes targets; omni keeps one runner; crystallized 2026-07-01, umbrella-scale (8 seed groups) - [WISH: v5-foundation](wishes/v5-foundation/WISH.md) — umbrella Groups 1+2: genie.db engine + CLI + core skills — DONE 2026-07-02, all groups SHIP diff --git a/.genie/brainstorms/genie-mcp/DESIGN.md b/.genie/brainstorms/genie-mcp/DESIGN.md new file mode 100644 index 000000000..d1c537953 --- /dev/null +++ b/.genie/brainstorms/genie-mcp/DESIGN.md @@ -0,0 +1,64 @@ +# Design: genie MCP server — Warp + Claude Code consume genie state + +| Field | Value | +|-------|-------| +| **Slug** | `genie-mcp` | +| **Date** | 2026-07-03 | +| **WRS** | 100/100 | + +## Problem +Genie's live state (wishes, tasks, board, what each worktree/pane is working on) is invisible to Warp and to the agents running inside it — there is no interface for Warp or a Claude Code session to read genie's state; the only Warp touchpoint today is a one-shot launch-config emitter. + +## Scope +### IN +- A `genie mcp` **stdio MCP server** exposing genie.db state READ-ONLY as MCP tools + resources: `genie_board` (counts + tasks, optional wish filter), `genie_wish_status` (a wish's group/DAG progress), `genie_worktree_context` (resolve the caller's git BRANCH — `wish/-` — → its wish, group, and tasks; the per-pane "what am I here for"), `genie_task` (detail by id), `genie_active` (all in-progress + who claimed what). Read layer already exists (`task-state.ts`: getBoardByName, listTasks{wish,status}, getTask, getWishGroups, claimed_by). +- The server opens genie.db **read-only via a net-new `new Database(path,{readonly:true})`** (NOT the existing `openSqlite()`, which force-creates + runs write pragmas) and DEGRADES to an empty board when the db file is absent. +- **Auto-registration for Warp + Claude Code**: `genie init` (and `genie launch` per worktree) writes/merges the PROJECT-scoped `/.warp/.mcp.json` (Warp) AND `/.mcp.json` (Claude Code), registering the `genie mcp` command under `mcpServers` — JSON parse → merge only the genie entry → preserve all other keys/servers → byte-identical rerun. +- A minimal automated MCP-protocol test driving the stdio server (initialize → tools/list → tools/call) + a non-MCP-path init probe (the MCP transport/dep must not load on `genie board`/`task`/etc.). +- Docs: how Warp/Claude Code pick up the genie MCP server; the honest tab-info limitation. + +### OUT +- WRITE tools (claim/complete task, create/advance wish) — a later wish; designed-for but not built (no second mutation path now; avoids racing the CLI/skills + §19/branch-guard rules). +- **Codex coverage** — Codex reads `~/.codex/config.toml` (TOML, global), NOT `.mcp.json`; wiring genie into Codex's global TOML is a separate later item. (Warp *detecting* Codex's config ≠ Codex seeing genie.) This wish covers **Warp + Claude Code** only. +- HTTP/SSE transport / any resident daemon (stdio only — fits the zero-daemon body). +- Pushing tab titles/blocks into Warp from outside — Warp exposes no such API (passive integration only); pane titles come from the launch config, live status from Warp's native Claude-Code integration. +- Any omni work; the global ~/.genie/genie.db omni tables (this is per-repo task/wish state first). + +## Approach +`genie mcp` is a stdio JSON-RPC MCP server that opens the per-repo `genie.db` read-only and answers `initialize`, `tools/list`, `tools/call`, `resources/list`, `resources/read`. MCP clients (Warp, Claude Code) launch it on demand as a CLI-command server per the project `.warp/.mcp.json` / `.mcp.json` that `genie init`/`launch` write. `genie_worktree_context` maps the caller's git branch (`wish/-`, the stable key — the worktree base dir is configurable via `GENIE_WORKTREES_DIR`) + genie.db → the pane's job. Alternatives considered: HTTP/SSE server (rejected — daemon); pushing state into Warp's UI (impossible — no API); a bespoke non-MCP protocol (rejected — MCP is the standard Warp + Claude Code both speak). + +## Decisions +| Decision | Rationale | +|----------|-----------| +| Read-only mirror now; write later | Delivers the "Warp consumes genie state" ask safely; no second mutation path racing the CLI/skills/§19 | +| stdio CLI-command server, not HTTP/SSE | Zero-daemon lightweight-body fit; MCP client launches it on demand, reads genie.db, exits | +| Auto-register Warp + Claude Code (.warp/.mcp.json + .mcp.json) | Both speak MCP and both are covered by these two project files; Codex uses a different (global TOML) format → deferred. Registering both is nearly free and bigger than Warp-only | +| Read-only open is NET-NEW code, owned by G2 | The existing `openSqlite()` always opens read-write with write pragmas; the server must open `{readonly:true}` itself + handle db-absent — not a reuse | +| G1 SPIKE decides hand-rolled MCP vs official SDK | genie is 4-deps-lean (hand-rolled JSON-RPC-over-stdio avoids a 5th dep, like Bun.YAML) BUT must actually satisfy real Warp + Claude Code MCP clients — verify against both before committing (dynamic-import the SDK like nats if needed) | +| `genie_worktree_context` resolves by BRANCH, not path | The worktree base dir is configurable; the branch `wish/-` is the stable key | +| Merge-if-exists config writes (JSON parse/merge/preserve) | Never clobber a user's existing `.warp/.mcp.json` / `.mcp.json` servers; this is new merge logic, not the gitignore line-append | + +## Risks & Assumptions +| Risk | Severity | Mitigation | +|------|----------|------------| +| Hand-rolled MCP subtly incompatible with Warp/CC clients | HIGH | G1 spike tests a minimal server against BOTH real clients before building; fall back to the official SDK (dynamic-imported like nats) if hand-roll fails | +| `.warp/.mcp.json` schema / location wrong | HIGH | G1 verifies the exact schema + pins the PROJECT path `/.warp/.mcp.json` (distinct from the GLOBAL `~/.warp/launch_configurations/` the emitter uses) against a real Warp | +| MCP dep bloats non-mcp startup | MEDIUM | If SDK is used, dynamic-import it inside `genie mcp` only (nats precedent, omni.ts:10); assert via a startup probe that `board`/`task` never load it | +| Read-only DB open is unbuilt capability | MEDIUM | G2 owns a net-new `new Database(path,{readonly:true})` + db-absent degrade-to-empty; do NOT rely on `openSqlite()` | +| Config write clobbers a user's MCP servers | MEDIUM | JSON parse → merge only the genie `mcpServers` entry → preserve other keys → byte-identical rerun; test empty + populated | +| Spawn `command` not on PATH for all installs | MEDIUM | G1/G3 confirm the launch command form (global `genie` vs the dist/bun install path) so the registered command actually runs | +| worktree→wish branch resolution outside a launch worktree | LOW | Fall back to repo-level board when the branch isn't `wish/-`; document | + +## Execution Groups (seed for /wish) +| Group | Deliverable | Depends on | Validation | +|-------|-------------|------------|------------| +| G1 Spike | Verify: (a) minimal hand-rolled MCP stdio server satisfies real Warp + Claude Code clients (or SDK needed); (b) exact PROJECT `.warp/.mcp.json` + `.mcp.json` schema + the spawn `command` form. SPIKE.md verdict + confirmed config shape | none | SPIKE.md exists; documents transport verdict + config schema + command form with evidence from real clients | +| G2 Server | `genie mcp` stdio server + the 5 read-only tools/resources; net-new readonly genie.db open with db-absent degrade; dynamic dep load if SDK | G1 | MCP-protocol test: initialize→tools/list→tools/call returns real genie.db state; readonly + absent-db test; non-mcp init probe (dep not loaded on board/task) | +| G3 Auto-register | `genie init`/`launch` JSON-merge-write `/.warp/.mcp.json` + `/.mcp.json` (preserve other servers); docs | G1 | idempotent merge test (empty + pre-populated with another server, byte-identical rerun preserving the other server); `genie init` in a fixture registers `genie mcp`; docs updated | + +## Success Criteria +- [ ] `genie mcp` answers MCP `initialize` + `tools/list` + `tools/call` over stdio and returns real genie.db state (board / wish_status / worktree_context) — automated protocol test; opens the db read-only and degrades to empty board when absent. +- [ ] `genie init` JSON-merge-writes idempotent `/.warp/.mcp.json` + `/.mcp.json` registering `genie mcp` (byte-identical on rerun; a pre-existing MCP server is preserved). +- [ ] A real Warp AND a real Claude Code session connect to `genie mcp` and read genie state (manual QA recorded in qa.md). +- [ ] Non-mcp paths (`genie board`/`task`/`--help`) do not initialize the MCP transport/dep (startup probe). +- [ ] `bun run check` + build green. diff --git a/.genie/brainstorms/genie-mcp/DRAFT.md b/.genie/brainstorms/genie-mcp/DRAFT.md new file mode 100644 index 000000000..d0db3c822 --- /dev/null +++ b/.genie/brainstorms/genie-mcp/DRAFT.md @@ -0,0 +1,21 @@ +# Brainstorm: genie MCP server — Warp (and any agent) consumes genie state + +## Seed / evidence (2026-07-03) +- genie has NO MCP server / `mcp` command today; it only emits one-shot Warp launch_configurations (warp-launch.ts). +- State lives in `genie.db` (per-repo: tasks/wishes/board; global ~/.genie/genie.db: omni approvals/inbox). +- Warp reality (docs.warp.dev, verified): MCP servers ARE Warp's plugin surface — defined in `.warp/.mcp.json` (project) / `~/.warp/.mcp.json` (global), as a CLI command (stdio) or HTTP/SSE endpoint. Warp reads MCP defs from Claude Code/Codex/other agents + AGENTS.md/WARP.md. Warp embeds Claude Code natively with per-tab status. NO external API to directly push tab titles/blocks — integration is passive (MCP listen / file detect), not active push. +- Implication: "Warp consumes genie state" = a genie MCP server Warp connects to. Same server auto-serves Claude Code + Codex (all MCP clients) → bigger than Warp alone. + +## Problem +Genie's state (wishes, tasks, board, what-each-worktree-is-doing) is invisible to Warp and to the agents running inside it; there's no way for Warp/Claude Code to read genie's live state. + +## Core idea +A `genie mcp` stdio server exposing genie.db state as MCP tools/resources, auto-registered via `.warp/.mcp.json` (+ `.mcp.json` for Claude Code) by `genie init`/`launch`. + +## Open questions +- Read-only state mirror vs read+write (agents drive genie via MCP)? +- Transport: stdio CLI-command (lightweight, no daemon — fits body) vs HTTP/SSE (daemon)? +- Breadth: Warp-only vs universal auto-register (Claude Code + Codex + Warp)? +- Which state matters most (board, wish progress, current-worktree context, activity)? + +## WRS: Problem ✅ | Scope ░ | Decisions ░ | Risks ░ | Criteria ░ From ad7044e50a5d211bf1896e7e21332ed09afe9d63 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 00:42:28 -0300 Subject: [PATCH 02/18] =?UTF-8?q?docs(wishes):=20genie-mcp=20wish=20(SHIP-?= =?UTF-8?q?reviewed)=20=E2=80=94=20Warp=20+=20Claude=20Code=20consume=20ge?= =?UTF-8?q?nie=20state=20via=20MCP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .genie/INDEX.md | 2 +- .genie/wishes/genie-mcp/WISH.md | 157 ++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 .genie/wishes/genie-mcp/WISH.md diff --git a/.genie/INDEX.md b/.genie/INDEX.md index 64b56b250..4dd4d1b82 100644 --- a/.genie/INDEX.md +++ b/.genie/INDEX.md @@ -14,7 +14,7 @@ - [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) — genie MCP server: Warp/Claude Code/Codex consume genie.db state read-only; stdio, auto-registered; spike-first (2026-07-03) +- [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) - [Genie v5 — lightweight body](brainstorms/genie-v5-lightweight-body/DESIGN.md) — skills+files+stock Warp replace the v4 harness; CC/Codex/Hermes targets; omni keeps one runner; crystallized 2026-07-01, umbrella-scale (8 seed groups) - [WISH: v5-foundation](wishes/v5-foundation/WISH.md) — umbrella Groups 1+2: genie.db engine + CLI + core skills — DONE 2026-07-02, all groups SHIP diff --git a/.genie/wishes/genie-mcp/WISH.md b/.genie/wishes/genie-mcp/WISH.md new file mode 100644 index 000000000..6c475c940 --- /dev/null +++ b/.genie/wishes/genie-mcp/WISH.md @@ -0,0 +1,157 @@ +# Wish: genie MCP server — Warp + Claude Code consume genie state + +| Field | Value | +|-------|-------| +| **Status** | DRAFT | +| **Slug** | `genie-mcp` | +| **Date** | 2026-07-03 | +| **Author** | Felipe + Genie | +| **Appetite** | ~3-4 days | +| **Branch** | `wish/genie-mcp` (from `dev`; PR to `dev`) | +| **Design** | [DESIGN.md](../../brainstorms/genie-mcp/DESIGN.md) | + +## Summary + +Genie's live state (wishes, tasks, board, what each worktree/pane is working on) is invisible to Warp and to the agents running inside it — the only Warp touchpoint today is a one-shot launch-config emitter. This wish adds a `genie mcp` stdio MCP server that exposes `genie.db` state READ-ONLY as MCP tools/resources, auto-registered into the project `.warp/.mcp.json` + `.mcp.json` by `genie init`/`launch`, so Warp and Claude Code (any MCP client) can read genie's live state — each Warp pane's agent can even ask "what wish/group am I here for." + +## Scope + +### IN +- `genie mcp` **stdio MCP server** answering `initialize` / `tools/list` / `tools/call` / `resources/list` / `resources/read`, exposing READ-ONLY: `genie_board` (counts + tasks, optional wish filter), `genie_wish_status` (a wish's group/DAG progress), `genie_worktree_context` (resolve the caller's git BRANCH `wish/-` → its wish/group/tasks), `genie_task` (detail by id), `genie_active` (in-progress + who claimed what). +- The server opens genie.db read-only via a **net-new `new Database(path,{readonly:true})`** (NOT `openSqlite()`, which force-creates + runs write pragmas) and DEGRADES to an empty board when the db file is absent. +- **Auto-registration** for Warp + Claude Code: `genie init` (and `genie launch` per worktree) JSON-merge-writes the project `/.warp/.mcp.json` AND `/.mcp.json`, registering `genie mcp` under `mcpServers` — parse → merge only the genie entry → preserve all other keys/servers → byte-identical rerun. +- An automated MCP-protocol test (initialize → tools/list → tools/call), a readonly + absent-db test, and a non-MCP-path init probe (the MCP transport/dep must not load on `board`/`task`/`--help`). +- Docs: how Warp/Claude Code pick up the genie MCP server + the honest tab-info limitation (ask-don't-push). + +### OUT +- WRITE tools (claim/complete task, create/advance wish) — a later wish; no second mutation path now. +- **Codex** coverage — Codex reads `~/.codex/config.toml` (TOML, global), NOT `.mcp.json`; wiring genie into Codex is a separate later item. This wish covers Warp + Claude Code only. +- HTTP/SSE transport / any resident daemon (stdio only). +- Pushing tab titles/blocks into Warp from outside — Warp exposes no such API (passive integration only). +- Any omni work; the global `~/.genie/genie.db` omni tables. + +## Decisions + +| # | Decision | Rationale | +|---|----------|-----------| +| 1 | Read-only mirror now; write later | Delivers the "Warp consumes genie state" ask safely; no mutation path racing the CLI/skills/§19 | +| 2 | stdio CLI-command server, not HTTP/SSE | Zero-daemon lightweight-body fit; the MCP client launches it on demand, reads genie.db, exits | +| 3 | Auto-register Warp + Claude Code (.warp/.mcp.json + .mcp.json); Codex deferred | Both speak MCP and both are covered by these two project files; Codex uses a different global TOML format | +| 4 | Read-only DB open is NET-NEW code (G2 owns it) | `openSqlite()` always opens read-write with write pragmas; the server opens `{readonly:true}` itself + degrades on absent db | +| 5 | G1 SPIKE decides hand-rolled MCP vs official SDK | genie is 4-deps-lean; a hand-rolled JSON-RPC-over-stdio avoids a 5th dep (Bun.YAML precedent) BUT must satisfy real Warp + Claude Code clients — verify against both first; dynamic-import the SDK (nats precedent) if needed | +| 6 | `genie_worktree_context` resolves by git BRANCH, not path | The worktree base dir is configurable (`GENIE_WORKTREES_DIR`); branch `wish/-` is the stable key | +| 7 | Config writes are JSON parse/merge/preserve, not line-append | Never clobber a user's existing MCP servers; byte-identical rerun | + +## Success Criteria + +- [ ] `genie mcp` answers MCP `initialize` + `tools/list` + `tools/call` over stdio and returns real genie.db state (board / wish_status / worktree_context) — automated protocol test; opens the db read-only and degrades to an empty board when absent. +- [ ] `genie init` JSON-merge-writes idempotent `/.warp/.mcp.json` + `/.mcp.json` registering `genie mcp` (byte-identical on rerun; a pre-existing MCP server is preserved). +- [ ] A real Warp AND a real Claude Code session connect to `genie mcp` and read genie state (manual QA recorded in qa.md). +- [ ] Non-mcp paths (`genie board`/`task`/`--help`) do not initialize the MCP transport/dep (startup probe). +- [ ] Full `bun run check` + build green; CI green on the PR. + +## Execution Strategy + +| Wave | Groups | Notes | +|------|--------|-------| +| 1 | Group 1 | Spike: transport verdict + Warp/CC config schema — gates G2/G3 | +| 2 | Group 2, Group 3 | Server ∥ auto-registration (disjoint: `src/term-commands/mcp.ts`+lib vs `init.ts`/`launch.ts` config-writing) | + +--- + +## Execution Group 1: Spike — MCP transport + Warp/Claude Code config schema +**Goal:** Decide hand-rolled-MCP-vs-SDK and pin the exact config format by testing against real Warp + Claude Code MCP clients before building. + +**Deliverables:** +1. A throwaway minimal stdio MCP server (under `.genie/wishes/genie-mcp/spike/`) implementing `initialize` + `tools/list` + one trivial `tools/call`, registered via a project `.warp/.mcp.json` AND `.mcp.json` in a scratch repo, and CONNECTED FROM real Warp AND real Claude Code — record whether the hand-rolled JSON-RPC satisfies both clients (or the official SDK is required). +2. `.genie/wishes/genie-mcp/SPIKE.md`: verdict (hand-rolled | SDK), the exact `.warp/.mcp.json` + `.mcp.json` schema (keys, the `command`/`args` shape, project vs global location), the spawn `command` form that actually runs on this install (global `genie` vs the dist/bun path), and the tools/resources JSON contract Group 2 must implement. + +**Acceptance Criteria:** +- [ ] SPIKE.md documents the transport verdict + the confirmed config schema + command form, with evidence from BOTH real clients. +- [ ] The scratch config uses the PROJECT `/.warp/.mcp.json` (confirmed distinct from the global `~/.warp/launch_configurations/`). + +**Validation:** +```bash +set -euo pipefail +cd "$(git rev-parse --show-toplevel)" +test -f .genie/wishes/genie-mcp/SPIKE.md +grep -qiE '^## Verdict: (hand-rolled|SDK)' .genie/wishes/genie-mcp/SPIKE.md +grep -q 'mcpServers' .genie/wishes/genie-mcp/SPIKE.md +grep -qiE 'command' .genie/wishes/genie-mcp/SPIKE.md +``` + +**depends-on:** none + +--- + +## Execution Group 2: `genie mcp` server + read-only tools +**Goal:** A working stdio MCP server exposing genie.db state read-only, per the G1 verdict. + +**Deliverables:** +1. `src/term-commands/mcp.ts` (+ a lib module for the tool implementations) — `genie mcp` stdio server per G1 (hand-rolled or dynamic-imported SDK); the 5 read-only tools/resources over genie.db reads (`getBoardByName`, `listTasks{wish,status}`, `getTask`, `getWishGroups`, `claimed_by`); registered in `src/genie.ts`. +2. Net-new **`new Database(path,{readonly:true})`** open (NOT `openSqlite()`), degrading to an empty board when the db file is absent; `genie_worktree_context` resolves by the git branch `wish/-` with a repo-level-board fallback when the branch doesn't match. +3. If the SDK is used, dynamic-import it inside `genie mcp` only (nats precedent). +4. Tests: an MCP-protocol test (spawn `genie mcp`, drive initialize → tools/list → tools/call, assert real genie.db state); a readonly + absent-db test; a non-mcp init probe (the MCP transport/dep is NOT loaded on `board`/`task`/`--help`). + +**Acceptance Criteria:** +- [ ] `genie mcp` answers initialize/tools/list/tools/call over stdio with real genie.db state; opens read-only; degrades to empty board on absent db. +- [ ] Non-mcp paths don't initialize the MCP transport/dep (probe test). +- [ ] typecheck + `bun test` for the mcp module + build green; `--help` lists `mcp`. + +**Validation:** +```bash +set -euo pipefail +cd "$(git rev-parse --show-toplevel)" +bun test src/term-commands/mcp.test.ts +bun run typecheck +bun run build +bun dist/genie.js --help | grep -qE '^ mcp' || { echo "FAIL: mcp command missing"; exit 1; } +# protocol smoke: initialize returns a result envelope (capture to a var to +# neutralize SIGPIPE/pipefail; timeout guards a non-exiting server; first line only) +INIT='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"t","version":"0"}}}' +OUT=$(printf '%s\n' "$INIT" | timeout 10 bun dist/genie.js mcp 2>/dev/null | head -n1 || true) +echo "$OUT" | grep -q '"result"' || { echo "FAIL: mcp initialize no result"; exit 1; } +``` + +> **G2 impl note:** the stdio server must flush the pending `initialize` response BEFORE exiting on stdin `end` — do NOT `process.exit(0)` on `stdin.on('end')` before async response writes drain, or stdout truncates. Also confirm a `claimed_by` column exists on TaskRow for `genie_active` (one-line check). + +**depends-on:** group-1 + +--- + +## Execution Group 3: Auto-registration + docs +**Goal:** `genie init`/`launch` wire `genie mcp` into Warp + Claude Code, preserving any existing config. + +**Deliverables:** +1. `genie init` (and `genie launch` per worktree) JSON-merge-writes `/.warp/.mcp.json` + `/.mcp.json` registering the `genie mcp` command (per the G1 command form) under `mcpServers`: parse existing → add/update only the `genie` entry → preserve all other servers/keys → byte-identical on rerun. Create the file when absent. +2. Docs: a short guide on how Warp + Claude Code pick up the genie MCP server, the state it exposes, and the honest tab-info limitation (ask-don't-push). +3. Tests: idempotent merge test — empty case AND a pre-populated `.mcp.json` with ANOTHER server (assert the other server survives + byte-identical rerun); `genie init` in a fixture repo registers `genie mcp` in both files. + +**Acceptance Criteria:** +- [ ] `genie init` writes/merges both files registering `genie mcp`; a pre-existing MCP server is preserved; rerun is byte-identical. +- [ ] Docs updated; the tab-info limitation stated honestly. +- [ ] Full `bun run check` green. + +**Validation:** +```bash +set -euo pipefail +cd "$(git rev-parse --show-toplevel)" +GENIE_JS="$PWD/dist/genie.js" +bun run build +bun test src/term-commands/init.test.ts +D=$(mktemp -d)/repo; mkdir -p "$D"; git -C "$D" init -q +# pre-populate .mcp.json with another server; assert it survives + genie is added +printf '%s\n' '{"mcpServers":{"other":{"command":"x"}}}' > "$D/.mcp.json" +( cd "$D" && bun "$GENIE_JS" init >/dev/null 2>&1 ) +python3 -c "import json;d=json.load(open('$D/.mcp.json'))['mcpServers'];assert 'other' in d,'other server dropped';assert 'genie' in d,'genie not registered'" +test -f "$D/.warp/.mcp.json" +bun run check +``` + +**depends-on:** group-1 + +--- + +## Cross-wish dependencies +- **Builds on** warp-integration (the launch worktree/branch convention `wish/-` that `genie_worktree_context` resolves). +- **Enables** (later wishes): MCP WRITE tools; Codex TOML wiring; richer per-agent genie context. From 9e976006510b8613f2f6f4ae21da2b7849c3ed92 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 3 Jul 2026 03:43:24 +0000 Subject: [PATCH 03/18] chore(version): bump to 5.260703.2 [auto-version] --- .claude-plugin/marketplace.json | 2 +- package.json | 2 +- plugins/genie/.claude-plugin/plugin.json | 2 +- plugins/genie/package.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 99e727463..7cd086aee 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "genie", - "version": "5.260703.1", + "version": "5.260703.2", "source": "./plugins/genie", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, wish them into plans, make with parallel agents, ship as one team. A coding genie that grows with your project." } diff --git a/package.json b/package.json index cf1a970d7..e969f22f8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@automagik/genie", - "version": "5.260703.1", + "version": "5.260703.2", "description": "Collaborative terminal toolkit for human + AI workflows. NOTE: npm distribution discontinued 2026-05-09 — install via `curl -fsSL https://raw.githubusercontent.com/automagik-dev/genie/main/install.sh | bash` (cosign + SLSA verified). See https://automagik.dev/genie/release-process", "type": "module", "bin": { diff --git a/plugins/genie/.claude-plugin/plugin.json b/plugins/genie/.claude-plugin/plugin.json index 4b0693a8e..3b3f7e550 100644 --- a/plugins/genie/.claude-plugin/plugin.json +++ b/plugins/genie/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "genie", - "version": "5.260703.1", + "version": "5.260703.2", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, turn them into wishes, execute with /work, validate with /review, and ship as one team.", "author": { "name": "Namastex Labs" diff --git a/plugins/genie/package.json b/plugins/genie/package.json index 7331eb84d..edc3c0d23 100644 --- a/plugins/genie/package.json +++ b/plugins/genie/package.json @@ -1,6 +1,6 @@ { "name": "genie-plugin", - "version": "5.260703.1", + "version": "5.260703.2", "private": true, "description": "Runtime dependencies for genie bundled CLIs", "type": "module", From c350dea265ee252f90aaf4905a0f2ea3c4a1eb4c Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 00:53:18 -0300 Subject: [PATCH 04/18] chore(genie-mcp): spike verdict hand-rolled, mcp contract pinned hand-rolled stdio server satisfies real claude code; contract pinned for g2 --- .genie/wishes/genie-mcp/SPIKE.md | 123 ++++++++++++++++++++ .genie/wishes/genie-mcp/WISH.md | 4 +- .genie/wishes/genie-mcp/spike/server.mjs | 90 ++++++++++++++ .genie/wishes/omni-approval-ux/WISH.md | 142 +++++++++++++++++++++++ 4 files changed, 357 insertions(+), 2 deletions(-) create mode 100644 .genie/wishes/genie-mcp/SPIKE.md create mode 100644 .genie/wishes/genie-mcp/spike/server.mjs create mode 100644 .genie/wishes/omni-approval-ux/WISH.md diff --git a/.genie/wishes/genie-mcp/SPIKE.md b/.genie/wishes/genie-mcp/SPIKE.md new file mode 100644 index 000000000..932902047 --- /dev/null +++ b/.genie/wishes/genie-mcp/SPIKE.md @@ -0,0 +1,123 @@ +# G1 Spike — MCP transport + Warp/Claude Code config schema + +Date: 2026-07-03 · Worker: g1-spike · Gates: G2, G3 + +## Verdict: hand-rolled + +A hand-rolled newline-delimited JSON-RPC 2.0 stdio server (zero deps) satisfied a **real Claude Code** client end-to-end (`initialize` → `tools/list` → `tools/call`) on two boxes, and speaks the exact MCP protocol (`2024-11-05`) Warp uses. Genie stays 4-runtime-deps-lean — no 5th dep (`@modelcontextprotocol/sdk`) required. The SDK remains the documented fallback (dynamic-imported like `nats` in `omni.ts`) **only if** Warp's live UI later reveals a framing/handshake quirk during Felipe's eyeball check (see Warp evidence below) — nothing observed suggests it will. + +Throwaway server: `.genie/wishes/genie-mcp/spike/server.mjs` (runs under both `node` and `bun`; NOT in `src/`). + +## Framing + +**Newline-delimited JSON** — one JSON-RPC 2.0 object per line on stdin/stdout, no embedded newlines, no `Content-Length` headers. This is the MCP stdio transport (Content-Length framing is LSP, not MCP). Confirmed: real Claude Code connected and invoked a tool against the newline-delimited server with no framing negotiation. + +Handshake contract: +- Client → `initialize` (`params.protocolVersion: "2024-11-05"`, `capabilities`, `clientInfo`). +- Server → result envelope `{ protocolVersion, capabilities: { tools: {...} }, serverInfo: { name, version } }`. +- Client → `notifications/initialized` (a notification — **no `id`, no response**; the server must not reply). +- Then `tools/list`, `tools/call`, (optional `ping`, `resources/list`, `resources/read`). +- Notifications (messages with no `id`) get NO response. Unknown method with an `id` → JSON-RPC error `-32601`. +- **Flush before exit:** on stdin `end`, drain stdout before `process.exit(0)` or the last response truncates (mirrors the WISH G2 impl note). + +## Evidence per client + +### Claude Code — CONFIRMED end-to-end (both Mac + Linux) + +- **Mac (local):** project `.mcp.json` was auto-discovered and listed by `claude mcp list` as `⏸ Pending approval` (the expected project-scope security gate — project servers require workspace trust before connecting). Adding the identical server at `--scope local` (auto-approved) health-checked to **`✔ Connected`** (a real `initialize` handshake). A headless `claude -p "...call the echo tool..." --allowedTools mcp__genie-spike__echo` returned the tool's exact output `echo: spike-ok-12345` → proves `initialize` + `tools/list` + `tools/call` through the real client. +- **Linux box `felipe` (where Warp-over-SSH would spawn the server):** the same hand-rolled server health-checked to **`✔ Connected`** under Claude Code `2.1.199`. +- Both registrations were removed after testing. + +Claude Code `.mcp.json` notes for G3: +- `type: "stdio"` is **optional** — a server object with `command` present defaults to stdio. +- Project `.mcp.json` servers are **pending approval** until the user trusts the workspace (runs `claude` interactively and accepts the trust dialog) — expected, not a bug; document it. +- Supports `${VAR}` env expansion and an optional per-server `"timeout"` (ms). + +### Warp — schema + protocol CONFIRMED; live UI connection NEEDS FELIPE'S EYES + +`felipe` is a **Linux** dev box (`warp-terminal` Linux build) that Felipe reaches **via Warp's SSH remote feature** — the Warp GUI runs on Felipe's Mac and drives a `~/.warp/remote-server/` component on `felipe`. Warp opens the repo on the remote, so it discovers `/.warp/.mcp.json` **on `felipe`** and spawns the stdio server **on `felipe`**. + +Confirmed without the GUI: +- **Authoritative schema** — Warp's own bundled skill on the box: `~/.warp/remote-server/bundled_resources/bundled/skills/add-mcp-server/SKILL.md` (see Config Schema below). This is Warp's shipped source of truth, not a third-party doc. +- **Warp's MCP subsystem is live on the remote** — `~/.local/state/warp-terminal/oz/warp.log`: `mcp_static_config: Some(McpStaticConfig{...})` and `Converting 0 legacy MCP servers into templatable MCP servers` (0 because the scratch repo hasn't been opened in a Warp session yet). +- **The server runs where Warp spawns it** — the raw `initialize`/`tools/list` handshake was replayed with `node` on `felipe` and returned correct envelopes. +- **Config staged for eyeball** — `~/genie-mcp-spike/scratch-repo/.warp/.mcp.json` (+ `.mcp.json`) on `felipe`, pointing at `~/genie-mcp-spike/server.mjs`. + +**What needs Felipe's eyes (cannot be driven over ssh — Warp is a GUI):** open Warp on the Mac → SSH into `felipe` → `cd ~/genie-mcp-spike/scratch-repo` → check **Settings → AI/Agents → MCP servers**. Expect `genie-spike` listed **"Detected from Warp"** and a green/connected status; optionally ask Warp's agent to call the `echo` tool. Warp auto-detects `.mcp.json` changes on save (no restart). Since Warp speaks the same MCP `2024-11-05` JSON-RPC that Claude Code accepted against this exact server, the risk is low; the only unproven bit is Warp's client-side connect, not the protocol. + +## Config schema (both files) + +**Identical shape for both clients.** Both use the top-level `mcpServers` key and the stdio `{ command, args, env?, working_directory? }` per-server object — a single JSON body satisfies both files. `type: "stdio"` is optional (omit it). + +Locations (PROJECT scope — what G3 writes; distinct from the GLOBAL launch-config path): +- Warp project: `/.warp/.mcp.json` — spawns from repo root by default. (Global would be `~/.warp/.mcp.json`.) **Distinct from** the launch-config emitter's global `~/.warp/launch_configurations/`. +- Claude Code project: `/.mcp.json` — checked into version control; team-shared. +- (Warp also reads `~/.claude.json` as a *global* config, per its bundled skill — not used by this wish.) + +Warp recognizes these wrapper keys, in order of preference: `mcpServers` (preferred), `mcp_servers`, `servers`, nested `mcp.servers`, or a flat map. G3 writes/merges under **`mcpServers`** and preserves whichever wrapper key an existing file already uses. Never remove existing server entries — add/update only the `genie` entry. + +Concrete genie block (what G3 writes to BOTH `/.warp/.mcp.json` and `/.mcp.json`): + +```json +{ + "mcpServers": { + "genie": { + "command": "/absolute/path/to/genie", + "args": ["mcp"] + } + } +} +``` + +## Spawn `command` form (pinned) + +**Use an ABSOLUTE path to the genie binary — NOT bare `"genie"`.** `{"command":"genie","args":["mcp"]}` assumes `genie` is on the spawning process's PATH, which is NOT reliable: +- **Mac (this install):** `genie` is **not** on PATH — it lives only at `~/.genie/bin/genie` (a 59 MB bun-compiled single-file binary). Bare `genie` would fail to spawn. +- **`felipe` (Linux):** `genie` IS on PATH (`/home/genie/.local/bin/genie`, also `~/.genie/bin/genie`). + +Recommendation for G3: at `genie init`/`launch` time, resolve the **absolute path of the currently-running genie executable** (for the bun-compiled binary that is `process.execPath`) and write that as `command`, with `args: ["mcp"]`. Fallback: `~/.genie/bin/genie` expanded to an absolute path. This is self-consistent under Warp-over-SSH: `genie init` runs on the box that owns the repo (e.g. `felipe`), so it writes that box's genie path, and Warp spawns the server on that same box. Warp supports `${VAR}` substitution, but a resolved absolute path is simpler and avoids PATH ambiguity. + +## Tools contract for G2 (the 5 read-only tools) + +All backed by the existing read layer in `src/lib/v5/task-state.ts` (`getBoardByName`, `getTask`, `listTasks{status,wish,boardId}`, `getWishGroups`) over a **net-new `new Database(resolveDbPath(cwd), { readonly: true })`** open (NOT `openSqlite()`); degrade to an empty board when the db file is absent. `resolveDbPath()` (in `src/lib/v5/genie-db.ts`) yields the worktree-shared `.genie/genie.db`. + +`tools/call` result envelope (MCP standard): `{ "content": [{ "type": "text", "text": "" }], "isError": false }`. Optionally also set `structuredContent` to the payload object. Payload shapes below (from `TaskRow`/`WishGroupRow`/`BoardRow`). + +Reference types (`task-state.ts`): `TaskStatus = 'blocked'|'ready'|'in_progress'|'done'`; `TaskRow = { id, boardId, title, status, claimedBy, claimedAt, wish, group, createdAt, updatedAt }`; `WishGroupRow = { wish, name, status, dependsOn:string[], assignee, startedAt, completedAt }`; `BoardRow = { id, name, createdAt }`. + +1. **`genie_board`** — board counts + tasks, optional wish filter. + - inputSchema: `{ type:"object", properties:{ board:{type:"string", description:"board name; default repo board"}, wish:{type:"string", description:"filter to a wish slug"} }, required:[] }` + - payload: `{ board: string|null, counts: { blocked:number, ready:number, in_progress:number, done:number, total:number }, tasks: TaskSummary[] }` where `TaskSummary = { id, title, status, claimedBy, wish, group }`. + - impl: `getBoardByName(db, board)` → `listTasks(db, { boardId, wish? })`; tally counts. + +2. **`genie_wish_status`** — a wish's group/DAG progress. + - inputSchema: `{ type:"object", properties:{ wish:{type:"string", description:"wish slug"} }, required:["wish"] }` + - payload: `{ wish: string, groups: { name, status, dependsOn:string[], assignee, startedAt, completedAt }[], tasks: TaskSummary[] }`. + - impl: `getWishGroups(db, wish)` + `listTasks(db, { wish })`. + +3. **`genie_worktree_context`** — resolve the caller's git BRANCH `wish/-` → its wish/group/tasks (the per-pane "what am I here for"). + - inputSchema: `{ type:"object", properties:{ branch:{type:"string", description:"override; default = current git branch"} }, required:[] }` + - payload: `{ branch: string|null, resolved: boolean, wish: string|null, group: string|null, tasks: TaskSummary[] }`. + - impl: read the current branch (`git rev-parse --abbrev-ref HEAD` / `git symbolic-ref`); parse `wish/-`; on match → `listTasks(db, { wish })` filtered to the group (`resolved:true`); on no match → `resolved:false` with the repo-level board tasks as fallback. Resolve by BRANCH, not path (worktree base dir is configurable via `GENIE_WORKTREES_DIR`). + +4. **`genie_task`** — full detail by id. + - inputSchema: `{ type:"object", properties:{ id:{type:"string", description:"task id, e.g. t_..."} }, required:["id"] }` + - payload: the full `TaskRow` (`{ id, boardId, title, status, claimedBy, claimedAt, wish, group, createdAt, updatedAt }`) or `{ error: "not_found", id }` (surface a not-found result, not a protocol error). + - impl: `getTask(db, id)`. + +5. **`genie_active`** — all in-progress tasks + who claimed what. + - inputSchema: `{ type:"object", properties:{}, required:[] }` + - payload: `{ tasks: { id, title, status, claimedBy, claimedAt, wish, group }[] }`. + - impl: `listTasks(db, { status: 'in_progress' })`. + +Also implement `initialize` (advertise `capabilities.tools`), `tools/list` (the 5 above), and `ping`. `resources/list`/`resources/read` are optional for G2 (the wish lists them but the 5 tools are the required contract); if included, expose the board/wishes as resources mirroring the same payloads. + +## Non-MCP startup probe (for G2) + +The MCP transport must not load on `genie board`/`task`/`--help`. Because the hand-rolled server is plain stdin/stdout JSON with zero imports, this is trivially satisfied by keeping the server module lazy (only reached inside the `genie mcp` command body). If the SDK fallback is ever taken, dynamic-import it inside `genie mcp` only (nats precedent, `omni.ts`). + +## Cleanup done + +- Removed both Claude Code registrations (Mac local-scope + felipe local-scope). +- Left intentionally for Felipe's Warp eyeball: `~/genie-mcp-spike/` on `felipe` (server.mjs + scratch-repo with `.warp/.mcp.json` + `.mcp.json`). Safe to `rm -rf ~/genie-mcp-spike` after the Warp check. +- Local Mac scratch repo is under `/var/folders/.../T/` (OS-cleaned tmp). diff --git a/.genie/wishes/genie-mcp/WISH.md b/.genie/wishes/genie-mcp/WISH.md index 6c475c940..12a15da73 100644 --- a/.genie/wishes/genie-mcp/WISH.md +++ b/.genie/wishes/genie-mcp/WISH.md @@ -67,8 +67,8 @@ Genie's live state (wishes, tasks, board, what each worktree/pane is working on) 2. `.genie/wishes/genie-mcp/SPIKE.md`: verdict (hand-rolled | SDK), the exact `.warp/.mcp.json` + `.mcp.json` schema (keys, the `command`/`args` shape, project vs global location), the spawn `command` form that actually runs on this install (global `genie` vs the dist/bun path), and the tools/resources JSON contract Group 2 must implement. **Acceptance Criteria:** -- [ ] SPIKE.md documents the transport verdict + the confirmed config schema + command form, with evidence from BOTH real clients. -- [ ] The scratch config uses the PROJECT `/.warp/.mcp.json` (confirmed distinct from the global `~/.warp/launch_configurations/`). +- [x] SPIKE.md documents the transport verdict + the confirmed config schema + command form, with evidence from BOTH real clients. +- [x] The scratch config uses the PROJECT `/.warp/.mcp.json` (confirmed distinct from the global `~/.warp/launch_configurations/`). **Validation:** ```bash diff --git a/.genie/wishes/genie-mcp/spike/server.mjs b/.genie/wishes/genie-mcp/spike/server.mjs new file mode 100644 index 000000000..a1a730843 --- /dev/null +++ b/.genie/wishes/genie-mcp/spike/server.mjs @@ -0,0 +1,90 @@ +#!/usr/bin/env node +// THROWAWAY G1 spike: hand-rolled JSON-RPC 2.0 MCP server over stdio. +// Newline-delimited framing (one JSON object per line) — MCP stdio transport. +// Implements just: initialize, tools/list, tools/call (echo). No deps. +// Runs under both `bun` and `node`. + +const PROTOCOL_VERSION = '2024-11-05'; +const SERVER_INFO = { name: 'genie-spike', version: '0.0.1' }; + +const TOOLS = [ + { + name: 'echo', + description: 'Echo back the provided text (spike smoke tool).', + inputSchema: { + type: 'object', + properties: { text: { type: 'string', description: 'Text to echo' } }, + required: ['text'], + }, + }, +]; + +function send(msg) { + process.stdout.write(JSON.stringify(msg) + '\n'); +} + +function reply(id, result) { + send({ jsonrpc: '2.0', id, result }); +} + +function replyError(id, code, message) { + send({ jsonrpc: '2.0', id, error: { code, message } }); +} + +function handle(msg) { + const { id, method, params } = msg; + // Notifications (no id) — e.g. notifications/initialized — need no response. + if (method === 'notifications/initialized' || method === 'initialized') return; + + switch (method) { + case 'initialize': + reply(id, { + protocolVersion: PROTOCOL_VERSION, + capabilities: { tools: { listChanged: false } }, + serverInfo: SERVER_INFO, + }); + return; + case 'ping': + reply(id, {}); + return; + case 'tools/list': + reply(id, { tools: TOOLS }); + return; + case 'tools/call': { + const name = params?.name; + const args = params?.arguments ?? {}; + if (name === 'echo') { + reply(id, { + content: [{ type: 'text', text: `echo: ${args.text ?? ''}` }], + isError: false, + }); + return; + } + replyError(id, -32602, `Unknown tool: ${name}`); + return; + } + default: + if (id !== undefined) replyError(id, -32601, `Method not found: ${method}`); + } +} + +let buf = ''; +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { + buf += chunk; + let nl; + while ((nl = buf.indexOf('\n')) >= 0) { + const line = buf.slice(0, nl).trim(); + buf = buf.slice(nl + 1); + if (!line) continue; + try { + handle(JSON.parse(line)); + } catch (e) { + replyError(null, -32700, `Parse error: ${e.message}`); + } + } +}); +// Do NOT exit on stdin end until stdout drains, or clients truncate responses. +process.stdin.on('end', () => { + process.stdout.write('', () => process.exit(0)); +}); diff --git a/.genie/wishes/omni-approval-ux/WISH.md b/.genie/wishes/omni-approval-ux/WISH.md new file mode 100644 index 000000000..078ae67e9 --- /dev/null +++ b/.genie/wishes/omni-approval-ux/WISH.md @@ -0,0 +1,142 @@ +# Wish: Omni Approval UX — Correlated Identity, Reactions, Anti-Spam + +| Field | Value | +|-------|-------| +| **Status** | DRAFT | +| **Slug** | `omni-approval-ux` | +| **Date** | 2026-07-03 | +| **Author** | Felipe + Genie | +| **Appetite** | ~3-4 days | +| **Branch** | `wish/omni-approval-ux` (from `dev`; PR to `dev`) | +| **Design** | _No brainstorm — grounded in the live WhatsApp QA on 2026-07-03_ | +| **Depends on** | `omni-runner-port` (merged), `wish/omni-hardening` (PR #2507) | + +## Summary + +The v5 omni approval bridge is live-proven (a PreToolUse hook → WhatsApp 🔔 → text `y` → allow envelope, verified end-to-end against the real Omni hub). But the live QA exposed three UX/correctness gaps: replies resolve the OLDEST pending approval rather than the one the human actually answered (concurrent-approval hazard), reactions (👍/👎) don't resolve because genie subscribes `omni.event.>` while this Omni build doesn't publish reactions there, and every approval is a fresh WhatsApp message with no resolution feedback (a stale "Approval Required" prompt lingers even after it's decided). This wish makes approvals correlated, reaction-driven, and self-updating. + +## Scope + +### IN +- **Correlated approval identity (fixes the oldest-pending hazard):** capture the REAL Omni message id of each sent approval request and store it in `approvals.omni_message_id`, so a reply/reaction resolves the EXACT approval it answered — not `resolveOldest`. Today the runner publishes fire-and-forget to `omni.reply.{instance}.{chat}` and never learns Omni's message id, so it falls back to oldest-pending (a correctness bug the omni-runner-port reviews flagged as LOW, confirmed live). Decision needed (Group 1 spike): send the approval via the **Omni HTTP send API** (which returns a `messageId` — proven: `omni send` returned `3EB097217C5450F5E0166D`) vs. correlating the NATS-published id. Prefer the path that yields a stable id genie can match inbound replies/reactions against. +- **Reaction-based approve/deny:** verify the ACTUAL NATS subject this Omni build publishes reactions on (the live QA could not confirm `omni.event.>` carries them), subscribe the correct subject, and resolve 👍/👎 by correlating to the stored `omni_message_id` from the correlated-identity work. Text `y`/`n` replies remain a fallback. Honor the instance-scope guard already added in PR #2507. +- **Resolution feedback (anti-spam):** on resolve/expire, update the human's thread so a decided approval doesn't linger as an open prompt — e.g. react ✅/❌ on the original approval message (via `omni react`) and/or send a one-line status ("approved by you ✅"). One message per approval, plus one lightweight status signal — never a re-announce (the per-approval `omniMessageId` dedup at omni-runner.ts:360 already prevents re-fires; keep it). +- **Operator guardrails learned from the QA:** document + enforce that enabling approvals requires the CC hook `timeout` ≥ `pollBudgetMs` (PR #2507 documents this; here, make `genie omni handshake`/a doctor check warn if the installed hook timeout is too low). Provide a `genie omni test-approval` helper that drives ONE approval round-trip end-to-end (so future QA is a single command, not ad-hoc scripts that spam). +- **Tests:** correlated-resolution unit tests (two concurrent pending approvals; a reply/reaction tagged to id B resolves B, not the older A); reaction-subject integration test with a fake transport on the verified subject; resolution-feedback test (approve → ✅ status emitted, row expired). All with injectable transport — zero network. + +### OUT +- Reworking the hook/dispatch layer or the global-db schema beyond adding/【using】the existing `omni_message_id` column. +- Multi-approval batching / a full interactive TUI in WhatsApp (buttons, lists) — a later wish if wanted. +- Reconnecting the offline `pessoal-whatsapp` instance (a QR scan — Felipe's manual action; not code). +- Sending live WhatsApp messages during automated tests (fake transport only; live QA is the single `genie omni test-approval` helper, run deliberately). +- Changing the approve/deny vocabulary or the timeout→ask fail-safe (both proven correct). + +## Decisions + +| # | Decision | Rationale | +|---|----------|-----------| +| 1 | Correlate resolution by the real Omni message id, retire `resolveOldest` as the primary path | Live QA confirmed the oldest-pending fallback resolves the WRONG approval under concurrency; the human answered a specific message | +| 2 | Group 1 is a SPIKE — verify the real reaction subject + the id-returning send path before building | The live QA disproved the `omni.event.>` assumption and showed `omni send` returns a messageId; the design must be grounded in this Omni build's actual contract, not the v4-ported assumptions | +| 3 | Reactions become first-class; text replies stay as fallback | Reactions are the low-friction, non-spammy approve/deny gesture the operator wants; text is the compatibility path | +| 4 | One approval message + one status update; never re-announce | Anti-spam: the human sees a prompt and its outcome, not a wall of repeated prompts. The per-approval dedup already prevents re-fires | +| 5 | Ship a `genie omni test-approval` one-command harness | The QA spam came from ad-hoc multi-iteration scripts; a single deliberate command makes future testing safe and repeatable | + +## Success Criteria + +- [ ] G1 spike: `.genie/wishes/omni-approval-ux/SPIKE.md` documents (with evidence from the real hub) the reaction NATS subject + payload shape AND the send path that yields a correlatable message id; verdict on API-send vs NATS-correlate. +- [ ] Correlated resolution: two concurrent pending approvals — a reply/reaction answering the 2nd resolves the 2nd (not the 1st); `omni_message_id` populated from the real send. Tested. +- [ ] Reaction approve/deny resolves on the verified subject, correlated to the right approval; instance-scoped (PR #2507 guard). Tested. +- [ ] Resolution feedback: on approve/deny the original message gets a ✅/❌ (react or status) and the row is expired/closed; no lingering open prompt. Tested. +- [ ] `genie omni test-approval` drives one clean round-trip (against a fake transport in CI; usable live for one deliberate message). +- [ ] Full `bun run check` green; no live messages sent by the automated suite. + +## Execution Strategy + +| Wave | Groups | Notes | +|------|--------|-------| +| 1 | Group 1 (spike) | Grounds the design in the real Omni contract; gates G2/G3 | +| 2 | Group 2, Group 3 | Correlated identity + reactions ∥ resolution feedback + test harness (touch disjoint areas: queue/runner-resolve vs runner-outbound/CLI) | + +--- + +## Execution Group 1: Spike — reaction subject + correlatable send id +**Goal:** Replace the disproven `omni.event.>` + oldest-pending assumptions with the real contract of this Omni build. + +**Deliverables:** +1. Against the live hub (via `ssh felipe`, MINIMAL messages — one send, subscribe-and-observe for reactions): determine (a) the exact NATS subject + payload omni publishes when a WhatsApp user REACTS to a message, and (b) whether the Omni HTTP send API (`/api/v1/...`) returns a stable `messageId` that later inbound replies/reactions reference. Use `omni --json` + a bounded NATS subscriber (omni's nats module) — do NOT run ad-hoc approval loops that spam. +2. `.genie/wishes/omni-approval-ux/SPIKE.md`: the reaction subject + payload shape, the id-returning send path, and a GO recommendation for Group 2 (API-send vs NATS-correlate) with the exact fields genie must store/match. + +**Acceptance Criteria:** +- [ ] SPIKE.md documents the reaction subject + payload and the correlatable-send decision, evidence-backed. +- [ ] No spam: the spike sends at most one or two clearly-labelled messages between Felipe's own numbers. + +**Validation:** +```bash +set -euo pipefail +cd "$(git rev-parse --show-toplevel)" +test -f .genie/wishes/omni-approval-ux/SPIKE.md +grep -qiE 'reaction.*subject|omni\.[a-z]+' .genie/wishes/omni-approval-ux/SPIKE.md +grep -qiE 'messageId|correlat' .genie/wishes/omni-approval-ux/SPIKE.md +``` + +**depends-on:** none + +--- + +## Execution Group 2: Correlated identity + reaction approve/deny +**Goal:** A reply/reaction resolves the exact approval it answers; 👍/👎 works. + +**Deliverables:** +1. Capture the real Omni message id on send (per the G1 verdict) and store it via `attachOmniMessageId` immediately, so `handleMessage`/`handleEvent` correlate by `omni_message_id` first and only fall back to oldest-pending when no id matches. +2. Subscribe the VERIFIED reaction subject (from G1) instead of the assumed `omni.event.>`; resolve 👍/👎 correlated to the stored id; keep the PR #2507 instance-scope guard. +3. Tests (fake transport): two concurrent pending approvals resolved to the correct one by id; reaction on the verified subject resolves the correlated approval; text fallback still works. + +**Acceptance Criteria:** +- [ ] Concurrent approvals resolve by id, not oldest; reactions resolve on the verified subject; instance-scoped. +- [ ] typecheck + `bun test src/lib/omni-runner.test.ts src/lib/v5/omni-queue.test.ts` green. + +**Validation:** +```bash +set -euo pipefail +cd "$(git rev-parse --show-toplevel)" +bun test src/lib/omni-runner.test.ts src/lib/v5/omni-queue.test.ts +bun run typecheck +``` + +**depends-on:** group-1 + +--- + +## Execution Group 3: Resolution feedback + one-command test harness +**Goal:** A decided approval stops looking open; future QA is one safe command. + +**Deliverables:** +1. On resolve/expire, emit a resolution signal to the original thread — react ✅/❌ on the approval message (via the omni react path / API) and/or a one-line status; expire/close the row so no stale prompt lingers. +2. `genie omni test-approval` command: drives one approval round-trip (enqueue → announce → resolve) against an injectable transport by default (CI-safe, no network); with `--live` it runs ONE real round-trip between the configured instance/approvalChat (deliberate, single message). +3. `genie doctor`/handshake warning when the installed CC hook `timeout` is below `pollBudgetMs` (the operator guardrail the QA proved essential). +4. Tests: resolution-feedback emitted + row closed; `test-approval` fake path green; doctor warning fires on a too-low timeout. + +**Acceptance Criteria:** +- [ ] Approve/deny yields a ✅/❌ thread signal and closes the row; no lingering prompt (tested). +- [ ] `genie omni test-approval` (fake) green in CI; `--live` documented. +- [ ] doctor/handshake warns on an inadequate hook timeout. +- [ ] Full `bun run check` green. + +**Validation:** +```bash +set -euo pipefail +cd "$(git rev-parse --show-toplevel)" +bun test src/lib/omni-runner.test.ts src/term-commands/omni.test.ts +bun run check +bun run build +bun dist/genie.js omni test-approval 2>&1 | grep -qiE 'approved|allow|round-trip' || { echo "FAIL: test-approval harness"; exit 1; } +``` + +**depends-on:** group-1 + +--- + +## Cross-wish dependencies + +- **Builds on** the live-validated `omni-runner-port` bridge + the `omni-hardening` (PR #2507) instance-scope + approval-budget-doc fixes. +- **Records** the 2026-07-03 live QA outcome (allow path proven end-to-end; deny code-verified; personal instance offline; reactions unconfirmed) — this wish closes the reaction + correlation gaps that QA exposed. From 37e457f8b06406c4e909d19c67487d98ca842f66 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 01:06:20 -0300 Subject: [PATCH 05/18] feat(mcp): auto-register genie mcp into Warp + Claude Code configs 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. --- .genie/wishes/genie-mcp/WISH.md | 6 +- README.md | 21 +++++ src/term-commands/init.test.ts | 81 ++++++++++++++++++ src/term-commands/init.ts | 141 ++++++++++++++++++++++++++++++-- src/term-commands/launch.ts | 4 + 5 files changed, 245 insertions(+), 8 deletions(-) diff --git a/.genie/wishes/genie-mcp/WISH.md b/.genie/wishes/genie-mcp/WISH.md index 12a15da73..355e8d6ed 100644 --- a/.genie/wishes/genie-mcp/WISH.md +++ b/.genie/wishes/genie-mcp/WISH.md @@ -128,9 +128,9 @@ echo "$OUT" | grep -q '"result"' || { echo "FAIL: mcp initialize no result"; exi 3. Tests: idempotent merge test — empty case AND a pre-populated `.mcp.json` with ANOTHER server (assert the other server survives + byte-identical rerun); `genie init` in a fixture repo registers `genie mcp` in both files. **Acceptance Criteria:** -- [ ] `genie init` writes/merges both files registering `genie mcp`; a pre-existing MCP server is preserved; rerun is byte-identical. -- [ ] Docs updated; the tab-info limitation stated honestly. -- [ ] Full `bun run check` green. +- [x] `genie init` writes/merges both files registering `genie mcp`; a pre-existing MCP server is preserved; rerun is byte-identical. +- [x] Docs updated; the tab-info limitation stated honestly. +- [x] Full `bun run check` green. **Validation:** ```bash diff --git a/README.md b/README.md index bb8cb40c2..934cf0b7e 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,27 @@ All linked worktrees of a repository share one `genie.db`, resolved from the git - Approval-gated agents launched with `--permission-mode default`. Under `auto` mode a passthrough `ask` can auto-resolve to allow, which defeats the timeout→ask fail-safe. - `genie omni serve` running as the one resident process. It is the *only* NATS client — `--help`, `task`, `board`, and every other command stay transport-free (`nats` never initializes on those paths). +## MCP server (Warp + Claude Code) + +`genie mcp` is a zero-dependency, read-only [MCP](https://modelcontextprotocol.io) server over stdio. It lets the AI agent inside a Warp pane or Claude Code session query live board and wish state without shelling out to the CLI. + +**How it gets picked up.** `genie init` registers the server into two project-scope config files, and `genie launch` writes the same pair into every worktree it opens: + +- `.mcp.json` — Claude Code's project MCP config. Project-scope servers are *pending approval* until you trust the workspace (accept the trust dialog in an interactive `claude` session) — expected, not a bug. +- `.warp/.mcp.json` — Warp auto-detects this on save (no restart) and lists `genie` under Settings → AI/Agents → MCP servers. + +Both files use the identical `mcpServers` shape and are merged idempotently: re-running `genie init` preserves every other server and top-level key and rewrites byte-identical. The registered `command` is the **absolute path to the running genie binary** (resolved from `process.execPath`), not bare `genie` — genie is not reliably on PATH (on macOS it lives only at `~/.genie/bin/genie`). Because `genie init`/`launch` run on the box that owns the repo, the recorded path is correct even under Warp's SSH-remote feature, where Warp spawns the server on that same box. + +**What it exposes** — five read-only tools backed by the per-repo `.genie/genie.db`: + +- `genie_board` — board counts + tasks (optional wish filter) +- `genie_wish_status` — a wish's group/DAG progress +- `genie_worktree_context` — resolves the pane's `wish/-` branch to its wish, group, and tasks (the per-pane "what am I here for") +- `genie_task` — full task detail by id +- `genie_active` — every in-progress task and who claimed it + +**Honest limitation — genie does not push into your tabs.** Warp exposes no external tab-push API, so genie cannot inject state into a pane. The flow is pull, not push: the pane's agent *asks* genie over MCP (`genie_worktree_context`, `genie_board`, …) when it wants to know the board state. `genie launch` still seeds each pane with a kickoff prompt at open time, but ongoing awareness is the agent querying the MCP server, not genie writing into the tab. + ## Roadmap No dates — direction, not promises: diff --git a/src/term-commands/init.test.ts b/src/term-commands/init.test.ts index e393f2a05..9991bf6ea 100644 --- a/src/term-commands/init.test.ts +++ b/src/term-commands/init.test.ts @@ -154,4 +154,85 @@ describe('genie init', () => { expect(second.gitignore).toBe('skipped'); expect(second.rulesAdded).toEqual([]); }); + + describe('MCP server registration', () => { + const mcpPath = (root: string) => join(root, '.mcp.json'); + const warpMcpPath = (root: string) => join(root, '.warp', '.mcp.json'); + + test('fresh repo: writes both .mcp.json and .warp/.mcp.json with the genie entry', () => { + initGitRepo(dir); + expect(runInit(dir).code).toBe(0); + + for (const path of [mcpPath(dir), warpMcpPath(dir)]) { + expect(existsSync(path)).toBe(true); + const servers = JSON.parse(readFileSync(path, 'utf-8')).mcpServers; + expect(servers.genie).toBeDefined(); + expect(servers.genie.args).toEqual(['mcp']); + // Absolute command resolved from the running executable — never bare "genie". + expect(servers.genie.command).not.toBe('genie'); + expect(servers.genie.command.startsWith('/')).toBe(true); + } + }); + + test('pre-populated .mcp.json: preserves the other server and adds genie', () => { + initGitRepo(dir); + writeFileSync(mcpPath(dir), '{"mcpServers":{"other":{"command":"x"}}}'); + + expect(runInit(dir).code).toBe(0); + + const servers = JSON.parse(readFileSync(mcpPath(dir), 'utf-8')).mcpServers; + expect(servers.other).toEqual({ command: 'x' }); + expect(servers.genie).toBeDefined(); + expect(servers.genie.args).toEqual(['mcp']); + }); + + test('preserves other top-level keys and an alternate wrapper key', () => { + initGitRepo(dir); + // Existing file uses the `servers` wrapper + carries an unrelated top-level key. + writeFileSync(mcpPath(dir), '{"$schema":"./s.json","servers":{"other":{"command":"x"}}}'); + + expect(runInit(dir).code).toBe(0); + + const parsed = JSON.parse(readFileSync(mcpPath(dir), 'utf-8')); + expect(parsed.$schema).toBe('./s.json'); + // Whichever wrapper key already held servers is preserved — no new mcpServers key. + expect(parsed.mcpServers).toBeUndefined(); + expect(parsed.servers.other).toEqual({ command: 'x' }); + expect(parsed.servers.genie).toBeDefined(); + }); + + test('rerun is byte-identical for both fresh and pre-populated configs', () => { + initGitRepo(dir); + writeFileSync(mcpPath(dir), '{"mcpServers":{"other":{"command":"x"}}}'); + expect(runInit(dir).code).toBe(0); + + const mcpBefore = readFileSync(mcpPath(dir)); + const warpBefore = readFileSync(warpMcpPath(dir)); + + expect(runInit(dir).code).toBe(0); + + expect(readFileSync(mcpPath(dir)).equals(mcpBefore)).toBe(true); + expect(readFileSync(warpMcpPath(dir)).equals(warpBefore)).toBe(true); + }); + + test('--json reports the mcp config writes as created then skipped', () => { + initGitRepo(dir); + const first = JSON.parse(runInit(dir, ['--json']).stdout); + const actions = first.mcp.map((c: { path: string; action: string }) => c.action); + expect(actions).toEqual(['created', 'created']); + + const second = JSON.parse(runInit(dir, ['--json']).stdout); + expect(second.mcp.map((c: { action: string }) => c.action)).toEqual(['skipped', 'skipped']); + }); + + test('malformed .mcp.json is surfaced, not clobbered', () => { + initGitRepo(dir); + writeFileSync(mcpPath(dir), 'not json {'); + const { code, stderr } = runInit(dir); + expect(code).toBe(1); + expect(stderr).toContain('.mcp.json'); + // The bad file is left untouched. + expect(readFileSync(mcpPath(dir), 'utf-8')).toBe('not json {'); + }); + }); }); diff --git a/src/term-commands/init.ts b/src/term-commands/init.ts index 96d2031c7..f61027c0b 100644 --- a/src/term-commands/init.ts +++ b/src/term-commands/init.ts @@ -1,10 +1,12 @@ /** * genie init — idempotent per-repo scaffold. * - * Bootstraps the two things a fresh repo needs before the genie lifecycle can - * run: the plans jar (`.genie/INDEX.md`) and the `.gitignore` rules that keep - * the SQLite state files out of version control. Both steps are idempotent — - * re-running `genie init` on an already-scaffolded repo produces zero diff. + * Bootstraps the things a fresh repo needs before the genie lifecycle can run: + * the plans jar (`.genie/INDEX.md`), the `.gitignore` rules that keep the + * SQLite state files out of version control, and the MCP server registration + * that lets Warp + Claude Code discover the read-only `genie mcp` server. Every + * step is idempotent — re-running `genie init` on an already-scaffolded repo + * produces zero diff. * * No network, no daemon, no database. Refuses politely outside a git repo. */ @@ -95,6 +97,7 @@ interface InitResult { index: ArtifactAction; gitignore: ArtifactAction; rulesAdded: string[]; + mcp: McpConfigResult[]; } /** Create `.genie/INDEX.md` with the jar skeleton if it does not exist. */ @@ -126,6 +129,123 @@ function scaffoldGitignore(root: string): { action: ArtifactAction; added: strin return { action: exists ? 'updated' : 'created', added: missing }; } +// ============================================================================ +// MCP server registration (.mcp.json + .warp/.mcp.json) +// ============================================================================ + +/** + * A stdio MCP server entry, as both Warp and Claude Code read it. `type:"stdio"` + * is optional (a `command` present defaults to stdio), so it is omitted. + */ +interface McpServerEntry { + command: string; + args: string[]; +} + +/** Arbitrary JSON object (an already-parsed config we merge into). */ +type JsonObject = Record; + +/** + * Wrapper keys Warp recognizes for the server map, in order of preference. + * `mcpServers` is what we write for a fresh file; if an existing file already + * holds its servers under one of the alternates, that key is preserved. + */ +const MCP_WRAPPER_KEYS = ['mcpServers', 'mcp_servers', 'servers'] as const; + +/** + * The genie server entry written under the `genie` key. The command is the + * ABSOLUTE path to the currently-running genie executable — for the shipped + * bun-compiled single-file binary that is {@link process.execPath}. Bare + * `"genie"` is NOT used: genie is not reliably on the spawning process's PATH + * (on macOS it lives only at `~/.genie/bin/genie`). Resolving at write time is + * self-consistent under Warp-over-SSH — `genie init` runs on the box that owns + * the repo, so it records that box's genie path, and Warp spawns there too. + */ +function genieMcpEntry(): McpServerEntry { + return { command: process.execPath, args: ['mcp'] }; +} + +/** True for a plain (non-array) JSON object. */ +function isJsonObject(value: unknown): value is JsonObject { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Parse an existing config's raw bytes into an object, or start from `{}` when + * the file is absent. A file that exists but is not a JSON object is a real + * problem worth surfacing (we must not silently clobber it), so it throws. + */ +function parseMcpConfig(raw: string | null, path: string): JsonObject { + if (raw === null || raw.trim() === '') return {}; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error(`Cannot register genie MCP server: ${path} is not valid JSON.`); + } + if (!isJsonObject(parsed)) { + throw new Error(`Cannot register genie MCP server: ${path} is not a JSON object.`); + } + return parsed; +} + +/** + * Locate the server map inside a parsed config, preserving whatever wrapper key + * an existing file already uses (`mcpServers`, `mcp_servers`, `servers`, or a + * nested `mcp.servers`). When none is present the `mcpServers` key is created — + * the preferred shape for a new file. + */ +function locateServerMap(config: JsonObject): JsonObject { + for (const key of MCP_WRAPPER_KEYS) { + const existing = config[key]; + if (isJsonObject(existing)) return existing; + } + const nested = config.mcp; + if (isJsonObject(nested) && isJsonObject(nested.servers)) return nested.servers; + + const created: JsonObject = {}; + config.mcpServers = created; + return created; +} + +/** + * Merge the genie MCP server entry into the config at `configPath`, creating the + * file (and its parent dir) when absent. Only the `genie` key under the server + * map is added/updated — every other server AND every other top-level key is + * preserved. Serialization is deterministic (2-space JSON + trailing newline) + * so a rerun that changes nothing is byte-identical and reports `skipped`. + */ +function mergeMcpConfig(configPath: string, entry: McpServerEntry): ArtifactAction { + const raw = existsSync(configPath) ? readFileSync(configPath, 'utf-8') : null; + const config = parseMcpConfig(raw, configPath); + const servers = locateServerMap(config); + servers.genie = entry; + + const serialized = `${JSON.stringify(config, null, 2)}\n`; + if (raw !== null && raw === serialized) return 'skipped'; + + mkdirSync(dirname(configPath), { recursive: true }); + writeFileSync(configPath, serialized); + return raw === null ? 'created' : 'updated'; +} + +interface McpConfigResult { + path: string; + action: ArtifactAction; +} + +/** + * Register the `genie mcp` server into both project-scope config files under + * `root`: `/.mcp.json` (Claude Code) and `/.warp/.mcp.json` (Warp). + * Both get the identical entry; the merge is idempotent and preserves existing + * servers. Exported so `genie launch` can register the same server per worktree. + */ +export function registerMcpConfigs(root: string): McpConfigResult[] { + const entry = genieMcpEntry(); + const targets = [join(root, '.mcp.json'), join(root, '.warp', '.mcp.json')]; + return targets.map((path) => ({ path, action: mergeMcpConfig(path, entry) })); +} + // ============================================================================ // Reporting // ============================================================================ @@ -134,6 +254,11 @@ function actionLabel(action: ArtifactAction): string { return action === 'skipped' ? 'already present' : action; } +/** Short, repo-relative label for an MCP config path (`.mcp.json`, `.warp/.mcp.json`). */ +function mcpConfigLabel(configPath: string): string { + return configPath.endsWith(join('.warp', '.mcp.json')) ? '.warp/.mcp.json' : '.mcp.json'; +} + function printHumanReport(result: InitResult): void { out('Initialized genie in this repository.'); out(''); @@ -143,6 +268,11 @@ function printHumanReport(result: InitResult): void { } else { out(` .gitignore ${actionLabel(result.gitignore)}`); } + for (const cfg of result.mcp) { + out(` ${mcpConfigLabel(cfg.path)} ${actionLabel(cfg.action)}`); + } + out(''); + out('Warp + Claude Code will discover the read-only `genie mcp` server from those files.'); out(''); out('Next steps — the plan/work lifecycle (run inside Claude Code):'); out(' /brainstorm Explore a fuzzy idea into a DESIGN.md'); @@ -168,7 +298,8 @@ function handleInit(opts: InitOptions): void { const index = scaffoldIndex(root); const gitignore = scaffoldGitignore(root); - const result: InitResult = { root, index, gitignore: gitignore.action, rulesAdded: gitignore.added }; + const mcp = registerMcpConfigs(root); + const result: InitResult = { root, index, gitignore: gitignore.action, rulesAdded: gitignore.added, mcp }; if (opts.json) { out(JSON.stringify(result, null, 2)); diff --git a/src/term-commands/launch.ts b/src/term-commands/launch.ts index d1486bcb4..b74739cd5 100644 --- a/src/term-commands/launch.ts +++ b/src/term-commands/launch.ts @@ -26,6 +26,7 @@ import { openDb } from '../lib/v5/genie-db.js'; import { type TaskRow, listTasks } from '../lib/v5/task-state.js'; import { type PaneSpec, buildLaunchConfigYaml, launchUri, writeLaunchConfig } from '../lib/v5/warp-launch.js'; import { genieHome } from '../lib/workspace.js'; +import { registerMcpConfigs } from './init.js'; // ============================================================================ // Typed errors @@ -524,6 +525,9 @@ function materialize(plan: LaunchPlan, deps: LaunchDeps, write: (line: string) = const action = ensureWorktree(plan.repoRoot, group, write); mkdirSync(dirname(group.promptPath), { recursive: true }); writeFileSync(group.promptPath, group.prompt, 'utf-8'); + // Register the read-only `genie mcp` server in this worktree so the pane's + // agent (Warp/Claude Code) can query board + wish state. Idempotent + merges. + registerMcpConfigs(group.worktree); write(` ${group.name} → ${group.worktree} [${action}]`); } From d5d23270211acc442812dcba79e8697f0641fed6 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 01:09:05 -0300 Subject: [PATCH 06/18] =?UTF-8?q?feat(mcp):=20genie=20mcp=20stdio=20server?= =?UTF-8?q?=20=E2=80=94=20Warp=20+=20Claude=20Code=20read=20genie=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/-), 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. --- .genie/wishes/genie-mcp/WISH.md | 16 +- src/genie.ts | 2 + src/lib/interactivity.ts | 4 + src/lib/v5/mcp-tools.ts | 280 ++++++++++++++++++++++++ src/term-commands/mcp.test.ts | 369 ++++++++++++++++++++++++++++++++ src/term-commands/mcp.ts | 180 ++++++++++++++++ 6 files changed, 843 insertions(+), 8 deletions(-) create mode 100644 src/lib/v5/mcp-tools.ts create mode 100644 src/term-commands/mcp.test.ts create mode 100644 src/term-commands/mcp.ts diff --git a/.genie/wishes/genie-mcp/WISH.md b/.genie/wishes/genie-mcp/WISH.md index 355e8d6ed..122fbdfe7 100644 --- a/.genie/wishes/genie-mcp/WISH.md +++ b/.genie/wishes/genie-mcp/WISH.md @@ -2,7 +2,7 @@ | Field | Value | |-------|-------| -| **Status** | DRAFT | +| **Status** | DONE — G1 spike (hand-rolled) + G2 server + G3 auto-register all SHIP-reviewed (2026-07-03); Warp live-UI QA awaits Felipe | | **Slug** | `genie-mcp` | | **Date** | 2026-07-03 | | **Author** | Felipe + Genie | @@ -44,11 +44,11 @@ Genie's live state (wishes, tasks, board, what each worktree/pane is working on) ## Success Criteria -- [ ] `genie mcp` answers MCP `initialize` + `tools/list` + `tools/call` over stdio and returns real genie.db state (board / wish_status / worktree_context) — automated protocol test; opens the db read-only and degrades to an empty board when absent. -- [ ] `genie init` JSON-merge-writes idempotent `/.warp/.mcp.json` + `/.mcp.json` registering `genie mcp` (byte-identical on rerun; a pre-existing MCP server is preserved). +- [x] `genie mcp` answers MCP `initialize` + `tools/list` + `tools/call` over stdio and returns real genie.db state (board / wish_status / worktree_context) — automated protocol test; opens the db read-only and degrades to an empty board when absent. +- [x] `genie init` JSON-merge-writes idempotent `/.warp/.mcp.json` + `/.mcp.json` registering `genie mcp` (byte-identical on rerun; a pre-existing MCP server is preserved). - [ ] A real Warp AND a real Claude Code session connect to `genie mcp` and read genie state (manual QA recorded in qa.md). -- [ ] Non-mcp paths (`genie board`/`task`/`--help`) do not initialize the MCP transport/dep (startup probe). -- [ ] Full `bun run check` + build green; CI green on the PR. +- [x] Non-mcp paths (`genie board`/`task`/`--help`) do not initialize the MCP transport/dep (startup probe). +- [x] Full `bun run check` + build green; CI green on the PR. ## Execution Strategy @@ -94,9 +94,9 @@ grep -qiE 'command' .genie/wishes/genie-mcp/SPIKE.md 4. Tests: an MCP-protocol test (spawn `genie mcp`, drive initialize → tools/list → tools/call, assert real genie.db state); a readonly + absent-db test; a non-mcp init probe (the MCP transport/dep is NOT loaded on `board`/`task`/`--help`). **Acceptance Criteria:** -- [ ] `genie mcp` answers initialize/tools/list/tools/call over stdio with real genie.db state; opens read-only; degrades to empty board on absent db. -- [ ] Non-mcp paths don't initialize the MCP transport/dep (probe test). -- [ ] typecheck + `bun test` for the mcp module + build green; `--help` lists `mcp`. +- [x] `genie mcp` answers initialize/tools/list/tools/call over stdio with real genie.db state; opens read-only; degrades to empty board on absent db. +- [x] Non-mcp paths don't initialize the MCP transport/dep (probe test). +- [x] typecheck + `bun test` for the mcp module + build green; `--help` lists `mcp`. **Validation:** ```bash diff --git a/src/genie.ts b/src/genie.ts index 8b5104b29..7e3c4744b 100644 --- a/src/genie.ts +++ b/src/genie.ts @@ -25,6 +25,7 @@ import { installWorkspaceCheck } from './lib/interactivity.js'; import { VERSION } from './lib/version.js'; import { registerInitCommand } from './term-commands/init.js'; import { registerLaunchCommand } from './term-commands/launch.js'; +import { registerMcpCommand } from './term-commands/mcp.js'; import { registerOmniCommands } from './term-commands/omni.js'; import { registerV5BoardCommands } from './term-commands/v5-board.js'; import { registerV5TaskCommands } from './term-commands/v5-task.js'; @@ -124,6 +125,7 @@ registerHookNamespace(program); registerInitCommand(program); registerLaunchCommand(program); +registerMcpCommand(program); registerV5TaskCommands(program); registerV5BoardCommands(program); registerOmniCommands(program); diff --git a/src/lib/interactivity.ts b/src/lib/interactivity.ts index 6acbcc6f3..919907dc4 100644 --- a/src/lib/interactivity.ts +++ b/src/lib/interactivity.ts @@ -70,6 +70,10 @@ const WORKSPACE_EXEMPT = new Set([ // `task`/`board` it self-resolves the global genie.db and never reads the // v4 workspace.json, so gating it on the legacy workspace concept is wrong. 'omni', + // `mcp` is the read-only stdio MCP server. It opens the shared genie.db + // read-only and DEGRADES to an empty board when the file is absent, so the + // legacy workspace gate must not exit 2 before the JSON-RPC loop even starts. + 'mcp', ]); /** diff --git a/src/lib/v5/mcp-tools.ts b/src/lib/v5/mcp-tools.ts new file mode 100644 index 000000000..d74342189 --- /dev/null +++ b/src/lib/v5/mcp-tools.ts @@ -0,0 +1,280 @@ +/** + * Genie v5 MCP tools — the read-only projection of `.genie/genie.db` exposed + * over the hand-rolled stdio MCP server (see `src/term-commands/mcp.ts`). + * + * This module is intentionally LAZY-LOADED: `genie mcp` dynamic-imports it + * inside the command body so that non-mcp code paths (`genie board`, `genie + * task`, `genie --help`) never touch the read-only `bun:sqlite` open here. The + * import-graph probe in `mcp.test.ts` locks that contract. + * + * The DB is opened NET-NEW and READ-ONLY (`new Database(path, {readonly:true})`) + * — deliberately NOT `openSqlite()`/`openDb()`, which force-create the file and + * run write pragmas. An absent db (readonly open throws) degrades to `null`, and + * every tool renders an empty board rather than erroring. + */ + +import { Database } from 'bun:sqlite'; +import { execFileSync } from 'node:child_process'; +import { resolveDbPath } from './genie-db.js'; +import { + type TaskFilter, + type TaskRow, + type WishGroupRow, + getBoardByName, + getTask, + getWishGroups, + listTasks, +} from './task-state.js'; + +// ============================================================================ +// Read-only DB open (net-new; degrade to null when the file is absent) +// ============================================================================ + +/** + * Open the repo's shared `.genie/genie.db` READ-ONLY. Returns `null` when the + * file does not exist (readonly open of a missing file throws) so callers can + * degrade to an empty board instead of crashing the server. The handle is the + * caller's to close. + */ +export function openReadonlyDb(cwd?: string): Database | null { + try { + return new Database(resolveDbPath(cwd), { readonly: true }); + } catch { + return null; + } +} + +// ============================================================================ +// Payload shapes +// ============================================================================ + +export interface TaskSummary { + id: string; + title: string; + status: TaskRow['status']; + claimedBy: string | null; + wish: string | null; + group: string | null; +} + +function toSummary(t: TaskRow): TaskSummary { + return { id: t.id, title: t.title, status: t.status, claimedBy: t.claimedBy, wish: t.wish, group: t.group }; +} + +interface StatusCounts { + blocked: number; + ready: number; + in_progress: number; + done: number; + total: number; +} + +function tally(tasks: TaskRow[]): StatusCounts { + const counts: StatusCounts = { blocked: 0, ready: 0, in_progress: 0, done: 0, total: 0 }; + for (const t of tasks) { + counts[t.status]++; + counts.total++; + } + return counts; +} + +// ============================================================================ +// Git branch resolution (for genie_worktree_context) +// ============================================================================ + +/** Current git branch of `cwd`, or `null` when unavailable (detached / not a repo). */ +function currentBranch(cwd: string): string | null { + try { + const branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { + cwd, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + return branch && branch !== 'HEAD' ? branch : null; + } catch { + return null; + } +} + +/** + * Parse a `wish/-` branch into `{ wish, group }`. The group is the + * final `-`-delimited segment; the slug is everything before it (slugs may + * themselves contain hyphens, e.g. `wish/genie-mcp-g2` → `genie-mcp` / `g2`). + * Returns `null` when the branch is not a wish worktree branch. + */ +function parseWishBranch(branch: string): { wish: string; group: string } | null { + const rest = branch.startsWith('wish/') ? branch.slice('wish/'.length) : null; + if (!rest) return null; + const dash = rest.lastIndexOf('-'); + if (dash <= 0 || dash === rest.length - 1) return null; + return { wish: rest.slice(0, dash), group: rest.slice(dash + 1) }; +} + +// ============================================================================ +// Tool context + registry +// ============================================================================ + +export interface ToolContext { + /** Read-only handle, or `null` when the db is absent (degrade to empty). */ + db: Database | null; + /** Working directory for git branch resolution. Defaults to `process.cwd()`. */ + cwd: string; +} + +export interface McpTool { + name: string; + description: string; + inputSchema: Record; + handler(ctx: ToolContext, args: Record): unknown; +} + +function argString(args: Record, key: string): string | undefined { + const v = args[key]; + return typeof v === 'string' && v.length > 0 ? v : undefined; +} + +// --- genie_board ----------------------------------------------------------- + +interface BoardPayload { + board: string | null; + counts: StatusCounts; + tasks: TaskSummary[]; +} + +function genieBoard(ctx: ToolContext, args: Record): 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) }; +} + +// --- genie_wish_status ----------------------------------------------------- + +interface WishStatusPayload { + wish: string; + groups: Array>; + tasks: TaskSummary[]; +} + +function genieWishStatus(ctx: ToolContext, args: Record): WishStatusPayload { + const wish = argString(args, 'wish') ?? ''; + if (!ctx.db) return { wish, groups: [], tasks: [] }; + const groups = getWishGroups(ctx.db, wish).map(({ wish: _w, ...rest }) => rest); + const tasks = listTasks(ctx.db, { wish }); + return { wish, groups, tasks: tasks.map(toSummary) }; +} + +// --- genie_worktree_context ------------------------------------------------ + +interface WorktreeContextPayload { + branch: string | null; + resolved: boolean; + wish: string | null; + group: string | null; + tasks: TaskSummary[]; +} + +function genieWorktreeContext(ctx: ToolContext, args: Record): WorktreeContextPayload { + const branch = argString(args, 'branch') ?? currentBranch(ctx.cwd); + const parsed = branch ? parseWishBranch(branch) : null; + + if (parsed) { + const tasks = ctx.db ? listTasks(ctx.db, { wish: parsed.wish }).filter((t) => t.group === parsed.group) : []; + return { branch, resolved: true, wish: parsed.wish, group: parsed.group, tasks: tasks.map(toSummary) }; + } + + // Non-wish branch (or none) → repo-board fallback: all tasks, unresolved. + const tasks = ctx.db ? listTasks(ctx.db, {}) : []; + return { branch, resolved: false, wish: null, group: null, tasks: tasks.map(toSummary) }; +} + +// --- genie_task ------------------------------------------------------------ + +function genieTask(ctx: ToolContext, args: Record): TaskRow | { error: 'not_found'; id: string } { + const id = argString(args, 'id') ?? ''; + const task = ctx.db ? getTask(ctx.db, id) : null; + return task ?? { error: 'not_found', id }; +} + +// --- genie_active ---------------------------------------------------------- + +interface ActiveTask extends TaskSummary { + claimedAt: number | null; +} + +function genieActive(ctx: ToolContext): { tasks: ActiveTask[] } { + if (!ctx.db) return { tasks: [] }; + const tasks = listTasks(ctx.db, { status: 'in_progress' }); + return { tasks: tasks.map((t) => ({ ...toSummary(t), claimedAt: t.claimedAt })) }; +} + +// ============================================================================ +// The 5 read-only tools +// ============================================================================ + +export const MCP_TOOLS: McpTool[] = [ + { + name: 'genie_board', + description: 'Board status counts and tasks; optional board name and wish-slug filters.', + inputSchema: { + type: 'object', + properties: { + board: { type: 'string', description: 'board name; default repo board' }, + wish: { type: 'string', description: 'filter to a wish slug' }, + }, + required: [], + }, + handler: genieBoard, + }, + { + name: 'genie_wish_status', + description: "A wish's execution groups (DAG progress) and its tasks.", + inputSchema: { + type: 'object', + properties: { wish: { type: 'string', description: 'wish slug' } }, + required: ['wish'], + }, + handler: genieWishStatus, + }, + { + name: 'genie_worktree_context', + description: + "Resolve a wish/- git branch to its wish, group, and tasks (the pane's 'what am I here for').", + inputSchema: { + type: 'object', + properties: { branch: { type: 'string', description: 'override; default = current git branch' } }, + required: [], + }, + handler: genieWorktreeContext, + }, + { + name: 'genie_task', + description: 'Full detail for a single task by id.', + inputSchema: { + type: 'object', + properties: { id: { type: 'string', description: 'task id, e.g. t_...' } }, + required: ['id'], + }, + handler: genieTask, + }, + { + name: 'genie_active', + description: 'All in-progress tasks and who claimed each.', + inputSchema: { type: 'object', properties: {}, required: [] }, + handler: (ctx) => genieActive(ctx), + }, +]; diff --git a/src/term-commands/mcp.test.ts b/src/term-commands/mcp.test.ts new file mode 100644 index 000000000..972f133ef --- /dev/null +++ b/src/term-commands/mcp.test.ts @@ -0,0 +1,369 @@ +/** + * genie mcp — CLI-level tests. Drives the real `genie.ts mcp` stdio server as a + * subprocess (as an MCP client would), speaking newline-delimited JSON-RPC 2.0, + * against throwaway git-repo fixtures seeded via the real v5 state layer. + * + * Also asserts the LAZY-LOAD contract with a static import-graph probe: the + * read-only `bun:sqlite` open in `mcp-tools.ts` must NOT be reachable from + * `genie.ts` except through the dynamic `import()` inside the `mcp` action. + */ + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { openDb } from '../lib/v5/genie-db.js'; +import { createBoard, createTask, createWishGroups } from '../lib/v5/task-state.js'; + +const GENIE = join(import.meta.dir, '..', 'genie.ts'); +const SRC_ROOT = resolve(import.meta.dir, '..'); + +// ============================================================================ +// Fixtures +// ============================================================================ + +let repo: string; + +function git(cwd: string, ...args: string[]): void { + execFileSync('git', args, { + cwd, + stdio: 'ignore', + env: { + ...process.env, + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@example.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@example.com', + }, + }); +} + +beforeEach(() => { + repo = mkdtempSync(join(tmpdir(), 'genie-mcp-')); + git(repo, 'init', '-b', 'main'); + git(repo, 'commit', '--allow-empty', '-m', 'init'); +}); + +afterEach(() => { + rmSync(repo, { recursive: true, force: true }); +}); + +// ============================================================================ +// JSON-RPC driver — write all requests, close stdin, collect response lines. +// ============================================================================ + +interface RpcResponse { + jsonrpc: '2.0'; + id: number | string | null; + result?: Record; + error?: { code: number; message: string }; +} + +async function driveMcp(cwd: string, requests: Record[]): Promise { + const proc = Bun.spawn(['bun', GENIE, 'mcp'], { + cwd, + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + env: { ...process.env, NO_COLOR: '1', GENIE_TEST_SKIP_PGSERVE: '1' }, + }); + const payload = `${requests.map((r) => JSON.stringify(r)).join('\n')}\n`; + proc.stdin.write(payload); + await proc.stdin.end(); + const stdout = await new Response(proc.stdout).text(); + await proc.exited; + return stdout + .split('\n') + .filter((l) => l.trim().length > 0) + .map((l) => JSON.parse(l) as RpcResponse); +} + +const INIT = { + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 't', version: '0' } }, +}; +const INITIALIZED = { jsonrpc: '2.0', method: 'notifications/initialized' }; + +/** Parse the JSON text payload out of a tools/call result envelope. */ +function toolPayload(res: RpcResponse): T { + const content = (res.result?.content as Array<{ type: string; text: string }>) ?? []; + return JSON.parse(content[0].text) as T; +} + +function seed(cwd: string): { taskId: string } { + const db = openDb({ cwd }); + const board = createBoard(db, 'repo'); + const t = createTask(db, { title: 'seed task', boardId: board.id, wish: 'genie-mcp', group: 'g2' }); + createTask(db, { title: 'other', boardId: board.id }); + createWishGroups(db, 'genie-mcp', [{ name: 'g1' }, { name: 'g2', dependsOn: ['g1'] }]); + db.close(); + return { taskId: t.id }; +} + +// ============================================================================ +// Handshake +// ============================================================================ + +describe('mcp handshake', () => { + test('initialize replies with protocolVersion, tools capability, and serverInfo', async () => { + const [res] = await driveMcp(repo, [INIT]); + expect(res.id).toBe(1); + expect(res.result?.protocolVersion).toBe('2024-11-05'); + expect(res.result?.capabilities).toEqual({ tools: {} }); + const serverInfo = res.result?.serverInfo as { name: string; version: string }; + expect(serverInfo.name).toBe('genie'); + expect(typeof serverInfo.version).toBe('string'); + }); + + test('notifications/initialized gets NO reply', async () => { + // Two ids (init, ping) + the notification → exactly two responses. + const responses = await driveMcp(repo, [INIT, INITIALIZED, { jsonrpc: '2.0', id: 2, method: 'ping' }]); + expect(responses.map((r) => r.id)).toEqual([1, 2]); + }); + + test('unknown method with an id → JSON-RPC -32601', async () => { + const responses = await driveMcp(repo, [INIT, { jsonrpc: '2.0', id: 9, method: 'bogus/method' }]); + const bogus = responses.find((r) => r.id === 9); + expect(bogus?.error?.code).toBe(-32601); + }); +}); + +// ============================================================================ +// tools/list +// ============================================================================ + +describe('mcp tools/list', () => { + test('lists exactly the 5 read-only tools with input schemas', async () => { + const responses = await driveMcp(repo, [INIT, INITIALIZED, { jsonrpc: '2.0', id: 2, method: 'tools/list' }]); + const list = responses.find((r) => r.id === 2); + const tools = list?.result?.tools as Array<{ name: string; inputSchema: unknown }>; + expect(tools.map((t) => t.name).sort()).toEqual([ + 'genie_active', + 'genie_board', + 'genie_task', + 'genie_wish_status', + 'genie_worktree_context', + ]); + for (const t of tools) expect(t.inputSchema).toBeDefined(); + }); +}); + +// ============================================================================ +// tools/call — real state +// ============================================================================ + +describe('mcp tools/call', () => { + test('genie_board reflects real seeded db state', async () => { + seed(repo); + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { jsonrpc: '2.0', id: 3, method: 'tools/call', params: { name: 'genie_board', arguments: {} } }, + ]); + const res = responses.find((r) => r.id === 3); + expect(res?.result?.isError).toBe(false); + const payload = toolPayload<{ counts: { total: number; ready: number }; tasks: Array<{ wish: string }> }>(res!); + expect(payload.counts.total).toBe(2); + expect(payload.counts.ready).toBe(2); + expect(payload.tasks.some((t) => t.wish === 'genie-mcp')).toBe(true); + }); + + test('genie_wish_status returns the group DAG and tasks', async () => { + seed(repo); + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { + jsonrpc: '2.0', + id: 4, + method: 'tools/call', + params: { name: 'genie_wish_status', arguments: { wish: 'genie-mcp' } }, + }, + ]); + const payload = toolPayload<{ wish: string; groups: Array<{ name: string; status: string; dependsOn: string[] }> }>( + responses.find((r) => r.id === 4)!, + ); + expect(payload.wish).toBe('genie-mcp'); + expect(payload.groups.map((g) => g.name).sort()).toEqual(['g1', 'g2']); + const g2 = payload.groups.find((g) => g.name === 'g2'); + expect(g2?.dependsOn).toEqual(['g1']); + expect(g2?.status).toBe('blocked'); + }); + + test('genie_task returns full detail by id, or not_found', async () => { + const { taskId } = seed(repo); + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { jsonrpc: '2.0', id: 5, method: 'tools/call', params: { name: 'genie_task', arguments: { id: taskId } } }, + { jsonrpc: '2.0', id: 6, method: 'tools/call', params: { name: 'genie_task', arguments: { id: 't_nope' } } }, + ]); + const found = toolPayload<{ id: string; title: string }>(responses.find((r) => r.id === 5)!); + expect(found.id).toBe(taskId); + expect(found.title).toBe('seed task'); + const missing = toolPayload<{ error: string; id: string }>(responses.find((r) => r.id === 6)!); + expect(missing).toEqual({ error: 'not_found', id: 't_nope' }); + }); + + test('genie_worktree_context resolves a wish/- branch', async () => { + seed(repo); + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { + jsonrpc: '2.0', + id: 7, + method: 'tools/call', + params: { name: 'genie_worktree_context', arguments: { branch: 'wish/genie-mcp-g2' } }, + }, + ]); + const payload = toolPayload<{ resolved: boolean; wish: string; group: string; tasks: Array<{ group: string }> }>( + responses.find((r) => r.id === 7)!, + ); + expect(payload.resolved).toBe(true); + expect(payload.wish).toBe('genie-mcp'); + expect(payload.group).toBe('g2'); + expect(payload.tasks.every((t) => t.group === 'g2')).toBe(true); + }); + + test('genie_worktree_context falls back to unresolved on a non-wish branch', async () => { + seed(repo); + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { + jsonrpc: '2.0', + id: 8, + method: 'tools/call', + params: { name: 'genie_worktree_context', arguments: { branch: 'main' } }, + }, + ]); + const payload = toolPayload<{ resolved: boolean; wish: null; tasks: unknown[] }>( + responses.find((r) => r.id === 8)!, + ); + expect(payload.resolved).toBe(false); + expect(payload.wish).toBeNull(); + expect(payload.tasks.length).toBe(2); // repo-board fallback: all tasks + }); + + test('genie_active lists in-progress tasks with claimant', async () => { + const { taskId } = seed(repo); + // Claim the seed task so it becomes in_progress. + const db = openDb({ cwd: repo }); + db.query("UPDATE tasks SET status='in_progress', claimed_by='worker-1', claimed_at=? WHERE id=?").run( + Date.now(), + taskId, + ); + db.close(); + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { jsonrpc: '2.0', id: 10, method: 'tools/call', params: { name: 'genie_active', arguments: {} } }, + ]); + const payload = toolPayload<{ tasks: Array<{ id: string; claimedBy: string; status: string }> }>( + responses.find((r) => r.id === 10)!, + ); + expect(payload.tasks).toHaveLength(1); + expect(payload.tasks[0].id).toBe(taskId); + expect(payload.tasks[0].claimedBy).toBe('worker-1'); + expect(payload.tasks[0].status).toBe('in_progress'); + }); + + test('unknown tool name → isError result (not a protocol error)', async () => { + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { jsonrpc: '2.0', id: 11, method: 'tools/call', params: { name: 'genie_nope', arguments: {} } }, + ]); + const res = responses.find((r) => r.id === 11); + expect(res?.error).toBeUndefined(); + expect(res?.result?.isError).toBe(true); + const payload = toolPayload<{ error: string; name: string }>(res!); + expect(payload).toEqual({ error: 'unknown_tool', name: 'genie_nope' }); + }); +}); + +// ============================================================================ +// Absent-db degrade +// ============================================================================ + +describe('mcp absent-db degrade', () => { + test('genie_board degrades to an empty board when genie.db is absent', async () => { + // Fresh repo, never seeded → no .genie/genie.db. Readonly open throws → null. + const responses = await driveMcp(repo, [ + INIT, + INITIALIZED, + { jsonrpc: '2.0', id: 12, method: 'tools/call', params: { name: 'genie_board', arguments: {} } }, + ]); + const res = responses.find((r) => r.id === 12); + expect(res?.result?.isError).toBe(false); + const payload = toolPayload<{ board: null; counts: { total: number }; tasks: unknown[] }>(res!); + expect(payload.counts.total).toBe(0); + expect(payload.tasks).toEqual([]); + }); +}); + +// ============================================================================ +// Lazy-load probe — mcp-tools (the readonly bun:sqlite open) must NOT be in the +// STATIC import graph reachable from genie.ts. It is only `await import`-ed +// inside the `mcp` action, so non-mcp paths (board/task/--help) never load it. +// ============================================================================ + +/** Value (non-type) static import/re-export specifiers in a source file. */ +function staticValueImports(file: string): string[] { + const src = readFileSync(file, 'utf-8'); + const specs: string[] = []; + // `import ... from ''` and `export ... from ''`, skipping the + // type-only forms (`import type` / `export type`) which are erased at runtime. + const re = /(?:^|\n)\s*(?:import|export)\s+(type\s+)?[^;'"]*?from\s+['"]([^'"]+)['"]/g; + let m: RegExpExecArray | null; + // biome-ignore lint/suspicious/noAssignInExpressions: standard regex exec loop + while ((m = re.exec(src)) !== null) { + if (m[1]) continue; // `type` import — erased, no runtime load + specs.push(m[2]); + } + // Bare side-effect imports: `import ''`. + const sideEffect = /(?:^|\n)\s*import\s+['"]([^'"]+)['"]/g; + // biome-ignore lint/suspicious/noAssignInExpressions: standard regex exec loop + while ((m = sideEffect.exec(src)) !== null) specs.push(m[1]); + return specs; +} + +/** Files transitively reachable via STATIC value imports from `entry`. */ +function reachableFrom(entry: string): Set { + const seen = new Set(); + const stack = [entry]; + while (stack.length > 0) { + const file = stack.pop() as string; + if (seen.has(file)) continue; + seen.add(file); + for (const spec of staticValueImports(file)) { + if (!spec.startsWith('.')) continue; // external / node: — not our source graph + const target = resolve(dirname(file), spec.replace(/\.js$/, '.ts')); + stack.push(target); + } + } + return seen; +} + +describe('mcp lazy-load probe', () => { + test('mcp-tools.ts is NOT statically reachable from genie.ts', () => { + const reachable = reachableFrom(join(SRC_ROOT, 'genie.ts')); + const mcpTools = join(SRC_ROOT, 'lib', 'v5', 'mcp-tools.ts'); + expect(reachable.has(mcpTools)).toBe(false); + // Sanity: the lightweight registration module IS eagerly loaded (expected). + expect(reachable.has(join(SRC_ROOT, 'term-commands', 'mcp.ts'))).toBe(true); + }); + + test('mcp.ts loads bun:sqlite / mcp-tools only via dynamic import()', () => { + const src = readFileSync(join(SRC_ROOT, 'term-commands', 'mcp.ts'), 'utf-8'); + // No static value import of the tools module or bun:sqlite. + expect(src).not.toMatch(/(?:^|\n)\s*import\s+\{[^}]*\}\s+from\s+['"]\.\.\/lib\/v5\/mcp-tools/); + expect(src).not.toMatch(/from\s+['"]bun:sqlite['"]/); + // The tools ARE reached, via a dynamic import inside the action. + expect(src).toMatch(/await import\(['"]\.\.\/lib\/v5\/mcp-tools\.js['"]\)/); + }); +}); diff --git a/src/term-commands/mcp.ts b/src/term-commands/mcp.ts new file mode 100644 index 000000000..9439b078f --- /dev/null +++ b/src/term-commands/mcp.ts @@ -0,0 +1,180 @@ +/** + * genie mcp — a hand-rolled, zero-dependency stdio MCP server exposing the v5 + * `.genie/genie.db` state READ-ONLY. + * + * Transport (per SPIKE.md, verdict "hand-rolled"): newline-delimited JSON-RPC + * 2.0 — one JSON object per line on stdin/stdout. NOT LSP `Content-Length` + * framing. Speaks MCP protocol `2024-11-05`, confirmed against real Claude Code + * and Warp's bundled schema. + * + * LAZY-LOAD contract: this module statically imports ONLY commander types and + * the version string. The `bun:sqlite` read-only open and the tool + * implementations live in `../lib/v5/mcp-tools.js`, which is `await import`-ed + * inside the command action — so `genie board`/`task`/`--help` never load it. + * `mcp.test.ts` locks this via an import-graph probe. + * + * The stdio protocol writes (`process.stdout.write`) ARE the server's output by + * design — not stray logging — so they satisfy biome's no-console rule. + */ + +import { createInterface } from 'node:readline'; +import type { Command } from 'commander'; +// Type-only import: erased at compile time, so it does NOT load mcp-tools (and +// its bun:sqlite open) at genie startup — the runtime load stays in the action. +import type { ToolContext } from '../lib/v5/mcp-tools.js'; +import { VERSION } from '../lib/version.js'; + +// ============================================================================ +// JSON-RPC 2.0 message shapes +// ============================================================================ + +interface JsonRpcRequest { + jsonrpc: '2.0'; + id?: number | string | null; + method: string; + params?: Record; +} + +interface JsonRpcResponse { + jsonrpc: '2.0'; + id: number | string | null; + result?: unknown; + error?: { code: number; message: string }; +} + +const PROTOCOL_VERSION = '2024-11-05'; + +// JSON-RPC standard error codes. +const METHOD_NOT_FOUND = -32601; +const INTERNAL_ERROR = -32603; + +function writeMessage(msg: JsonRpcResponse): void { + // Newline-delimited: exactly one JSON object per line, no embedded newlines. + process.stdout.write(`${JSON.stringify(msg)}\n`); +} + +function ok(id: number | string | null, result: unknown): void { + writeMessage({ jsonrpc: '2.0', id, result }); +} + +function err(id: number | string | null, code: number, message: string): void { + writeMessage({ jsonrpc: '2.0', id, error: { code, message } }); +} + +// ============================================================================ +// Server run loop +// ============================================================================ + +/** + * Drive the stdio MCP server until stdin closes. Opens a NET-NEW read-only db + * handle (degrading to an empty board when the file is absent) and dispatches + * each newline-delimited JSON-RPC message. + */ +export async function runMcpServer(): Promise { + // Lazy: the read-only bun:sqlite open + tools load here, not at genie startup. + const { MCP_TOOLS, openReadonlyDb } = await import('../lib/v5/mcp-tools.js'); + const cwd = process.cwd(); + const db = openReadonlyDb(cwd); + const ctx: ToolContext = { db, cwd }; + + const toolByName = new Map(MCP_TOOLS.map((t) => [t.name, t] as const)); + + function handleToolsCall(id: number | string | null, params: Record | undefined): void { + const name = typeof params?.name === 'string' ? params.name : ''; + const args = (params?.arguments as Record | undefined) ?? {}; + const tool = toolByName.get(name); + if (!tool) { + // Surface as an isError tool result (not a protocol error) per MCP. + ok(id, { + content: [{ type: 'text', text: JSON.stringify({ error: 'unknown_tool', name }) }], + isError: true, + }); + return; + } + const payload = tool.handler(ctx, args); + ok(id, { + content: [{ type: 'text', text: JSON.stringify(payload) }], + structuredContent: payload, + isError: false, + }); + } + + function dispatch(req: JsonRpcRequest): void { + // A message with no id (or null id) is a notification: process side effects + // but send NO reply (e.g. notifications/initialized). + const isNotification = req.id === undefined || req.id === null; + const id = req.id ?? null; + + switch (req.method) { + case 'initialize': + ok(id, { + protocolVersion: PROTOCOL_VERSION, + capabilities: { tools: {} }, + serverInfo: { name: 'genie', version: VERSION }, + }); + return; + case 'ping': + if (!isNotification) ok(id, {}); + return; + case 'tools/list': + if (!isNotification) { + ok(id, { + tools: MCP_TOOLS.map((t) => ({ name: t.name, description: t.description, inputSchema: t.inputSchema })), + }); + } + return; + case 'tools/call': + if (!isNotification) handleToolsCall(id, req.params); + return; + default: + // Notifications (e.g. notifications/initialized, notifications/*) get no + // reply. An unknown method WITH an id is a JSON-RPC method-not-found. + if (!isNotification) err(id, METHOD_NOT_FOUND, `Method not found: ${req.method}`); + return; + } + } + + 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)); + } + } + + const rl = createInterface({ input: process.stdin, crlfDelay: Number.POSITIVE_INFINITY }); + rl.on('line', handleLine); + + await new Promise((resolve) => { + rl.on('close', () => { + db?.close(); + // Flush stdout BEFORE exiting: the empty-write callback fires after the + // stream's buffer (including all prior response writes) drains to the OS, + // so the final response is never truncated. See SPIKE.md flush note. + process.stdout.write('', () => resolve()); + }); + }); +} + +// ============================================================================ +// Registration (the single line in genie.ts calls this) +// ============================================================================ + +export function registerMcpCommand(program: Command): void { + program + .command('mcp') + .description('Run a read-only stdio MCP server exposing genie.db task/board state') + .action(async () => { + await runMcpServer(); + }); +} From 37ed79eb4760727ddee2e52d16cbc3ec694c1c3a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 3 Jul 2026 04:15:02 +0000 Subject: [PATCH 07/18] chore(version): bump to 5.260703.3 [auto-version] --- .claude-plugin/marketplace.json | 2 +- package.json | 2 +- plugins/genie/.claude-plugin/plugin.json | 2 +- plugins/genie/package.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 7cd086aee..48d0f1081 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "genie", - "version": "5.260703.2", + "version": "5.260703.3", "source": "./plugins/genie", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, wish them into plans, make with parallel agents, ship as one team. A coding genie that grows with your project." } diff --git a/package.json b/package.json index e969f22f8..c6ea8276e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@automagik/genie", - "version": "5.260703.2", + "version": "5.260703.3", "description": "Collaborative terminal toolkit for human + AI workflows. NOTE: npm distribution discontinued 2026-05-09 — install via `curl -fsSL https://raw.githubusercontent.com/automagik-dev/genie/main/install.sh | bash` (cosign + SLSA verified). See https://automagik.dev/genie/release-process", "type": "module", "bin": { diff --git a/plugins/genie/.claude-plugin/plugin.json b/plugins/genie/.claude-plugin/plugin.json index 3b3f7e550..9b8f25afb 100644 --- a/plugins/genie/.claude-plugin/plugin.json +++ b/plugins/genie/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "genie", - "version": "5.260703.2", + "version": "5.260703.3", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, turn them into wishes, execute with /work, validate with /review, and ship as one team.", "author": { "name": "Namastex Labs" diff --git a/plugins/genie/package.json b/plugins/genie/package.json index edc3c0d23..da4b0e217 100644 --- a/plugins/genie/package.json +++ b/plugins/genie/package.json @@ -1,6 +1,6 @@ { "name": "genie-plugin", - "version": "5.260703.2", + "version": "5.260703.3", "private": true, "description": "Runtime dependencies for genie bundled CLIs", "type": "module", From 96cf5c300ea3d961c8525d1a7a703f2f3825fc1c Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 02:06:44 -0300 Subject: [PATCH 08/18] docs(omni-approval-ux): reviewed wish + G1 spike (SHIP) on current dev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .genie/wishes/omni-approval-ux/SPIKE.md | 133 ++++++++++++++++++++++++ .genie/wishes/omni-approval-ux/WISH.md | 96 +++++++++-------- 2 files changed, 184 insertions(+), 45 deletions(-) create mode 100644 .genie/wishes/omni-approval-ux/SPIKE.md diff --git a/.genie/wishes/omni-approval-ux/SPIKE.md b/.genie/wishes/omni-approval-ux/SPIKE.md new file mode 100644 index 000000000..8f4f8d476 --- /dev/null +++ b/.genie/wishes/omni-approval-ux/SPIKE.md @@ -0,0 +1,133 @@ +# SPIKE — Omni Approval UX contract verification (Group 1) + +| Field | Value | +|-------|-------| +| **Wish** | `omni-approval-ux` | +| **Group** | G1 (SPIKE) — gates G2 + G3 | +| **Date** | 2026-07-03 | +| **Method** | Source-of-truth grounding in the LIVE Omni build (`~/prod/omni`, running as PID 291967) + running NATS (`nats-server -js`, PID 291945, `:4222`) reached via `ssh felipe`. Zero live WhatsApp messages sent (see "Live messaging" below). | +| **Omni build** | bun monorepo at `/home/genie/prod/omni`; genie-facing bridge = `packages/core/src/providers/nats-genie-provider.ts`; reaction dispatch = `packages/api/src/plugins/agent-dispatcher.ts`; WhatsApp channel = `packages/channel-whatsapp/src/plugin.ts` + `handlers/messages.ts`. | + +## TL;DR verdicts + +| # | Unknown | Verdict | +|---|---------|---------| +| (a) | Inbound reaction subject + payload | **`omni.message.{instance}.{chatId}`** (the SAME subject genie already subscribes for text) — NOT `omni.event.>` (that subject has ZERO publishers in this build → the disproven v4 assumption is now root-caused). Reaction arrives as content string **`[Reaction: on message ]`** with the target id also in the top-level `messageId` field. | +| (b) | Id-returning send path | Channel-level send (`omni send --json`) returns **`SendResult.messageId`** = the WhatsApp **stanza id / externalId** (e.g. `3EB097217C5450F5E0166D`). That is the correlatable id. Genie's CURRENT announce path (NATS publish on `omni.reply.*`) does NOT capture it — `onReply` discards the send result. G2 must switch announce to an id-returning send. | +| (c) | Outbound set-reaction (⏳→✅) | **GO.** `omni send --reaction --message ` (and `omni react`) exist; `sendReaction(..., fromMe)` supports reacting to genie's OWN sent message (`fromMe` defaults `true`). Emoji swaps in place (WhatsApp = one reaction per sender per message; new emoji replaces prior). Fallback documented below in case live QA later contradicts the in-place swap. | +| (d) | Inbound text-reply quoted id | **NOT available to genie today.** Omni HAS the quoted id server-side (`replyToExternalId` on the event), but the genie NATS provider payload (`NatsOutboundMessage`) drops it — no `replyTo` field is forwarded. → bare text stays **oldest-pending fallback** (matches the wish's OUT-scope). Enabling text correlation would require an Omni provider change, out of this wish's scope. | + +--- + +## (a) Inbound reaction subject + payload — EVIDENCE + +**Subject: `omni.message.{instanceId}.{chatId}` — the same subscription as text.** + +Root cause of the QA disproof of `omni.event.>`: +``` +$ grep -rn "omni.event" ~/prod/omni/packages/*/src ~/prod/omni/apps/*/src +# → only comments / omni_events DB table references. NO nats publish to omni.event.* anywhere. +``` +`omni.event.>` is a **dead subject** in this build. Genie's `omni-runner.ts:506` subscribes `omni.event.>` and its `handleEvent` (`omni-runner.ts:424-452`) therefore receives NOTHING — exactly what the live QA saw. + +**How a reaction actually reaches genie** (traced through source): + +1. WhatsApp/Baileys reaction → `handlers/messages.ts:176-180` extracts `targetMessageId = m.reactionMessage.key.id` (the WhatsApp **stanza id / externalId** of the reacted-to message). +2. `plugin.ts:3044 handleReactionReceived(...)` → `emitReactionReceived({ messageId: targetMessageId, chatId, from, emoji, rawPayload:{ externalId, isFromMe } })`. So **`ReactionReceivedPayload.messageId` = the reacted-to message's externalId** (type doc: `packages/core/src/events/types.ts:595` — *"The message being reacted to"*). +3. `agent-dispatcher.ts:4159 processReactionTrigger` builds an `AgentTrigger` with `type:'reaction'`, `source.messageId = payload.messageId`, `content = { emoji, referencedMessageId: payload.messageId }`, and calls the `NatsGenieProvider`. +4. `nats-genie-provider.ts:135` publishes to **`omni.message.${instanceId}.${chatId}`** with `buildMessage()` (`nats-genie-provider.ts:~405`) producing: + ``` + [Reaction: on message ] + ``` + and (if `prefixSenderName`, default true) prefixed `[]: `. The NATS payload also carries top-level **`messageId: `**. + +**Reference payload genie will receive on `omni.message.{instance}.{chat}` for a 👍 reaction:** +```json +{ + "content": "[Felipe Rosa]: [Reaction: 👍 on message 3EB097217C5450F5E0166D]", + "sender": "Felipe Rosa", + "instanceId": "506377b1-eb79-4ae3-abc1-80bd00986f6b", + "chatId": "5512982298888@s.whatsapp.net", + "messageId": "3EB097217C5450F5E0166D", + "agent": "", + "timestamp": "2026-07-03T...Z" +} +``` +The **emoji** and **target message id** are both recoverable: emoji + id parse out of `content` via `/\[Reaction: (.+?) on message (.+?)\]/`, and the target id is ALSO the top-level `messageId`. + +> **Dual-emit caveat for G2:** `plugin.ts:3078` — with `OMNI_DUAL_EMIT_REACTIONS !== 'false'` (default ON) and `!isFromMe`, a HUMAN reaction ALSO fires a second `message.received` (`content.type:'reaction'`, `text:`, `rawPayload.targetMessageId`). So one human 👍 can reach genie TWICE on `omni.message.*`: once as `[Reaction: …]` (reliable, id-bearing) and once as a bare emoji. G2 should key on the `[Reaction: … on message ]` form and ignore/dedupe the bare-emoji echo. Genie's OWN (`isFromMe`) status reactions do NOT dual-emit (guard at `plugin.ts:3078`). + +--- + +## (b) Id-returning send path — EVIDENCE + +- **`SendResult` schema** (`packages/core/src/schemas/message.ts:145`): `{ success: boolean, messageId?: string, error?: string, timestamp: number }`. +- At the **channel level** `SendResult.messageId` = the Baileys **stanza id / externalId**. Empirical prior send returned `3EB097217C5450F5E0166D` (stanza-id format, not a UUID) — surfaced by `omni send --json` (`packages/cli/src/commands/send.ts:103` `output.success('Message sent', result)`). +- The **persisted REST route** `POST /messages` (`routes/v2/messages.ts:855`) instead returns `messageId: message.id` = the Omni **UUID** of the persisted row. Two different id namespaces — pick deliberately. + +**Which id to store:** the **stanza id / externalId**, because inbound reactions reference `m.reactionMessage.key.id` = the same externalId (see (a)). Storing the externalId gives a direct **stanza-id ↔ stanza-id** string match on resolve. (If only the UUID were available, `getReactionTargetByOmniId` / `getByExternalId` can bridge — but the direct match is simpler and is what `omni send` already returns.) + +**Why the current announce path can't correlate today:** +- `omni-runner.ts:358 announce()` publishes on `omni.reply.${instance}.${approvalChat}` and stores a **local `genId()`** ref via `attachOmniMessageId` (`omni-runner.ts:367`) — a self-referential value matching nothing inbound. +- The reply is delivered by Omni's `onReply` handler (`agent-dispatcher.ts:3844`), which calls `sendTextMessage(...)` and **discards the returned messageId**. The real stanza id is never published back on any subject genie subscribes. + +**G2 requirement:** replace the NATS-`omni.reply`-publish announce with an **id-returning send** (Omni HTTP send API or `omni send`) so `announce()` captures `SendResult.messageId` (the stanza id) and stores THAT via the existing `attachOmniMessageId` → `omni_message_id` column (`global-db.ts:90`, `omni-queue.ts:217`). Match inbound reactions against it in `handleMessage` (reactions now arrive there, not `handleEvent`). + +--- + +## (c) Outbound set-reaction (⏳→✅) — GO / NO-GO + +### Verdict: **GO** — genie CAN set and swap a reaction on a message it sent. + +**Set-reaction API (both exist):** +- `omni send --instance --to --reaction --message ` (`cli/src/commands/send.ts:129-142`, HTTP `POST /messages` reaction branch, `routes/v2/messages.ts:555`). +- `omni react --message --chat --instance ` (verb form). + +**Own-message reaction is supported:** +- `plugin.ts:1469` (send-reaction path): `const fromMe = message.metadata?.fromMe ?? true;` then `plugin.ts:1487 sendReaction(sock, jid, targetMessageId, reactionEmoji, fromMe, targetParticipant)`. The reaction-target resolver (`routes/v2/messages.ts:108-125,147-169`) reads the target message's own `isFromMe`, so reacting to genie's OWN approval message resolves `fromMe=true` → correct Baileys reaction key. (Note: the bare `omni react` verb at `plugin.ts:1651` hardcodes `fromMe=false`; G3 should use the **`omni send --reaction`** / HTTP reaction path with correct `fromMe`, not the raw verb, when reacting to genie's own message.) + +**Swap-in-place (⏳→✅):** WhatsApp reactions are one-per-sender-per-message; sending a new emoji from the same sender REPLACES the prior one. So: set `⏳` on announce → later send `✅`/`❌` on the same `--message ` and it swaps in place. Reaction removal = react with empty emoji (`plugin.ts:1660 unreact` / `removeReaction`). + +**No feedback loop:** genie's own status reaction is `isFromMe=true` → dual-emit `message.received` is SKIPPED (`plugin.ts:3078`); and ⏳/✅/❌ are not in the approve/deny vocab, so they cannot self-resolve an approval. + +**Fallback (if live QA later shows the in-place swap does NOT hold on this WA/Baileys build):** +1. **Preferred fallback:** EDIT the sent approval message to prepend a status glyph (`⏳ …` → `✅ …`). WhatsApp/Baileys message-edit is supported by the channel; genie holds the stanza id needed to target the edit. +2. **Last-resort fallback:** send a single one-line status REPLY (`--reply-to `) — noisier, one extra message per state change; use only if neither reaction-swap nor edit works. + +The mechanism is source-proven; the ONLY unproven bit is the empirical in-place swap render, which the fallback covers. + +--- + +## (d) Inbound text-reply quoted id — EVIDENCE + +- Omni's event DOES carry the quoted target: an event has `replyToExternalId` / `replyToEventId` (seen on a live event via `omni events get … --json`; both null for non-reply messages). The channel populates it from Baileys' `contextInfo.stanzaId`. +- BUT the **genie-facing NATS payload drops it.** `NatsOutboundMessage` (`nats-genie-provider.ts:56-75`) has fields `content, sender, instanceId, chatId, messageId, …` and **NO `replyTo`/`quoted` field.** `trigger()` sets `messageId: context.source.messageId` (the reply's OWN id, not the quoted target) and never forwards the quoted id. + +**Consequence for G2:** a bare text reply that quotes an approval message arrives at genie with no quoted-id → genie **cannot correlate text by quoted id** with the current Omni build. Bare unquoted text (and quoted text alike) stays the **oldest-pending fallback** — exactly the wish's OUT-scope ("Correlating bare unquoted text replies… stays oldest-pending fallback"). Enabling quoted-text correlation is an Omni-side provider change (add `replyToMessageId` to `NatsOutboundMessage`), NOT in this wish. + +--- + +## Exact fields G2/G3 must STORE and MATCH + +**STORE (G2, in `announce()`):** +- The send's **`SendResult.messageId` = WhatsApp stanza id / externalId** (e.g. `3EB097217C5450F5E0166D`), captured from an **id-returning send** (Omni HTTP send / `omni send`), written via existing `attachOmniMessageId(db, appr.id, )` into `omni_message_id` (`global-db.ts:90`). Retire the `genId()` self-ref at `omni-runner.ts:367`. + +**MATCH (G2, resolve paths):** +- **Reactions** now arrive in `handleMessage` on `omni.message.${instance}.>` (NOT `handleEvent`/`omni.event.>`). Detect content `^(\[.*?\]: )?\[Reaction: (?.+?) on message (?.+?)\]$` (or use top-level `messageId`); map emoji via `matchReaction`; resolve the pending approval whose `omni_message_id === `; else oldest fallback. Keep the PR #2507 instance-scope guard (`instanceId === config.instance`). +- **Subscription change:** point the reaction handling at `omni.message.${instance}.>` (already subscribed); the `omni.event.>` subscription (`omni-runner.ts:506`) is dead and can be retired. +- **Text:** unconditional `resolveOldest` stays for bare text (no quoted id available — finding (d)). + +**STATUS ack (G3):** +- On announce: `omni send --reaction ⏳ --message ` (fromMe path). On approve: swap `✅`; on deny/expire: `❌`. Target = the same stored stanza id. Use the `omni send --reaction` / HTTP reaction path (correct `fromMe`), not the raw `omni react` verb. Fallback: message-edit prepend, then status-reply. + +--- + +## Live messaging + +**Live WhatsApp messages sent: 0.** One labelled test send (AI number → Felipe's personal number, DM) was attempted to empirically confirm (b) the returned field and (c) the in-place swap; the Claude Code auto-mode safety classifier correctly blocked it (a teammate-message is not the user's explicit intent for an outbound real-world message, and the recipient number was agent-inferred). Per the anti-spam directive I did **not** work around the block. All four findings above are **source-proven** against the live running build; the only item awaiting empirical confirmation is (c)'s in-place reaction swap render, which is covered by the documented NO-GO fallbacks. A single deliberate `--live` round-trip (via the G3 `genie omni test-approval --live` harness, or a user-approved `omni send`) can confirm it later. + +## GO/NO-GO summary +- (a) subject grounded → **GO** (build against `omni.message.*`, retire `omni.event.*`). +- (b) correlatable send id → **GO** (id-returning send; store the stanza id). +- (c) outbound set-reaction ⏳→✅ → **GO** (with message-edit / status-reply fallback documented). +- (d) text quoted-id → **NO** for this build → bare/quoted text stays oldest-pending **fallback** (as scoped). diff --git a/.genie/wishes/omni-approval-ux/WISH.md b/.genie/wishes/omni-approval-ux/WISH.md index 078ae67e9..6ebeecfa7 100644 --- a/.genie/wishes/omni-approval-ux/WISH.md +++ b/.genie/wishes/omni-approval-ux/WISH.md @@ -1,4 +1,4 @@ -# Wish: Omni Approval UX — Correlated Identity, Reactions, Anti-Spam +# Wish: Omni Approval UX — Correlated Identity, Reactions, ⏳→✅ Acks | Field | Value | |-------|-------| @@ -9,66 +9,71 @@ | **Appetite** | ~3-4 days | | **Branch** | `wish/omni-approval-ux` (from `dev`; PR to `dev`) | | **Design** | _No brainstorm — grounded in the live WhatsApp QA on 2026-07-03_ | -| **Depends on** | `omni-runner-port` (merged), `wish/omni-hardening` (PR #2507) | +| **Depends on** | `omni-runner-port` (merged), `omni-hardening` (PR #2507, merged) | ## Summary -The v5 omni approval bridge is live-proven (a PreToolUse hook → WhatsApp 🔔 → text `y` → allow envelope, verified end-to-end against the real Omni hub). But the live QA exposed three UX/correctness gaps: replies resolve the OLDEST pending approval rather than the one the human actually answered (concurrent-approval hazard), reactions (👍/👎) don't resolve because genie subscribes `omni.event.>` while this Omni build doesn't publish reactions there, and every approval is a fresh WhatsApp message with no resolution feedback (a stale "Approval Required" prompt lingers even after it's decided). This wish makes approvals correlated, reaction-driven, and self-updating. +The v5 omni approval bridge is live-proven (a PreToolUse hook → WhatsApp 🔔 → text `y` → allow envelope, verified end-to-end against the real Omni hub). But the live QA exposed UX/correctness gaps: replies resolve the OLDEST pending approval rather than the one the human answered (concurrent-approval hazard); reactions (👍/👎) don't reliably resolve; and an approval message gives the human NO feedback about its state — it looks unread until (and after) it's decided. This wish makes approvals **correlated** (a reaction — or a quoted reply — resolves the exact approval it targets, via the real Omni message id; bare text stays an oldest-pending fallback), **reaction-driven** (👍/👎 approve/deny), and **self-updating** via a **two-state ⏳→✅ ack**: genie sets a ⏳ (ampulheta) status reaction on its OWN approval message the moment it's sent, then swaps it to ✅ (approved) / ❌ (denied/expired) once resolved — so the human always sees a message's live state at a glance. ## Scope ### IN -- **Correlated approval identity (fixes the oldest-pending hazard):** capture the REAL Omni message id of each sent approval request and store it in `approvals.omni_message_id`, so a reply/reaction resolves the EXACT approval it answered — not `resolveOldest`. Today the runner publishes fire-and-forget to `omni.reply.{instance}.{chat}` and never learns Omni's message id, so it falls back to oldest-pending (a correctness bug the omni-runner-port reviews flagged as LOW, confirmed live). Decision needed (Group 1 spike): send the approval via the **Omni HTTP send API** (which returns a `messageId` — proven: `omni send` returned `3EB097217C5450F5E0166D`) vs. correlating the NATS-published id. Prefer the path that yields a stable id genie can match inbound replies/reactions against. -- **Reaction-based approve/deny:** verify the ACTUAL NATS subject this Omni build publishes reactions on (the live QA could not confirm `omni.event.>` carries them), subscribe the correct subject, and resolve 👍/👎 by correlating to the stored `omni_message_id` from the correlated-identity work. Text `y`/`n` replies remain a fallback. Honor the instance-scope guard already added in PR #2507. -- **Resolution feedback (anti-spam):** on resolve/expire, update the human's thread so a decided approval doesn't linger as an open prompt — e.g. react ✅/❌ on the original approval message (via `omni react`) and/or send a one-line status ("approved by you ✅"). One message per approval, plus one lightweight status signal — never a re-announce (the per-approval `omniMessageId` dedup at omni-runner.ts:360 already prevents re-fires; keep it). -- **Operator guardrails learned from the QA:** document + enforce that enabling approvals requires the CC hook `timeout` ≥ `pollBudgetMs` (PR #2507 documents this; here, make `genie omni handshake`/a doctor check warn if the installed hook timeout is too low). Provide a `genie omni test-approval` helper that drives ONE approval round-trip end-to-end (so future QA is a single command, not ad-hoc scripts that spam). -- **Tests:** correlated-resolution unit tests (two concurrent pending approvals; a reply/reaction tagged to id B resolves B, not the older A); reaction-subject integration test with a fake transport on the verified subject; resolution-feedback test (approve → ✅ status emitted, row expired). All with injectable transport — zero network. +- **Two-state ⏳→✅ acknowledgement (the headline UX):** the instant an approval request is sent, genie sets a ⏳ reaction on its OWN approval message (received & awaiting you); on resolve it swaps that reaction to ✅ (approved) / ❌ (denied), and on expiry to ❌/⌛. WhatsApp reactions are one-per-sender-per-message, so genie's status reaction swaps in place and coexists with the human's separate 👍/👎 decision reaction. This needs an OUTBOUND set-reaction capability (react to a message genie sent) that **does not exist today** — G1 spike verifies it (GO) or picks a fallback (NO-GO): editing the sent message to prepend a status glyph, or a one-line status reply. +- **Correlated approval identity (fixes the oldest-pending hazard):** the `omni_message_id` column + `attachOmniMessageId` already exist (`omni-queue.ts:217`, `global-db.ts:90`), but `announce()` stores genie's OWN local `genId()` ref (`omni-runner.ts:367`), not the real Omni message id — a self-referential illusion. Capture the REAL Omni id on send (per the G1 verdict) and store THAT, then make the inbound paths correlate by it first: `handleEvent` already correlates reactions by `omniMessageId` with an oldest fallback (`omni-runner.ts:444-451`) — extend the same id-first correlation to the text path (`handleMessage`, currently unconditional `resolveOldest` at `omni-runner.ts:421`). A reaction or a quoted reply resolves the exact approval; bare unquoted text keeps the oldest-pending fallback (documented, not claimed as correlated). +- **Reaction-based approve/deny:** verify the ACTUAL NATS subject this Omni build publishes inbound reactions on (the live QA could not confirm `omni.event.>`), subscribe the correct subject, and resolve 👍/👎 correlated to the stored real `omni_message_id`. Honor the instance-scope guard added in PR #2507. +- **Operator guardrails learned from the QA:** a `genie doctor` (`src/genie-commands/doctor.ts`)/handshake warning when the installed CC hook `timeout` is below `pollBudgetMs`. A `genie omni test-approval` helper that drives ONE approval round-trip end-to-end (so future QA is a single command, not ad-hoc scripts that spam). +- **Tests (injectable transport, zero network):** two concurrent pending approvals — a reaction/quoted reply tagged to id B resolves B, not the older A; the ⏳→✅/❌ status-reaction (or NO-GO fallback) lifecycle; reaction approve/deny on the verified subject; resolution closes the row; the `test-approval` fake path; the doctor timeout warning. ### OUT -- Reworking the hook/dispatch layer or the global-db schema beyond adding/【using】the existing `omni_message_id` column. -- Multi-approval batching / a full interactive TUI in WhatsApp (buttons, lists) — a later wish if wanted. +- Reworking the hook/dispatch layer or the global-db schema beyond USING the existing `omni_message_id` column (+ at most a small status-reaction bookkeeping field if the spike shows one is needed). +- Multi-approval batching / a full interactive TUI in WhatsApp (buttons, lists) — a later wish. - Reconnecting the offline `pessoal-whatsapp` instance (a QR scan — Felipe's manual action; not code). -- Sending live WhatsApp messages during automated tests (fake transport only; live QA is the single `genie omni test-approval` helper, run deliberately). +- Sending live WhatsApp messages during automated tests (fake transport only; live QA is the single `genie omni test-approval --live` helper, run deliberately, between Felipe's own numbers). - Changing the approve/deny vocabulary or the timeout→ask fail-safe (both proven correct). +- Correlating bare unquoted text replies (no quoted-message id → stays oldest-pending fallback; correlation is for reactions + quoted replies). ## Decisions | # | Decision | Rationale | |---|----------|-----------| -| 1 | Correlate resolution by the real Omni message id, retire `resolveOldest` as the primary path | Live QA confirmed the oldest-pending fallback resolves the WRONG approval under concurrency; the human answered a specific message | -| 2 | Group 1 is a SPIKE — verify the real reaction subject + the id-returning send path before building | The live QA disproved the `omni.event.>` assumption and showed `omni send` returns a messageId; the design must be grounded in this Omni build's actual contract, not the v4-ported assumptions | -| 3 | Reactions become first-class; text replies stay as fallback | Reactions are the low-friction, non-spammy approve/deny gesture the operator wants; text is the compatibility path | -| 4 | One approval message + one status update; never re-announce | Anti-spam: the human sees a prompt and its outcome, not a wall of repeated prompts. The per-approval dedup already prevents re-fires | -| 5 | Ship a `genie omni test-approval` one-command harness | The QA spam came from ad-hoc multi-iteration scripts; a single deliberate command makes future testing safe and repeatable | +| 1 | Two-state ⏳→✅ ack via genie's OWN swapping status reaction (fallback: message-edit / status line if no outbound-react API) | Felipe's explicit ask: hourglass on receipt, green check once answered. A single swapping reaction is the lowest-noise way to show live state; coexists with the human's 👍/👎. The capability is unverified, so G1 gates it with a NO-GO fallback | +| 2 | Correlate resolution by the REAL Omni message id; retire the self-referential local ref at `omni-runner.ts:367` | Live QA confirmed oldest-pending resolves the WRONG approval under concurrency; today `announce()` stores a genId() ref that matches nothing inbound. The real id is also the target for the status reaction | +| 3 | Group 1 is a SPIKE — verify (a) the inbound reaction subject, (b) the id-returning send path, (c) the OUTBOUND set-reaction capability, AND (d) whether inbound text replies carry a quoted-message id — before building | The live QA disproved `omni.event.>`; the ⏳→✅ ack depends on outbound-react existing; text correlation depends on a quoted id. Ground all four in this Omni build's actual contract with an explicit GO/NO-GO per capability | +| 4 | Reactions first-class (both directions); text replies stay as fallback | Genie's ⏳/✅/❌ status outbound, the human's 👍/👎 inbound. Bare text is the compatibility path (oldest-pending) | +| 5 | One approval message + in-place status; never re-announce | Anti-spam: the human sees a prompt and its live state, not a wall of prompts. The per-approval `omniMessageId` dedup already prevents re-fires | +| 6 | Ship a `genie omni test-approval` one-command harness | The QA spam came from ad-hoc multi-iteration scripts; one deliberate command makes future testing safe and repeatable | ## Success Criteria -- [ ] G1 spike: `.genie/wishes/omni-approval-ux/SPIKE.md` documents (with evidence from the real hub) the reaction NATS subject + payload shape AND the send path that yields a correlatable message id; verdict on API-send vs NATS-correlate. -- [ ] Correlated resolution: two concurrent pending approvals — a reply/reaction answering the 2nd resolves the 2nd (not the 1st); `omni_message_id` populated from the real send. Tested. -- [ ] Reaction approve/deny resolves on the verified subject, correlated to the right approval; instance-scoped (PR #2507 guard). Tested. -- [ ] Resolution feedback: on approve/deny the original message gets a ✅/❌ (react or status) and the row is expired/closed; no lingering open prompt. Tested. -- [ ] `genie omni test-approval` drives one clean round-trip (against a fake transport in CI; usable live for one deliberate message). -- [ ] Full `bun run check` green; no live messages sent by the automated suite. +- [ ] **G1 spike:** `.genie/wishes/omni-approval-ux/SPIKE.md` documents (evidence from the real hub) the inbound reaction NATS subject + payload, the send path that yields a correlatable message id, the outbound set-reaction capability (GO/NO-GO + the swap mechanism or the fallback), AND whether inbound text replies carry a quoted-message id — with the exact fields genie stores/matches. +- [ ] **⏳→✅ ack lifecycle:** on announce, the approval message carries a ⏳ status (reaction, or the NO-GO fallback); on approve it becomes ✅; on deny/expire ❌. Verified with a fake transport asserting the set-reaction/edit calls + target id. +- [ ] **Correlated resolution:** two concurrent pending approvals — a reaction/quoted reply answering the 2nd resolves the 2nd (not the 1st); `omni_message_id` holds the REAL send id. Tested. (Bare unquoted text still resolves oldest — documented fallback, also tested.) +- [ ] **Reaction approve/deny** resolves on the verified subject, correlated to the right approval; instance-scoped (PR #2507 guard). Text fallback still works. Tested. +- [ ] `genie omni test-approval` drives one clean round-trip (fake transport in CI; `--live` documented); `genie doctor`/handshake warns when the CC hook timeout < pollBudgetMs. +- [ ] Full `bun run check` green; NO live messages sent by the automated suite. ## Execution Strategy | Wave | Groups | Notes | |------|--------|-------| -| 1 | Group 1 (spike) | Grounds the design in the real Omni contract; gates G2/G3 | -| 2 | Group 2, Group 3 | Correlated identity + reactions ∥ resolution feedback + test harness (touch disjoint areas: queue/runner-resolve vs runner-outbound/CLI) | +| 1 | Group 1 (spike) | Grounds the design: inbound reaction subject + id-returning send + outbound set-reaction (GO/NO-GO) + text quoted-id. Gates all | +| 2 | Group 2 | Correlated identity + inbound reactions — captures the real send id G3 needs | +| 3 | Group 3 | ⏳→✅ status lifecycle + resolution close + `test-approval` + doctor. Depends on G2 (the ⏳ target is the real id G2 captures) | + +> Sequential (G1→G2→G3), not parallel: G3's status reaction targets the real Omni id that only G2 teaches the runner to capture, and both modify `announce()`/the resolve path in `omni-runner.ts` — parallel edits would conflict and G3 would react to a non-existent ref. --- -## Execution Group 1: Spike — reaction subject + correlatable send id -**Goal:** Replace the disproven `omni.event.>` + oldest-pending assumptions with the real contract of this Omni build. +## Execution Group 1: Spike — reaction subjects (in + out), correlatable send id, text quoted-id +**Goal:** Replace the disproven `omni.event.>` + self-referential-id assumptions, confirm genie can SET a reaction on a message it sent (or pick a fallback), and learn whether text replies carry a quoted id — using the real contract of this Omni build. **Deliverables:** -1. Against the live hub (via `ssh felipe`, MINIMAL messages — one send, subscribe-and-observe for reactions): determine (a) the exact NATS subject + payload omni publishes when a WhatsApp user REACTS to a message, and (b) whether the Omni HTTP send API (`/api/v1/...`) returns a stable `messageId` that later inbound replies/reactions reference. Use `omni --json` + a bounded NATS subscriber (omni's nats module) — do NOT run ad-hoc approval loops that spam. -2. `.genie/wishes/omni-approval-ux/SPIKE.md`: the reaction subject + payload shape, the id-returning send path, and a GO recommendation for Group 2 (API-send vs NATS-correlate) with the exact fields genie must store/match. +1. Against the live hub (via `ssh felipe`, MINIMAL messages — one/two clearly-labelled sends between Felipe's OWN numbers 1986780008↔12982298888, subscribe-and-observe for reactions): determine (a) the exact NATS subject + payload omni publishes when a WhatsApp user REACTS; (b) whether the Omni HTTP send API returns a stable `messageId` later inbound events reference; (c) the OUTBOUND set-reaction path — can genie set/change a reaction (⏳ then ✅) on a message it sent, and does the emoji swap in place? If NO, the fallback (message-edit prepend, or a status reply); (d) whether an inbound text reply payload carries a quoted/replied-to message id. Use `omni --json` + a bounded NATS subscriber — NO ad-hoc approval loops. +2. `.genie/wishes/omni-approval-ux/SPIKE.md`: the inbound reaction subject + payload, the id-returning send path, the outbound set-reaction GO/NO-GO + swap-or-fallback mechanism, the text quoted-id finding, and a GO recommendation for G2/G3 with the exact fields genie must store/match. **Acceptance Criteria:** -- [ ] SPIKE.md documents the reaction subject + payload and the correlatable-send decision, evidence-backed. -- [ ] No spam: the spike sends at most one or two clearly-labelled messages between Felipe's own numbers. +- [ ] SPIKE.md documents the inbound reaction subject + payload, the correlatable-send decision, the outbound set-reaction GO/NO-GO (+ fallback if NO-GO), and the text quoted-id finding — evidence-backed. +- [ ] No spam: at most one or two clearly-labelled messages between Felipe's own numbers. **Validation:** ```bash @@ -77,22 +82,23 @@ cd "$(git rev-parse --show-toplevel)" test -f .genie/wishes/omni-approval-ux/SPIKE.md grep -qiE 'reaction.*subject|omni\.[a-z]+' .genie/wishes/omni-approval-ux/SPIKE.md grep -qiE 'messageId|correlat' .genie/wishes/omni-approval-ux/SPIKE.md +grep -qiE 'set.?reaction|outbound react|GO/NO-GO|fallback' .genie/wishes/omni-approval-ux/SPIKE.md ``` **depends-on:** none --- -## Execution Group 2: Correlated identity + reaction approve/deny -**Goal:** A reply/reaction resolves the exact approval it answers; 👍/👎 works. +## Execution Group 2: Correlated identity + inbound reaction approve/deny +**Goal:** Store the REAL Omni send id and resolve a reaction/quoted reply to the exact approval it answers; 👍/👎 works. **Deliverables:** -1. Capture the real Omni message id on send (per the G1 verdict) and store it via `attachOmniMessageId` immediately, so `handleMessage`/`handleEvent` correlate by `omni_message_id` first and only fall back to oldest-pending when no id matches. -2. Subscribe the VERIFIED reaction subject (from G1) instead of the assumed `omni.event.>`; resolve 👍/👎 correlated to the stored id; keep the PR #2507 instance-scope guard. -3. Tests (fake transport): two concurrent pending approvals resolved to the correct one by id; reaction on the verified subject resolves the correlated approval; text fallback still works. +1. In `announce()` (`omni-runner.ts:358-370`), replace the self-referential local `genId()` ref stored at `omni-runner.ts:367` with the REAL Omni message id captured from the send (per the G1 verdict — API-send return or NATS-correlated), via the existing `attachOmniMessageId` (`omni-queue.ts:217`) into the existing `omni_message_id` column (`global-db.ts:90`). Do NOT re-add the column/helper — they exist. +2. Extend the id-first correlation already present in `handleEvent` (`omni-runner.ts:444-451`) to the text path `handleMessage` (`omni-runner.ts:421`, currently unconditional `resolveOldest`): correlate by the real `omni_message_id` when the inbound carries a quoted/replied id (per G1(d)); fall back to oldest only for bare unquoted text. Subscribe the VERIFIED inbound reaction subject (from G1) instead of the assumed `omni.event.>`; keep the PR #2507 instance-scope guard. +3. Tests (fake transport): two concurrent pending approvals resolved to the correct one by the real id; reaction on the verified subject resolves the correlated approval; a quoted reply resolves the correlated approval; bare text still resolves oldest. **Acceptance Criteria:** -- [ ] Concurrent approvals resolve by id, not oldest; reactions resolve on the verified subject; instance-scoped. +- [ ] Concurrent approvals resolve by the real id (reaction/quoted reply), not oldest; reactions resolve on the verified subject; bare text = oldest fallback; instance-scoped. - [ ] typecheck + `bun test src/lib/omni-runner.test.ts src/lib/v5/omni-queue.test.ts` green. **Validation:** @@ -107,17 +113,17 @@ bun run typecheck --- -## Execution Group 3: Resolution feedback + one-command test harness -**Goal:** A decided approval stops looking open; future QA is one safe command. +## Execution Group 3: ⏳→✅ status lifecycle + resolution close + test harness + doctor +**Goal:** Every approval shows its live state via a swapping status (reaction or NO-GO fallback); a decided approval closes; future QA is one safe command. **Deliverables:** -1. On resolve/expire, emit a resolution signal to the original thread — react ✅/❌ on the approval message (via the omni react path / API) and/or a one-line status; expire/close the row so no stale prompt lingers. -2. `genie omni test-approval` command: drives one approval round-trip (enqueue → announce → resolve) against an injectable transport by default (CI-safe, no network); with `--live` it runs ONE real round-trip between the configured instance/approvalChat (deliberate, single message). -3. `genie doctor`/handshake warning when the installed CC hook `timeout` is below `pollBudgetMs` (the operator guardrail the QA proved essential). -4. Tests: resolution-feedback emitted + row closed; `test-approval` fake path green; doctor warning fires on a too-low timeout. +1. **⏳→✅ ack lifecycle:** on announce, set a ⏳ status on the approval message targeting the REAL Omni id captured in G2 — via the outbound set-reaction path from G1 (GO), or the G1 fallback (message-edit / status line) if NO-GO; on resolve, swap to ✅ (approved) / ❌ (denied); on expiry, ❌/⌛. Close/expire the row so no stale open prompt lingers. Never re-announce. +2. `genie omni test-approval` command: drives one approval round-trip (enqueue → announce+⏳ → resolve+✅) against an injectable transport by default (CI-safe, no network); `--live` runs ONE real round-trip between the configured instance/approvalChat (deliberate, single message). +3. `genie doctor` (`src/genie-commands/doctor.ts`)/handshake warning when the installed CC hook `timeout` is below `pollBudgetMs`. +4. Tests (fake transport): ⏳ set on announce with the right target id; swapped to ✅ on approve, ❌ on deny/expire; row closed; `test-approval` fake path green; doctor warning fires on a too-low timeout. **Acceptance Criteria:** -- [ ] Approve/deny yields a ✅/❌ thread signal and closes the row; no lingering prompt (tested). +- [ ] Announce sets ⏳ on the approval message (real id); approve→✅, deny/expire→❌; row closed; no lingering prompt (tested with a fake transport asserting the set-reaction/edit calls + target). - [ ] `genie omni test-approval` (fake) green in CI; `--live` documented. - [ ] doctor/handshake warns on an inadequate hook timeout. - [ ] Full `bun run check` green. @@ -132,11 +138,11 @@ bun run build bun dist/genie.js omni test-approval 2>&1 | grep -qiE 'approved|allow|round-trip' || { echo "FAIL: test-approval harness"; exit 1; } ``` -**depends-on:** group-1 +**depends-on:** group-2 --- ## Cross-wish dependencies - **Builds on** the live-validated `omni-runner-port` bridge + the `omni-hardening` (PR #2507) instance-scope + approval-budget-doc fixes. -- **Records** the 2026-07-03 live QA outcome (allow path proven end-to-end; deny code-verified; personal instance offline; reactions unconfirmed) — this wish closes the reaction + correlation gaps that QA exposed. +- **Records** the 2026-07-03 live QA outcome (allow path proven end-to-end; deny code-verified; personal instance offline; reactions unconfirmed) — this wish closes the reaction + correlation + feedback gaps that QA exposed. From 24df536a8e93b2dae750ff7886cfa2cd6123ede9 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 02:07:10 -0300 Subject: [PATCH 09/18] docs(omni-approval-ux): tick G1 spike done --- .genie/wishes/omni-approval-ux/WISH.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.genie/wishes/omni-approval-ux/WISH.md b/.genie/wishes/omni-approval-ux/WISH.md index 6ebeecfa7..7bd6fa3d0 100644 --- a/.genie/wishes/omni-approval-ux/WISH.md +++ b/.genie/wishes/omni-approval-ux/WISH.md @@ -72,8 +72,8 @@ The v5 omni approval bridge is live-proven (a PreToolUse hook → WhatsApp 🔔 2. `.genie/wishes/omni-approval-ux/SPIKE.md`: the inbound reaction subject + payload, the id-returning send path, the outbound set-reaction GO/NO-GO + swap-or-fallback mechanism, the text quoted-id finding, and a GO recommendation for G2/G3 with the exact fields genie must store/match. **Acceptance Criteria:** -- [ ] SPIKE.md documents the inbound reaction subject + payload, the correlatable-send decision, the outbound set-reaction GO/NO-GO (+ fallback if NO-GO), and the text quoted-id finding — evidence-backed. -- [ ] No spam: at most one or two clearly-labelled messages between Felipe's own numbers. +- [x] SPIKE.md documents the inbound reaction subject + payload, the correlatable-send decision, the outbound set-reaction GO/NO-GO (+ fallback if NO-GO), and the text quoted-id finding — evidence-backed. +- [x] No spam: at most one or two clearly-labelled messages between Felipe's own numbers. **Validation:** ```bash From 72e87febaa327113e6b25108d5732766a44ed4f8 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 02:26:22 -0300 Subject: [PATCH 10/18] feat(omni): correlate approvals by real stanza id + reaction resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: on message ], 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. --- .genie/wishes/omni-approval-ux/WISH.md | 4 +- src/lib/omni-runner.test.ts | 126 ++++++++++- src/lib/omni-runner.ts | 282 +++++++++++++++++-------- src/term-commands/omni.test.ts | 56 +++-- 4 files changed, 360 insertions(+), 108 deletions(-) diff --git a/.genie/wishes/omni-approval-ux/WISH.md b/.genie/wishes/omni-approval-ux/WISH.md index 7bd6fa3d0..c4fca2e2d 100644 --- a/.genie/wishes/omni-approval-ux/WISH.md +++ b/.genie/wishes/omni-approval-ux/WISH.md @@ -98,8 +98,8 @@ grep -qiE 'set.?reaction|outbound react|GO/NO-GO|fallback' .genie/wishes/omni-ap 3. Tests (fake transport): two concurrent pending approvals resolved to the correct one by the real id; reaction on the verified subject resolves the correlated approval; a quoted reply resolves the correlated approval; bare text still resolves oldest. **Acceptance Criteria:** -- [ ] Concurrent approvals resolve by the real id (reaction/quoted reply), not oldest; reactions resolve on the verified subject; bare text = oldest fallback; instance-scoped. -- [ ] typecheck + `bun test src/lib/omni-runner.test.ts src/lib/v5/omni-queue.test.ts` green. +- [x] Concurrent approvals resolve by the real id (reaction/quoted reply), not oldest; reactions resolve on the verified subject; bare text = oldest fallback; instance-scoped. +- [x] typecheck + `bun test src/lib/omni-runner.test.ts src/lib/v5/omni-queue.test.ts` green. **Validation:** ```bash diff --git a/src/lib/omni-runner.test.ts b/src/lib/omni-runner.test.ts index 10facdb6b..a5cb28b06 100644 --- a/src/lib/omni-runner.test.ts +++ b/src/lib/omni-runner.test.ts @@ -10,9 +10,9 @@ import type { Database } from 'bun:sqlite'; import { afterEach, describe, expect, test } from 'bun:test'; import type { OmniRuntimeConfig } from './omni-config.js'; -import { type SpawnClaude, type SpawnClaudeResult, createOmniRunner } from './omni-runner.js'; +import { type OmniSend, type SpawnClaude, type SpawnClaudeResult, createOmniRunner } from './omni-runner.js'; import { openGlobalDb } from './v5/global-db.js'; -import { listInbox } from './v5/omni-queue.js'; +import { enqueueApproval, getApproval, listInbox } from './v5/omni-queue.js'; const INSTANCE = 'inst-A'; const ROUTE_CHAT = 'chat-42'; @@ -251,3 +251,125 @@ describe('omni runner — inbound one-shot routing', () => { expect(reply.endsWith('…')).toBe(true); }); }); + +// --------------------------------------------------------------------------- +// Correlated approval identity + inbound reaction resolve (Group 2). +// +// `announce()` sends each approval via the injectable id-returning send and +// stores the REAL stanza id it returns; an inbound reaction (on `omni.message.*`, +// NOT the retired `omni.event.*`) resolves the approval whose stored id it +// targets, falling back to oldest only for bare text. Every test drives a FAKE +// `sendApproval` — zero network. +// --------------------------------------------------------------------------- +const APPROVAL_CHAT = 'approval-chat'; // matches rt() above + +/** Fake id-returning send: assigns each approval a stanza id derived from its + * input-summary marker (embedded in the formatted message), so correlation is + * order-independent. `summary-A` → `stanza-A`, `summary-B` → `stanza-B`. */ +const sendApproval: OmniSend = async ({ text }) => { + const marker = text.includes('summary-B') ? 'B' : 'A'; + return { success: true, messageId: `stanza-${marker}` }; +}; + +/** An inbound frame on the approval chat, as omni would publish it. */ +function approvalInbound(payload: Record): [string, string] { + return [ + `omni.message.${INSTANCE}.${APPROVAL_CHAT}`, + JSON.stringify({ chatId: APPROVAL_CHAT, instanceId: INSTANCE, sender: 'boss', ...payload }), + ]; +} + +const reactionContent = (emoji: string, targetId: string): string => `[Reaction: ${emoji} on message ${targetId}]`; +const THUMBS_UP = '\u{1F44D}'; + +// A fixed clock so the runner's `expireStale` (pollBudgetMs = 10_000) never +// expires a just-enqueued row: rows are stamped a second or two before NOW. +const NOW = 1_000_000; +const approvalRunner = (db: Database) => + createOmniRunner({ db, config: rt(), publish: () => {}, sendApproval, now: () => NOW }); + +describe('omni runner — correlated approval identity + reactions', () => { + test('announce stores the REAL stanza id returned by the send (not a genId ref)', async () => { + const db = freshDb(); + const runner = approvalRunner(db); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); + await runner.whenIdle(); + expect(getApproval(db, a)?.omniMessageId).toBe('stanza-A'); + }); + + test('reaction resolves the exact approval by stored stanza id, not the oldest', async () => { + const db = freshDb(); + const runner = approvalRunner(db); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + const b = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-B', now: NOW - 1000 }); + runner.tick(); + await runner.whenIdle(); + // Both rows tagged with their own real stanza ids. + expect(getApproval(db, a)?.omniMessageId).toBe('stanza-A'); + expect(getApproval(db, b)?.omniMessageId).toBe('stanza-B'); + + // A 👍 reaction on B's stanza id resolves B — NOT the older A. + runner.handleMessage( + ...approvalInbound({ content: reactionContent(THUMBS_UP, 'stanza-B'), messageId: 'stanza-B' }), + ); + expect(getApproval(db, b)?.status).toBe('approved'); + expect(getApproval(db, a)?.status).toBe('pending'); + }); + + test('dual-emit bare-emoji echo does not double-resolve a second approval', async () => { + const db = freshDb(); + const runner = approvalRunner(db); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + const b = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-B', now: NOW - 1000 }); + runner.tick(); + await runner.whenIdle(); + + // A human 👍 reaches genie twice (SPIKE dual-emit): the id-bearing reaction + // form resolves A... + runner.handleMessage( + ...approvalInbound({ content: reactionContent(THUMBS_UP, 'stanza-A'), messageId: 'stanza-A' }), + ); + // ...and a bare-emoji echo, which must be ignored (not treated as a reaction, + // not a text token) so it can't fall through to resolve the oldest (now B). + runner.handleMessage(...approvalInbound({ content: THUMBS_UP, messageId: 'stanza-A' })); + + expect(getApproval(db, a)?.status).toBe('approved'); + expect(getApproval(db, b)?.status).toBe('pending'); + }); + + test('bare text reply still resolves the oldest pending approval (documented fallback)', async () => { + const db = freshDb(); + const runner = approvalRunner(db); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + const b = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-B', now: NOW - 1000 }); + runner.tick(); + await runner.whenIdle(); + + runner.handleMessage(...approvalInbound({ content: 'y' })); + // Oldest (A) resolves; B untouched — bare text carries no quoted id here. + expect(getApproval(db, a)?.status).toBe('approved'); + expect(getApproval(db, b)?.status).toBe('pending'); + }); + + test('a reaction from another instance is ignored (PR #2507 instance-scope guard)', async () => { + const db = freshDb(); + const runner = approvalRunner(db); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); + await runner.whenIdle(); + + // Same emoji + same stored stanza id, but a DIFFERENT instanceId → dropped. + runner.handleMessage( + `omni.message.other-instance.${APPROVAL_CHAT}`, + JSON.stringify({ + content: reactionContent(THUMBS_UP, 'stanza-A'), + messageId: 'stanza-A', + chatId: APPROVAL_CHAT, + instanceId: 'other-instance', + sender: 'stranger', + }), + ); + expect(getApproval(db, a)?.status).toBe('pending'); + }); +}); diff --git a/src/lib/omni-runner.ts b/src/lib/omni-runner.ts index 72c3969c0..321c67b60 100644 --- a/src/lib/omni-runner.ts +++ b/src/lib/omni-runner.ts @@ -3,13 +3,19 @@ * * Owns the NATS transport and the two-way bridge between the phone and the * global approval queue: - * - subscribes `omni.message.{instance}.>` (text replies) and `omni.event.>` - * (reactions); - * - on each new pending approval row, PUBLISHES an approval-request to - * `omni.reply.{instance}.{chat}` (outbound send subject, mirroring v4's - * omni-bridge) and tags the row with a correlation id; + * - subscribes `omni.message.{instance}.>` — both text replies AND reactions + * arrive here. A WhatsApp reaction reaches genie as a `message.received` + * whose content is `[Reaction: on message ]` (the target + * id is also the top-level `messageId`); the legacy `omni.event.>` subject + * has zero publishers in this Omni build, so it is retired (SPIKE finding a); + * - on each new pending approval row, SENDS an approval-request over the Omni + * HTTP API (an id-returning send) and tags the row with the REAL Omni + * message id (the WhatsApp stanza id) the send returns — so an inbound + * reaction can be correlated to the exact approval it targets, rather than + * the old self-referential `genId()` ref that matched nothing (SPIKE b); * - matches inbound replies/reactions against the approve/deny vocabulary and - * resolves the oldest pending approval (or the reaction-correlated one); + * resolves the correlated approval (reaction → the stored stanza id; bare + * text → oldest-pending fallback); * - records every inbound message to the inbox (`recordInbound`). One-shot * agent spawning on inbound is Group 4 — this group only stores. * @@ -25,6 +31,7 @@ import type { Database } from 'bun:sqlite'; import type { OmniRoute, OmniRuntimeConfig } from './omni-config.js'; import { matchReaction, matchTextToken } from './omni-matching.js'; +import { signOmniRequest } from './omni-signature.js'; import { ApprovalConflictError, type ApprovalDecision, @@ -105,6 +112,73 @@ export const defaultSpawnClaude: SpawnClaude = async ({ message, cwd, signal }) return { stdout, exitCode }; }; +// ============================================================================ +// Injectable id-returning Omni send (approval announce) +// ============================================================================ + +/** + * Result of an id-returning Omni send. `messageId` is the correlatable id — the + * WhatsApp stanza id / externalId (SPIKE finding (b)) — that an inbound reaction + * later references. Absent on a failed send (the row stays un-tagged and is + * retried on the next tick). + */ +export interface OmniSendResult { + messageId?: string; + success?: boolean; + error?: string; +} + +export interface OmniSendOpts { + instance: string; + chat: string; + text: string; +} + +/** + * Send an approval-request message and RETURN the id it was assigned. Injectable + * so tests drive a fake (zero network); the default posts to the Omni HTTP send + * API. This replaces the fire-and-forget NATS `omni.reply.*` publish whose id + * Omni's `onReply` discarded — capturing the real Omni message id is what makes + * a reaction correlatable to the exact approval it targets. + */ +export type OmniSend = (opts: OmniSendOpts) => Promise; + +/** + * Default id-returning send — POSTs the approval message to Omni's HTTP send + * route, signed exactly like {@link registerAgentInOmni} (bearer + optional + * ed25519). Returns the message id from the response so `announce()` can store + * the correlatable stanza id. Degrades to an error result (silent retry, never a + * throw) when the Omni API URL is unconfigured. + * + * NOTE: the exact send route + response field that surface the WhatsApp *stanza + * id* (vs the persisted Omni UUID) are documented in SPIKE (b) but not yet + * exercised against the live hub — the injectable seam above is what the test + * suite proves; G3's `--live` round-trip confirms this default's wire format. + */ +export function makeDefaultOmniSend(config: OmniRuntimeConfig): OmniSend { + return async ({ instance, chat, text }) => { + const apiUrl = config.apiUrl; + if (!apiUrl) return { success: false, error: 'omni apiUrl not configured' }; + const path = '/api/v2/messages'; + const bodyJson = JSON.stringify({ instanceId: instance, chatId: chat, text }); + const headers: Record = { 'Content-Type': 'application/json' }; + if (config.apiKey) headers.Authorization = `Bearer ${config.apiKey}`; + const sig = signOmniRequest('POST', path, bodyJson); + if (sig) Object.assign(headers, sig); + const res = await fetch(`${apiUrl.replace(/\/+$/, '')}${path}`, { + method: 'POST', + headers, + body: bodyJson, + signal: AbortSignal.timeout(10_000), + }); + if (!res.ok) return { success: false, error: `HTTP ${res.status}` }; + // SendResult ({ success, messageId, ... }) may be top-level or `{ data }`-wrapped. + const json = (await res.json()) as { messageId?: string; data?: { messageId?: string } }; + const messageId = json.messageId ?? json.data?.messageId; + return { success: Boolean(messageId), messageId }; + }; +} + /** Default factory — dynamically imports `nats`. Only invoked by `omni serve`. */ export const defaultNatsFactory: NatsFactory = async ({ servers }) => { const nats = await import('nats'); @@ -135,15 +209,12 @@ interface InboundMessagePayload { sender?: string; instanceId?: string; chatId?: string; -} - -interface ReactionEventPayload { - type?: string; - emoji?: string; + /** + * For a reaction `message.received`, the reacted-to message's stanza id (also + * embedded in `content` as `[Reaction: on message ]`). Used + * to correlate the reaction to the exact approval it targets (SPIKE a). + */ messageId?: string; - chatId?: string; - instanceId?: string; - sender?: string; } // ============================================================================ @@ -159,22 +230,29 @@ export interface OmniRunnerDeps { log?: (line: string) => void; /** Injectable clock. */ now?: () => number; - /** Injectable correlation-id generator (tagged onto the outbound request). */ + /** Injectable correlation-id generator (tagged onto routed one-shot replies). */ genCorrelationId?: () => string; /** * Injectable one-shot executor. Defaults to a real `claude -p`; tests pass a * fake so no real claude is ever forked. */ spawnClaude?: SpawnClaude; + /** + * Injectable id-returning approval send. Defaults to the Omni HTTP send + * ({@link makeDefaultOmniSend}); tests pass a fake that returns a known stanza + * id so `announce()` is exercised with zero network. + */ + sendApproval?: OmniSend; } export interface OmniRunner { - /** Publish any un-announced pending approvals + expire stale rows. */ + /** Send any un-announced pending approvals + expire stale rows. */ tick(): void; - /** Handle one inbound `omni.message.*` frame. */ + /** + * Handle one inbound `omni.message.*` frame — a text reply OR a reaction (both + * arrive on this subject in this Omni build). + */ handleMessage(subject: string, data: string): void; - /** Handle one inbound `omni.event.*` frame (reactions). */ - handleEvent(subject: string, data: string): void; /** * Resolve once every in-flight one-shot run has settled. Test seam — the serve * loop never awaits it (runs are fire-and-forget), but tests use it to observe @@ -183,7 +261,7 @@ export interface OmniRunner { whenIdle(): Promise; } -function formatApprovalMessage(tool: string, inputSummary: string, correlationId: string): string { +function formatApprovalMessage(tool: string, inputSummary: string): string { const preview = inputSummary.length > 200 ? `${inputSummary.slice(0, 197)}...` : inputSummary; return [ '\u{1F514} *Approval Required*', @@ -193,26 +271,21 @@ function formatApprovalMessage(tool: string, inputSummary: string, correlationId '', 'Reply *y* to approve or *n* to deny', 'Or react \u{1F44D} / \u{1F44E}', - '', - `_ref: ${correlationId}_`, ].join('\n'); } -/** Outbound reply payload shape — mirrors origin/v4 omni-bridge replies. */ -function buildOutboundPayload( - config: OmniRuntimeConfig, - content: string, - correlationId: string, - nowMs: number, -): string { - return JSON.stringify({ - content, - agent: 'genie', - chat_id: config.approvalChat, - instance_id: config.instance, - request_id: correlationId, - timestamp: new Date(nowMs).toISOString(), - }); +/** + * Parse a reaction `message.received` content — `[Reaction: on message + * ]`, optionally prefixed by `[]: ` — into its emoji + target + * stanza id. Returns null for ordinary text so the caller falls through to the + * text-token path. This is the sole reaction shape genie treats as a decision; + * the dual-emit bare-emoji echo (SPIKE a) is deliberately NOT matched here, so + * it can never double-resolve. + */ +const REACTION_RE = /\[Reaction:\s*(.+?)\s+on message\s+(.+?)\]/; +function parseReaction(content: string): { emoji: string; targetId: string } | null { + const m = REACTION_RE.exec(content); + return m ? { emoji: m[1].trim(), targetId: m[2].trim() } : null; } /** chat id from payload, else parsed from `omni.message.{instance}.{chat...}`. */ @@ -223,8 +296,8 @@ function chatIdFromSubject(subject: string): string | undefined { /** * Outbound reply payload for a routed one-shot, addressed at the exact - * (instance, chat) the inbound arrived on (NOT the approval chat). Same shape as - * {@link buildOutboundPayload} so the omni side deserializes replies uniformly. + * (instance, chat) the inbound arrived on (NOT the approval chat). Mirrors the + * origin/v4 omni-bridge reply shape so the omni side deserializes it uniformly. */ function buildRoutedReplyPayload( instance: string, @@ -275,6 +348,7 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { const genId = deps.genCorrelationId ?? (() => `req_${Date.now().toString(36)}${Math.random().toString(36).slice(2, 10)}`); const spawnClaude = deps.spawnClaude ?? defaultSpawnClaude; + const sendApproval = deps.sendApproval ?? makeDefaultOmniSend(config); const vocab = { approveTokens: config.approveTokens, denyTokens: config.denyTokens, @@ -290,6 +364,11 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { const inFlight = new Set(); /** Live one-shot promises, so `whenIdle` (tests) can await completion. */ const pending = new Set>(); + /** Approval ids with an announce send in flight — guards against a second tick + * re-sending a row whose `omni_message_id` is not yet stored. */ + const announcing = new Set(); + /** Live announce-send promises, drained by `whenIdle` alongside `pending`. */ + const inFlightSends = new Set>(); const routeKey = (instance: string, chat: string): string => `${instance}${chat}`; const findRoute = (instance: string, chat: string): OmniRoute | undefined => routes.find((r) => r.instance === instance && r.chat === chat); @@ -355,17 +434,45 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { pending.add(run); } + /** + * Announce a single pending approval via the id-returning send and tag the row + * with the REAL Omni message id (the stanza id) the send returns — so an + * inbound reaction correlates to THIS approval. Tags only on a successful send; + * a failed/id-less send leaves the row un-tagged so the next tick retries + * (mirrors the old tag-after-publish semantics). Never throws. + */ + async function sendAnnounce(appr: { id: string; tool: string; inputSummary: string }): Promise { + const text = formatApprovalMessage(appr.tool, appr.inputSummary); + try { + const result = await sendApproval({ instance: config.instance ?? '', chat: config.approvalChat ?? '', text }); + if (!result.messageId) { + log( + `[omni] announce ${appr.id}: send returned no messageId${result.error ? ` (${result.error})` : ''} — will retry`, + ); + return; + } + attachOmniMessageId(db, appr.id, result.messageId); + log(`[omni] announced approval ${appr.id} (omni ${result.messageId})`); + } catch (err) { + log(`[omni] announce ${appr.id} failed: ${errText(err)}`); + } + } + + /** + * Send every un-announced pending approval. Fire-and-forget per row (the tick + * loop never blocks on a slow HTTP send), guarded by `announcing` so a second + * tick before the first send settles does not double-send. + */ function announce(): void { for (const appr of listPendingApprovals(db)) { if (appr.omniMessageId) continue; // already announced - const correlationId = genId(); - const text = formatApprovalMessage(appr.tool, appr.inputSummary, correlationId); - const subject = `omni.reply.${config.instance}.${config.approvalChat}`; - publish(subject, buildOutboundPayload(config, text, correlationId, now())); - // Tag AFTER publishing so a crash mid-publish re-announces rather than - // silently dropping the request. - attachOmniMessageId(db, appr.id, correlationId); - log(`[omni] announced approval ${appr.id} (ref ${correlationId})`); + if (announcing.has(appr.id)) continue; // send in flight + announcing.add(appr.id); + const send = sendAnnounce(appr).finally(() => { + announcing.delete(appr.id); + inFlightSends.delete(send); + }); + inFlightSends.add(send); } } @@ -387,6 +494,25 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { } } + /** + * Resolve an approval from an inbound reaction. Correlates by the stored real + * Omni message id (the reaction's target stanza id, from the `[Reaction: … on + * message ]` content or the top-level `messageId`); falls back to oldest + * only when no pending row carries that id. Non-vocabulary emoji are ignored. + */ + function resolveReaction(emoji: string, targetId: string | undefined, sender: string): void { + const decision = matchReaction(emoji, vocab); + if (!decision) return; + if (targetId) { + const match = listPendingApprovals(db).find((a) => a.omniMessageId === targetId); + if (match) { + tryResolve(match.id, decision, sender, `reaction ${emoji}`); + return; + } + } + resolveOldest(decision, sender, `reaction ${emoji} (fallback)`); + } + return { tick(): void { expireStale(db, config.approvals.pollBudgetMs, now()); @@ -415,46 +541,34 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { // Only the approval chat can resolve approvals. if (!chatId || chatId !== config.approvalChat) return; - if (!msg.content) return; - const decision = matchTextToken(msg.content, vocab); - if (!decision) return; - resolveOldest(decision, sender, `text:"${msg.content.trim().toLowerCase()}"`); - }, - - handleEvent(subject: string, data: string): void { - let evt: ReactionEventPayload; - try { - evt = JSON.parse(data) as ReactionEventPayload; - } catch { + // Instance-scope guard (PR #2507): another instance's reply/reaction must + // never resolve our approval, even if the approvalChat JID repeats. + if (msg.instanceId && config.instance && msg.instanceId !== config.instance) return; + if (!body) return; + + // A reaction reaches genie on THIS subject (SPIKE a) as + // `[Reaction: on message ]`; the target id is also the + // top-level `messageId`. Only this form is a reaction — the dual-emit + // bare-emoji echo matches neither branch below, so it never double-resolves. + const reaction = parseReaction(body); + if (reaction) { + resolveReaction(reaction.emoji, reaction.targetId || msg.messageId, sender); return; } - if (evt.type !== 'reaction' || !evt.emoji) return; - // Scope to our own instance — text replies are already subject-scoped to - // `omni.message.${config.instance}.>`, but reactions arrive on a shared - // `omni.event.>` bus where approvalChat JIDs can repeat across instances. - // Without this, another instance's 👍/👎 could resolve our approval. - if (evt.instanceId && config.instance && evt.instanceId !== config.instance) return; - const chatId = evt.chatId ?? chatIdFromSubject(subject); - if (!chatId || chatId !== config.approvalChat) return; - const decision = matchReaction(evt.emoji, vocab); + + // Bare text reply → oldest-pending fallback (no quoted id in this build). + const decision = matchTextToken(body, vocab); if (!decision) return; - const sender = evt.sender ?? 'whatsapp-user'; - - // Correlate by the ref we tagged onto the row, else fall back to oldest. - if (evt.messageId) { - const match = listPendingApprovals(db).find((a) => a.omniMessageId === evt.messageId); - if (match) { - tryResolve(match.id, decision, sender, `reaction ${evt.emoji}`); - return; - } - } - resolveOldest(decision, sender, `reaction ${evt.emoji} (fallback)`); + resolveOldest(decision, sender, `text:"${body.trim().toLowerCase()}"`); }, async whenIdle(): Promise { - // Drain in a loop: a settling run must not add new work, but snapshot-await - // is cheap insurance against future callers that fan out mid-drain. - while (pending.size > 0) await Promise.allSettled([...pending]); + // Drain both one-shot runs and announce sends in a loop: a settling task + // must not add new work, but snapshot-await is cheap insurance against + // future callers that fan out mid-drain. + while (pending.size > 0 || inFlightSends.size > 0) { + await Promise.allSettled([...pending, ...inFlightSends]); + } }, }; } @@ -502,10 +616,11 @@ export async function runOmniServe(opts: RunOmniServeOptions): Promise { const nc = await factory({ servers: config.natsUrl }); const runner = createOmniRunner({ db, config, publish: nc.publish, log }); + // Both text replies AND reactions arrive on `omni.message.{instance}.>` in this + // Omni build; the legacy `omni.event.>` subject has no publishers (SPIKE a), so + // it is no longer subscribed. const msgSub = nc.subscribe(`omni.message.${config.instance}.>`); - const evtSub = nc.subscribe('omni.event.>'); void consume(msgSub, runner.handleMessage, log); - void consume(evtSub, runner.handleEvent, log); const timer = setInterval(() => { try { @@ -526,7 +641,6 @@ export async function runOmniServe(opts: RunOmniServeOptions): Promise { clearInterval(timer); msgSub.unsubscribe(); - evtSub.unsubscribe(); await nc.close(); log('[omni] stopped'); } diff --git a/src/term-commands/omni.test.ts b/src/term-commands/omni.test.ts index 2f0ba056d..8a9ff2e60 100644 --- a/src/term-commands/omni.test.ts +++ b/src/term-commands/omni.test.ts @@ -72,22 +72,36 @@ interface Published { payload: string; } +interface Sent { + instance: string; + chat: string; + text: string; +} + +/** The stanza id the fake id-returning send returns; the real Omni message id + * `announce()` stores and a reaction correlates against. */ +const STANZA_ID = 'stanza-fixed-1'; + /** * Run the real handler↔runner loop once. The handler enqueues + polls; on its - * first `sleep` we play the runner (announce + phone action), then the handler's - * next poll observes the resolution. + * first `sleep` we play the runner (announce via a FAKE id-returning send, then + * the phone action), and the handler's next poll observes the resolution. */ async function driveRoundTrip( db: Database, config: OmniRuntimeConfig, - phone: (runner: ReturnType, published: Published[]) => void, + phone: (runner: ReturnType, published: Published[], sent: Sent[]) => void, ): Promise { const published: Published[] = []; + const sent: Sent[] = []; const runner = createOmniRunner({ db, config, publish: (subject, payload) => published.push({ subject, payload }), - genCorrelationId: () => 'ref-fixed-1', + sendApproval: async (opts) => { + sent.push(opts); + return { success: true, messageId: STANZA_ID }; + }, }); let phoned = false; return omniApproval(PAYLOAD, { @@ -96,8 +110,9 @@ async function driveRoundTrip( sleep: async () => { if (phoned) return; phoned = true; - runner.tick(); // announce the pending approval (records a publish) - phone(runner, published); + runner.tick(); // announce the pending approval (fires the id-returning send) + await runner.whenIdle(); // wait for the send to store the real stanza id + phone(runner, published, sent); }, }); } @@ -106,36 +121,37 @@ describe('omni runner — five round-trips (no network)', () => { test('1. token-approve: inbound "yes" → allow, request announced, message stored', async () => { const config = rt(); const db = freshDb(); - let publishedRef: Published[] = []; - const res = await driveRoundTrip(db, config, (runner, published) => { - publishedRef = published; + let sentRef: Sent[] = []; + const res = await driveRoundTrip(db, config, (runner, _published, sent) => { + sentRef = sent; runner.handleMessage( `omni.message.${config.instance}.${config.approvalChat}`, JSON.stringify({ content: 'yes', chatId: config.approvalChat, sender: 'boss', instanceId: config.instance }), ); }); expect(res?.hookSpecificOutput?.permissionDecision).toBe('allow'); - // Outbound approval-request published to the reply subject. - expect(publishedRef.length).toBe(1); - expect(publishedRef[0].subject).toBe(`omni.reply.${config.instance}.${config.approvalChat}`); - expect(publishedRef[0].payload).toContain('Approval Required'); + // Approval-request sent via the id-returning send, addressed at the approval chat. + expect(sentRef.length).toBe(1); + expect(sentRef[0].chat).toBe(config.approvalChat as string); + expect(sentRef[0].text).toContain('Approval Required'); // Inbound reply stored to the inbox. const inbox = listInbox(db); expect(inbox.length).toBe(1); expect(inbox[0].body).toBe('yes'); }); - test('2. reaction-approve: 👍 correlated by ref → allow', async () => { + test('2. reaction-approve: 👍 correlated by the stored stanza id → allow', async () => { const config = rt(); const db = freshDb(); const res = await driveRoundTrip(db, config, (runner) => { - // The runner tagged the row with genCorrelationId() = 'ref-fixed-1'. - runner.handleEvent( - 'omni.event.reaction', + // announce() stored the REAL stanza id (STANZA_ID) the send returned; a + // reaction referencing it (on `omni.message.*`, not the retired + // `omni.event.*`) resolves this exact approval. + runner.handleMessage( + `omni.message.${config.instance}.${config.approvalChat}`, JSON.stringify({ - type: 'reaction', - emoji: '\u{1F44D}', - messageId: 'ref-fixed-1', + content: `[Reaction: \u{1F44D} on message ${STANZA_ID}]`, + messageId: STANZA_ID, chatId: config.approvalChat, instanceId: config.instance, sender: 'boss', From 483df2af3434edb040a61a54c15536453b8aef82 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 03:21:13 -0300 Subject: [PATCH 11/18] =?UTF-8?q?feat(omni):=20two-state=20=E2=8F=B3?= =?UTF-8?q?=E2=86=92=E2=9C=85=20approval=20acks=20+=20reconciliation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .genie/wishes/omni-approval-ux/WISH.md | 22 +-- src/genie-commands/doctor.test.ts | 48 +++++- src/genie-commands/doctor.ts | 91 +++++++++- src/lib/omni-config.ts | 5 + src/lib/omni-runner.test.ts | 226 +++++++++++++++++++++++++ src/lib/omni-runner.ts | 196 +++++++++++++++++++-- src/lib/v5/global-db.test.ts | 87 ++++++++++ src/lib/v5/global-db.ts | 39 ++++- src/lib/v5/omni-queue.test.ts | 96 ++++++++++- src/lib/v5/omni-queue.ts | 62 +++++++ src/term-commands/omni.test.ts | 26 +++ src/term-commands/omni.ts | 160 ++++++++++++++++- src/types/genie-config.ts | 10 +- 13 files changed, 1037 insertions(+), 31 deletions(-) diff --git a/.genie/wishes/omni-approval-ux/WISH.md b/.genie/wishes/omni-approval-ux/WISH.md index c4fca2e2d..adaf3e334 100644 --- a/.genie/wishes/omni-approval-ux/WISH.md +++ b/.genie/wishes/omni-approval-ux/WISH.md @@ -2,7 +2,7 @@ | Field | Value | |-------|-------| -| **Status** | DRAFT | +| **Status** | DONE — G1 spike + G2 correlation + G3 ⏳→✅ lifecycle all SHIP-reviewed (2 HIGHs found+fixed); ⏳→✅ in-place render awaits one Felipe-approved live round-trip (2026-07-03) | | **Slug** | `omni-approval-ux` | | **Date** | 2026-07-03 | | **Author** | Felipe + Genie | @@ -45,12 +45,12 @@ The v5 omni approval bridge is live-proven (a PreToolUse hook → WhatsApp 🔔 ## Success Criteria -- [ ] **G1 spike:** `.genie/wishes/omni-approval-ux/SPIKE.md` documents (evidence from the real hub) the inbound reaction NATS subject + payload, the send path that yields a correlatable message id, the outbound set-reaction capability (GO/NO-GO + the swap mechanism or the fallback), AND whether inbound text replies carry a quoted-message id — with the exact fields genie stores/matches. -- [ ] **⏳→✅ ack lifecycle:** on announce, the approval message carries a ⏳ status (reaction, or the NO-GO fallback); on approve it becomes ✅; on deny/expire ❌. Verified with a fake transport asserting the set-reaction/edit calls + target id. -- [ ] **Correlated resolution:** two concurrent pending approvals — a reaction/quoted reply answering the 2nd resolves the 2nd (not the 1st); `omni_message_id` holds the REAL send id. Tested. (Bare unquoted text still resolves oldest — documented fallback, also tested.) -- [ ] **Reaction approve/deny** resolves on the verified subject, correlated to the right approval; instance-scoped (PR #2507 guard). Text fallback still works. Tested. -- [ ] `genie omni test-approval` drives one clean round-trip (fake transport in CI; `--live` documented); `genie doctor`/handshake warns when the CC hook timeout < pollBudgetMs. -- [ ] Full `bun run check` green; NO live messages sent by the automated suite. +- [x] **G1 spike:** `.genie/wishes/omni-approval-ux/SPIKE.md` documents (evidence from the real hub) the inbound reaction NATS subject + payload, the send path that yields a correlatable message id, the outbound set-reaction capability (GO/NO-GO + the swap mechanism or the fallback), AND whether inbound text replies carry a quoted-message id — with the exact fields genie stores/matches. +- [x] **⏳→✅ ack lifecycle:** on announce, the approval message carries a ⏳ status (reaction, or the NO-GO fallback); on approve it becomes ✅; on deny/expire ❌. Verified with a fake transport asserting the set-reaction/edit calls + target id. +- [x] **Correlated resolution:** two concurrent pending approvals — a reaction/quoted reply answering the 2nd resolves the 2nd (not the 1st); `omni_message_id` holds the REAL send id. Tested. (Bare unquoted text still resolves oldest — documented fallback, also tested.) +- [x] **Reaction approve/deny** resolves on the verified subject, correlated to the right approval; instance-scoped (PR #2507 guard). Text fallback still works. Tested. +- [x] `genie omni test-approval` drives one clean round-trip (fake transport in CI; `--live` documented); `genie doctor`/handshake warns when the CC hook timeout < pollBudgetMs. +- [x] Full `bun run check` green; NO live messages sent by the automated suite. ## Execution Strategy @@ -123,10 +123,10 @@ bun run typecheck 4. Tests (fake transport): ⏳ set on announce with the right target id; swapped to ✅ on approve, ❌ on deny/expire; row closed; `test-approval` fake path green; doctor warning fires on a too-low timeout. **Acceptance Criteria:** -- [ ] Announce sets ⏳ on the approval message (real id); approve→✅, deny/expire→❌; row closed; no lingering prompt (tested with a fake transport asserting the set-reaction/edit calls + target). -- [ ] `genie omni test-approval` (fake) green in CI; `--live` documented. -- [ ] doctor/handshake warns on an inadequate hook timeout. -- [ ] Full `bun run check` green. +- [x] Announce sets ⏳ on the approval message (real id); approve→✅, deny/expire→❌; row closed; no lingering prompt (tested with a fake transport asserting the set-reaction/edit calls + target). +- [x] `genie omni test-approval` (fake) green in CI; `--live` documented. +- [x] doctor/handshake warns on an inadequate hook timeout. +- [x] Full `bun run check` green. **Validation:** ```bash diff --git a/src/genie-commands/doctor.test.ts b/src/genie-commands/doctor.test.ts index 0539c5855..d02021c63 100644 --- a/src/genie-commands/doctor.test.ts +++ b/src/genie-commands/doctor.test.ts @@ -4,7 +4,7 @@ import { execFileSync } from 'node:child_process'; import { existsSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { doctorCommand } from './doctor.js'; +import { doctorCommand, evaluateOmniHookTimeout, findDispatchHookTimeoutSec } from './doctor.js'; /** * Capture everything written to stdout during `fn`, restoring the real @@ -76,6 +76,52 @@ describe('doctorCommand', () => { }); }); +describe('omni hook-timeout guardrail', () => { + const DISPATCH = '"$HOME/.genie/bin/genie" hook dispatch'; + + test('finds the smallest dispatch timeout across PreToolUse entries', () => { + const settings = { + hooks: { + PreToolUse: [ + { matcher: 'Bash', hooks: [{ command: 'bash git-safety.sh', timeout: 5 }] }, + { matcher: '*', hooks: [{ command: DISPATCH, timeout: 30 }] }, + { matcher: 'Write', hooks: [{ command: DISPATCH, timeout: 15 }] }, + ], + }, + }; + expect(findDispatchHookTimeoutSec(settings)).toBe(15); + }); + + test('returns null when no dispatch hook is present', () => { + expect( + findDispatchHookTimeoutSec({ hooks: { PreToolUse: [{ hooks: [{ command: 'other', timeout: 9 }] }] } }), + ).toBeNull(); + expect(findDispatchHookTimeoutSec({})).toBeNull(); + }); + + test('warns when the hook timeout is below pollBudgetMs', () => { + const res = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: 110_000, timeoutSec: 5 }); + expect(res?.status).toBe('warn'); + expect(res?.detail).toContain('< pollBudget'); + expect(res?.suggestion).toContain('120'); + }); + + test('warns when approvals are enabled but no dispatch timeout is found', () => { + const res = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: 110_000, timeoutSec: null }); + expect(res?.status).toBe('warn'); + expect(res?.detail).toContain('no `genie hook dispatch`'); + }); + + test('passes when the hook timeout meets or exceeds pollBudgetMs', () => { + const res = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: 110_000, timeoutSec: 120 }); + expect(res?.status).toBe('pass'); + }); + + test('emits no check when omni approvals are disabled', () => { + expect(evaluateOmniHookTimeout({ enabled: false, pollBudgetMs: 110_000, timeoutSec: 1 })).toBeNull(); + }); +}); + describe('doctorCommand — genie.db check branches', () => { // The db check resolves its path from the current repo root (git rev-parse), // so we drive doctorCommand inside a throwaway git repo to exercise the diff --git a/src/genie-commands/doctor.ts b/src/genie-commands/doctor.ts index 4fa76df73..d892294d5 100644 --- a/src/genie-commands/doctor.ts +++ b/src/genie-commands/doctor.ts @@ -14,8 +14,10 @@ */ import { execFileSync } from 'node:child_process'; -import { existsSync } from 'node:fs'; +import { existsSync, readFileSync } from 'node:fs'; +import { homedir } from 'node:os'; import { join } from 'node:path'; +import { resolveOmniRuntimeConfig } from '../lib/omni-config.js'; import { CURRENT_SCHEMA_VERSION, GenieDbError, openDb, resolveDbPath, resolveRepoRoot } from '../lib/v5/genie-db.js'; import { VERSION } from '../lib/version.js'; @@ -185,6 +187,92 @@ function checkBun(): CheckResult[] { ]; } +// ============================================================================ +// Omni approval hook-timeout guardrail +// ============================================================================ + +interface HookCommand { + command?: unknown; + timeout?: unknown; +} +interface HookMatcher { + hooks?: unknown; +} +interface CcSettings { + hooks?: { PreToolUse?: unknown }; +} + +/** + * Smallest `timeout` (SECONDS) among PreToolUse hooks that run `genie hook + * dispatch` in a Claude Code settings object — that is the ceiling the omni + * approval handler polls under. null when no such hook is installed. Pure + + * exported so the guardrail is unit-tested without a settings file on disk. + */ +export function findDispatchHookTimeoutSec(settings: CcSettings): number | null { + const pre = settings.hooks?.PreToolUse; + if (!Array.isArray(pre)) return null; + let min: number | null = null; + for (const entry of pre as HookMatcher[]) { + if (!Array.isArray(entry?.hooks)) continue; + for (const h of entry.hooks as HookCommand[]) { + if (typeof h?.command === 'string' && h.command.includes('hook dispatch') && typeof h.timeout === 'number') { + min = min === null ? h.timeout : Math.min(min, h.timeout); + } + } + } + return min; +} + +/** + * Compare the installed hook timeout against the approval poll budget. Returns + * null when omni approvals are off (no check emitted). A hook timeout below the + * budget is a WARN: CC kills `genie hook dispatch` before the omni handler can + * allow/deny OR reach its timeout→ask fail-safe. Pure + exported for testing. + */ +export function evaluateOmniHookTimeout(params: { + enabled: boolean; + pollBudgetMs: number; + timeoutSec: number | null; +}): CheckResult | null { + if (!params.enabled) return null; + const name = 'omni hook timeout ≥ pollBudget'; + const needSec = Math.ceil(params.pollBudgetMs / 1000); + if (params.timeoutSec === null) { + return { + name, + status: 'warn', + detail: 'omni approvals enabled but no `genie hook dispatch` PreToolUse timeout found', + suggestion: `Install the genie PreToolUse hook with a timeout ≥ ${needSec}s so approvals can resolve.`, + }; + } + const timeoutMs = params.timeoutSec * 1000; + if (timeoutMs < params.pollBudgetMs) { + return { + name, + status: 'warn', + detail: `hook timeout ${params.timeoutSec}s (${timeoutMs}ms) < pollBudget ${params.pollBudgetMs}ms — CC will kill the hook before it can allow/deny or reach its ask fail-safe`, + suggestion: `Raise the PreToolUse \`genie hook dispatch\` timeout to ≥ ${needSec}s (e.g. 120) in ~/.claude/settings.json.`, + }; + } + return { name, status: 'pass', detail: `hook timeout ${params.timeoutSec}s ≥ pollBudget ${params.pollBudgetMs}ms` }; +} + +async function checkOmniHookTimeout(): Promise { + const rt = await resolveOmniRuntimeConfig(); + if (!rt.approvals.enabled) return []; // omni off → stay silent + const settingsPath = join(homedir(), '.claude', 'settings.json'); + let timeoutSec: number | null = null; + try { + if (existsSync(settingsPath)) { + timeoutSec = findDispatchHookTimeoutSec(JSON.parse(readFileSync(settingsPath, 'utf8')) as CcSettings); + } + } catch { + timeoutSec = null; // unreadable/malformed settings → treated as "not found" + } + const result = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: rt.approvals.pollBudgetMs, timeoutSec }); + return result ? [result] : []; +} + // ============================================================================ // Entry point // ============================================================================ @@ -196,6 +284,7 @@ export async function doctorCommand(options?: { json?: boolean }): Promise ...checkDatabase(), ...checkSkills(), ...checkBun(), + ...(await checkOmniHookTimeout()), ]; const failed = results.filter((r) => r.status === 'fail'); diff --git a/src/lib/omni-config.ts b/src/lib/omni-config.ts index 7e4471e21..9f7364d36 100644 --- a/src/lib/omni-config.ts +++ b/src/lib/omni-config.ts @@ -16,6 +16,11 @@ import { loadGenieConfig } from './genie-config.js'; // Ported verbatim from origin/v4:src/lib/omni-approval-handler.ts so remote // operators keep the exact token/reaction vocabulary they trained their muscle // memory on. `sim`/`nao` are Portuguese yes/no (the original omni deployment). +// +// Vocabulary split: TEXT words (`OMNI_APPROVE_TOKENS`/`approveTokens`) vs +// REACTION emoji (`approveReactions`/`denyReactions`). Keep emoji OUT of the +// token lists — WhatsApp's bare-emoji dual-emit would echo them back and could +// double-resolve an approval. export const DEFAULT_APPROVE_TOKENS = ['y', 'yes', 'approve', 'sim']; export const DEFAULT_DENY_TOKENS = ['n', 'no', 'deny', 'nao']; export const DEFAULT_APPROVE_REACTIONS = ['\u{1F44D}', '\u{2705}', '\u{1F44C}']; // 👍 ✅ 👌 diff --git a/src/lib/omni-runner.test.ts b/src/lib/omni-runner.test.ts index a5cb28b06..d2c5ef047 100644 --- a/src/lib/omni-runner.test.ts +++ b/src/lib/omni-runner.test.ts @@ -373,3 +373,229 @@ describe('omni runner — correlated approval identity + reactions', () => { expect(getApproval(db, a)?.status).toBe('pending'); }); }); + +// --------------------------------------------------------------------------- +// ⏳→✅/❌ status-reaction lifecycle + the explicit-non-matching-target no-op +// (Group 3). Every test injects a FAKE `setReaction` recorder — zero network — +// and asserts the (target id, emoji) genie sets on its OWN approval message. +// --------------------------------------------------------------------------- +const HOURGLASS = '\u{23F3}'; // ⏳ +const CHECK = '\u{2705}'; // ✅ +const CROSS = '\u{274C}'; // ❌ +const THUMBS_DOWN = '\u{1F44E}'; // 👎 (in rt() denyReactions) + +interface ReactionCall { + messageId: string; + emoji: string; +} + +/** A status runner with a fake set-reaction recorder and a mutable clock. */ +function statusRunner(db: Database, reactions: ReactionCall[], clock: { now: number }) { + return createOmniRunner({ + db, + config: rt(), + publish: () => {}, + sendApproval, + setReaction: async ({ messageId, emoji }) => { + reactions.push({ messageId, emoji }); + return { success: true }; + }, + now: () => clock.now, + }); +} + +describe('omni runner — ⏳→✅/❌ status-reaction lifecycle', () => { + test('announce sets ⏳ on the stored stanza id', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const runner = statusRunner(db, reactions, { now: NOW }); + enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); + await runner.whenIdle(); + expect(reactions).toEqual([{ messageId: 'stanza-A', emoji: HOURGLASS }]); + }); + + test('approve swaps ⏳→✅ on the same stanza id and closes the row', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const runner = statusRunner(db, reactions, { now: NOW }); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); + await runner.whenIdle(); + + runner.handleMessage( + ...approvalInbound({ content: reactionContent(THUMBS_UP, 'stanza-A'), messageId: 'stanza-A' }), + ); + await runner.whenIdle(); + + expect(getApproval(db, a)?.status).toBe('approved'); // row closed + expect(reactions).toEqual([ + { messageId: 'stanza-A', emoji: HOURGLASS }, + { messageId: 'stanza-A', emoji: CHECK }, + ]); + }); + + test('deny swaps ⏳→❌', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const runner = statusRunner(db, reactions, { now: NOW }); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); + await runner.whenIdle(); + + runner.handleMessage( + ...approvalInbound({ content: reactionContent(THUMBS_DOWN, 'stanza-A'), messageId: 'stanza-A' }), + ); + await runner.whenIdle(); + + expect(getApproval(db, a)?.status).toBe('denied'); + expect(reactions[reactions.length - 1]).toEqual({ messageId: 'stanza-A', emoji: CROSS }); + }); + + test('expiry swaps ⏳→❌ and closes the row', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const clock = { now: NOW }; + const runner = statusRunner(db, reactions, clock); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); // announce + ⏳ + await runner.whenIdle(); + + // Advance past pollBudgetMs (10_000) so the row is stale, then tick to expire. + clock.now = NOW + 20_000; + runner.tick(); + await runner.whenIdle(); + + expect(getApproval(db, a)?.status).toBe('expired'); // row closed + expect(reactions[reactions.length - 1]).toEqual({ messageId: 'stanza-A', emoji: CROSS }); + }); + + test('reaction with an explicit non-matching target NO-OPs (does not resolve oldest)', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const runner = statusRunner(db, reactions, { now: NOW }); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + const b = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-B', now: NOW - 1000 }); + runner.tick(); + await runner.whenIdle(); + + // A 👍 targeting a stanza id no pending approval carries (already-resolved or + // unknown) must NOT fall back to resolving the oldest — the MEDIUM fix. + runner.handleMessage( + ...approvalInbound({ content: reactionContent(THUMBS_UP, 'stanza-UNKNOWN'), messageId: 'stanza-UNKNOWN' }), + ); + await runner.whenIdle(); + + expect(getApproval(db, a)?.status).toBe('pending'); + expect(getApproval(db, b)?.status).toBe('pending'); + // No ✅ was set — only the two ⏳ announces happened. + expect(reactions.filter((r) => r.emoji === CHECK)).toEqual([]); + }); + + test('records the glyph on each successful ack (⏳ on announce, ✅ on approve)', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const runner = statusRunner(db, reactions, { now: NOW }); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); + await runner.whenIdle(); + expect(getApproval(db, a)?.lastStatusGlyph).toBe(HOURGLASS); // recorded ⏳ + + runner.handleMessage( + ...approvalInbound({ content: reactionContent(THUMBS_UP, 'stanza-A'), messageId: 'stanza-A' }), + ); + await runner.whenIdle(); + expect(getApproval(db, a)?.lastStatusGlyph).toBe(CHECK); // recorded ✅ (terminal) + }); +}); + +// --------------------------------------------------------------------------- +// Reconciliation: the RUNNER is the authoritative acker even when the hook fork +// wins the expiry race (expires the row but sets NO reaction). Without this, the +// ⏳ set on announce would stick on the phone forever (the G3-review HIGH). +// --------------------------------------------------------------------------- +describe('omni runner — status-ack reconciliation (hook-fork-expiry race)', () => { + /** Faithful to the hook's expireOwnRow: expire ONE row, no status glyph. */ + function hookForkExpire(db: Database, id: string): void { + db.query("UPDATE approvals SET status = 'expired', resolved_at = ? WHERE id = ? AND status = 'pending'").run( + NOW, + id, + ); + } + + test('hook fork expires the row (no ack) → runner tick reconciles a ❌', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const runner = statusRunner(db, reactions, { now: NOW }); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); // announce + ⏳ + await runner.whenIdle(); + expect(getApproval(db, a)?.lastStatusGlyph).toBe(HOURGLASS); + + // The hook fork wins the expiry race — row is expired with a stuck ⏳. + hookForkExpire(db, a); + expect(getApproval(db, a)?.status).toBe('expired'); + expect(getApproval(db, a)?.lastStatusGlyph).toBe(HOURGLASS); // still ⏳ — no ack + + // The runner's next tick reconciles the stuck ⏳ to ❌. + runner.tick(); + await runner.whenIdle(); + expect(reactions[reactions.length - 1]).toEqual({ messageId: 'stanza-A', emoji: CROSS }); + expect(getApproval(db, a)?.lastStatusGlyph).toBe(CROSS); + }); + + test('reconciliation is idempotent — a second tick does not re-emit ❌', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + const runner = statusRunner(db, reactions, { now: NOW }); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); + await runner.whenIdle(); + hookForkExpire(db, a); + + runner.tick(); // reconciles → ❌ recorded + await runner.whenIdle(); + const countAfterFirst = reactions.length; + + runner.tick(); // glyph now terminal → nothing to reconcile + await runner.whenIdle(); + expect(reactions.length).toBe(countAfterFirst); + // Exactly one ❌ ever emitted for this row. + expect(reactions.filter((r) => r.emoji === CROSS).length).toBe(1); + }); + + test('a transport-dropped resolve ack is retried by reconciliation', async () => { + const db = freshDb(); + const reactions: ReactionCall[] = []; + // setReaction FAILS the first call (the ⏳ or the ✅), succeeds afterwards — + // so the terminal glyph is not recorded until reconciliation retries it. + let calls = 0; + const runner = createOmniRunner({ + db, + config: rt(), + publish: () => {}, + sendApproval, + setReaction: async ({ messageId, emoji }) => { + calls++; + reactions.push({ messageId, emoji }); + return calls === 2 ? { success: false, error: 'dropped' } : { success: true }; + }, + now: () => NOW, + }); + const a = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'summary-A', now: NOW - 2000 }); + runner.tick(); // announce ⏳ (call 1, ok → recorded) + await runner.whenIdle(); + + runner.handleMessage( + ...approvalInbound({ content: reactionContent(THUMBS_UP, 'stanza-A'), messageId: 'stanza-A' }), + ); + await runner.whenIdle(); // resolve ✅ (call 2, DROPPED → glyph stays ⏳) + expect(getApproval(db, a)?.status).toBe('approved'); + expect(getApproval(db, a)?.lastStatusGlyph).toBe(HOURGLASS); // not recorded — drop + + runner.tick(); // reconciliation retries ✅ (call 3, ok → recorded) + await runner.whenIdle(); + expect(getApproval(db, a)?.lastStatusGlyph).toBe(CHECK); + expect(reactions[reactions.length - 1]).toEqual({ messageId: 'stanza-A', emoji: CHECK }); + }); +}); diff --git a/src/lib/omni-runner.ts b/src/lib/omni-runner.ts index 321c67b60..4329bda83 100644 --- a/src/lib/omni-runner.ts +++ b/src/lib/omni-runner.ts @@ -37,9 +37,11 @@ import { type ApprovalDecision, attachOmniMessageId, expireStale, + listApprovalsNeedingStatusAck, listPendingApprovals, markHandled, recordInbound, + recordStatusGlyph, resolveApproval, } from './v5/omni-queue.js'; @@ -179,6 +181,71 @@ export function makeDefaultOmniSend(config: OmniRuntimeConfig): OmniSend { }; } +// ============================================================================ +// Injectable outbound set-reaction (⏳→✅/❌ status ack) +// ============================================================================ + +export interface OmniSetReactionResult { + success?: boolean; + error?: string; +} + +export interface OmniSetReactionOpts { + instance: string; + chat: string; + /** The WhatsApp stanza id of the message to react to (genie's own approval). */ + messageId: string; + /** The status emoji to set (⏳/✅/❌). A new emoji SWAPS the prior one in place. */ + emoji: string; +} + +/** + * Set (or swap) genie's OWN status reaction on a message it sent. Injectable so + * tests drive a fake (zero network); the default POSTs the Omni `--reaction` + * HTTP path. WhatsApp allows one reaction per sender per message, so setting a + * new emoji on the same stanza id replaces the prior one — that is how the + * ⏳→✅/❌ ack swaps in place. + * + * This is the seam the SPIKE-documented fallback swaps behind: if a later + * `--live` QA shows the in-place reaction swap does not RENDER, replace this + * default impl with a message-edit (prepend a status glyph) or a status-reply + * variant — the runner calls this one seam, so the fallback is an impl swap, not + * a rewrite. + */ +export type OmniSetReaction = (opts: OmniSetReactionOpts) => Promise; + +/** + * Default set-reaction — POSTs to Omni's HTTP reaction route, signed exactly + * like {@link makeDefaultOmniSend}. Uses the `--reaction` path (correct + * `fromMe` for genie's own message), NOT the bare `react` verb. Degrades to an + * error result (never a throw) when the Omni API URL is unconfigured. + * + * NOTE: like {@link makeDefaultOmniSend}, the exact reaction-route body shape is + * documented in SPIKE (c) but not yet exercised against the live hub — the + * injectable seam above is what the test suite proves; G3's `--live` round-trip + * confirms this default's wire format (and whether the in-place swap renders). + */ +export function makeDefaultOmniSetReaction(config: OmniRuntimeConfig): OmniSetReaction { + return async ({ instance, chat, messageId, emoji }) => { + const apiUrl = config.apiUrl; + if (!apiUrl) return { success: false, error: 'omni apiUrl not configured' }; + const path = '/api/v2/messages'; + const bodyJson = JSON.stringify({ instanceId: instance, chatId: chat, reaction: emoji, messageId }); + const headers: Record = { 'Content-Type': 'application/json' }; + if (config.apiKey) headers.Authorization = `Bearer ${config.apiKey}`; + const sig = signOmniRequest('POST', path, bodyJson); + if (sig) Object.assign(headers, sig); + const res = await fetch(`${apiUrl.replace(/\/+$/, '')}${path}`, { + method: 'POST', + headers, + body: bodyJson, + signal: AbortSignal.timeout(10_000), + }); + if (!res.ok) return { success: false, error: `HTTP ${res.status}` }; + return { success: true }; + }; +} + /** Default factory — dynamically imports `nats`. Only invoked by `omni serve`. */ export const defaultNatsFactory: NatsFactory = async ({ servers }) => { const nats = await import('nats'); @@ -243,6 +310,14 @@ export interface OmniRunnerDeps { * id so `announce()` is exercised with zero network. */ sendApproval?: OmniSend; + /** + * Injectable outbound set-reaction for the ⏳→✅/❌ status ack. Defaults to the + * Omni HTTP reaction path ({@link makeDefaultOmniSetReaction}); tests pass a + * fake that records the (target id, emoji) so the status lifecycle is asserted + * with zero network. The seam the SPIKE fallback (message-edit / status-reply) + * swaps behind if the in-place reaction swap does not render live. + */ + setReaction?: OmniSetReaction; } export interface OmniRunner { @@ -333,6 +408,29 @@ const errorNotice = (detail: string): string => `\u{26A0}\u{FE0F} agent run fail /** Compact message for an unknown thrown value. */ const errText = (err: unknown): string => (err instanceof Error ? err.message : String(err)); +/** + * Status-ack glyphs for genie's OWN swapping reaction on the approval message. + * ⏳ on announce (awaiting you), ✅ once approved, ❌ once denied or expired. All + * three are outside the approve/deny vocab and are set with `fromMe=true`, so + * they never dual-emit and never self-resolve an approval (SPIKE c). + */ +const STATUS_PENDING = '\u{23F3}'; // ⏳ +const STATUS_APPROVED = '\u{2705}'; // ✅ +const STATUS_DENIED = '\u{274C}'; // ❌ (denied and expired both land here) +/** The two glyphs that mean "done" — a row whose recorded glyph is one of these + * needs no further status ack (the reconciliation pass skips it). */ +const TERMINAL_STATUS_GLYPHS = [STATUS_APPROVED, STATUS_DENIED]; + +/** + * How far back the reconciliation pass looks for a row still needing a terminal + * ack (24h). Generous on purpose: a row resolved/expired while `omni serve` was + * briefly down must still get its ✅/❌ when serve returns, so this must exceed + * any realistic downtime — it is ~785× the default 110s pollBudget. Older closed + * rows are presumed already acked (or too stale to matter) and are left alone, so + * per-tick reconciliation work is bounded and history is never swept. + */ +const RECONCILE_WINDOW_MS = 24 * 60 * 60 * 1000; + /** Distinguishes a budget-driven abort from a genuine child crash in the catch. */ class OneShotTimeoutError extends Error { constructor() { @@ -349,6 +447,7 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { deps.genCorrelationId ?? (() => `req_${Date.now().toString(36)}${Math.random().toString(36).slice(2, 10)}`); const spawnClaude = deps.spawnClaude ?? defaultSpawnClaude; const sendApproval = deps.sendApproval ?? makeDefaultOmniSend(config); + const setReaction = deps.setReaction ?? makeDefaultOmniSetReaction(config); const vocab = { approveTokens: config.approveTokens, denyTokens: config.denyTokens, @@ -369,6 +468,12 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { const announcing = new Set(); /** Live announce-send promises, drained by `whenIdle` alongside `pending`. */ const inFlightSends = new Set>(); + /** Live status-reaction promises (⏳→✅/❌), also drained by `whenIdle`. */ + const inFlightReactions = new Set>(); + /** Omni message ids with a status-ack HTTP call currently in flight — guards + * the reconciliation pass (every tick) from double-firing an ack whose glyph + * the prior emit has not yet recorded. Keyed by the stanza id. */ + const ackInFlight = new Set(); const routeKey = (instance: string, chat: string): string => `${instance}${chat}`; const findRoute = (instance: string, chat: string): OmniRoute | undefined => routes.find((r) => r.instance === instance && r.chat === chat); @@ -434,12 +539,50 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { pending.add(run); } + /** + * Set (or swap) genie's OWN status reaction (⏳/✅/❌) on the approval message. + * Fire-and-forget behind the injectable {@link setReaction} seam and drained by + * `whenIdle`, so the resolve/announce hot paths never block on the HTTP call. A + * missing target id (send failed → row un-tagged) is a no-op, as is a target + * whose ack is already in flight (the reconciliation guard). + * + * The glyph is recorded (via {@link recordStatusGlyph}) ONLY on a confirmed + * successful set: a dropped/failed react leaves `last_status_glyph` unchanged so + * the reconciliation pass retries it next tick. Never throws — a failed status + * reaction is logged and the approval decision still stands. + */ + function emitStatusReaction(targetId: string | null | undefined, emoji: string): void { + if (!targetId || ackInFlight.has(targetId)) return; + ackInFlight.add(targetId); + const react = setReaction({ + instance: config.instance ?? '', + chat: config.approvalChat ?? '', + messageId: targetId, + emoji, + }) + .then((res) => { + if (res && res.success === false) { + log(`[omni] status ${emoji} on ${targetId} failed${res.error ? ` (${res.error})` : ''}`); + return; + } + recordStatusGlyph(db, targetId, emoji); // persist only on confirmed success + }) + .catch((err) => log(`[omni] status ${emoji} on ${targetId} failed: ${errText(err)}`)) + .finally(() => { + ackInFlight.delete(targetId); + inFlightReactions.delete(react); + }); + inFlightReactions.add(react); + } + /** * Announce a single pending approval via the id-returning send and tag the row * with the REAL Omni message id (the stanza id) the send returns — so an * inbound reaction correlates to THIS approval. Tags only on a successful send; * a failed/id-less send leaves the row un-tagged so the next tick retries - * (mirrors the old tag-after-publish semantics). Never throws. + * (mirrors the old tag-after-publish semantics). On a successful tag it sets the + * ⏳ status reaction on the sent message (SPIKE c) — the first half of the + * ⏳→✅/❌ ack. Never throws. */ async function sendAnnounce(appr: { id: string; tool: string; inputSummary: string }): Promise { const text = formatApprovalMessage(appr.tool, appr.inputSummary); @@ -453,6 +596,7 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { } attachOmniMessageId(db, appr.id, result.messageId); log(`[omni] announced approval ${appr.id} (omni ${result.messageId})`); + emitStatusReaction(result.messageId, STATUS_PENDING); // ⏳ awaiting you } catch (err) { log(`[omni] announce ${appr.id} failed: ${errText(err)}`); } @@ -484,8 +628,13 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { function tryResolve(id: string, decision: ApprovalDecision, resolvedBy: string, note: string): boolean { try { - resolveApproval(db, id, decision, resolvedBy); + // Stamp resolved_at with the runner clock so the reconciliation recency + // window (which reads now() from the same dep) lines up deterministically. + const row = resolveApproval(db, id, decision, resolvedBy, now()); log(`[omni] resolved ${id} → ${decision} by ${resolvedBy} (${note})`); + // Swap the ⏳ status reaction to ✅ (approved) / ❌ (denied) — the row is + // now closed, so this is the second half of the ⏳→✅/❌ ack. + emitStatusReaction(row.omniMessageId, decision === 'approved' ? STATUS_APPROVED : STATUS_DENIED); return true; } catch (err) { // Lost the race to another resolver — benign under concurrency. @@ -497,8 +646,12 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { /** * Resolve an approval from an inbound reaction. Correlates by the stored real * Omni message id (the reaction's target stanza id, from the `[Reaction: … on - * message ]` content or the top-level `messageId`); falls back to oldest - * only when no pending row carries that id. Non-vocabulary emoji are ignored. + * message ]` content or the top-level `messageId`). An EXPLICIT target that + * matches no pending row is a NO-OP (logged, ignored) — NOT an oldest fallback: + * a reaction on an already-resolved / unknown message must never resolve some + * unrelated pending approval (the concurrency hazard this wish exists to kill). + * Oldest fallback is reserved for the no-target-id case; reactions always carry + * a target id, so this branch is defensive. Non-vocabulary emoji are ignored. */ function resolveReaction(emoji: string, targetId: string | undefined, sender: string): void { const decision = matchReaction(emoji, vocab); @@ -507,15 +660,37 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { const match = listPendingApprovals(db).find((a) => a.omniMessageId === targetId); if (match) { tryResolve(match.id, decision, sender, `reaction ${emoji}`); - return; + } else { + log(`[omni] reaction ${emoji} targets unknown/resolved id ${targetId} — ignored (no oldest fallback)`); } + return; } resolveOldest(decision, sender, `reaction ${emoji} (fallback)`); } + /** + * Reconcile the status reaction on every DONE-but-not-terminally-acked row: + * swap its ⏳ (or missing) glyph to ✅ (approved) / ❌ (denied or expired). This + * makes the RUNNER the authoritative acker regardless of WHO closed the row — + * closing the hook-fork-expiry race (the hook's `expireOwnRow` expires the row + * but sets no reaction, so a ⏳ would otherwise stick on the phone forever) AND + * any transport-dropped swap (a failed resolve/announce react left the glyph + * non-terminal). Idempotent: {@link emitStatusReaction} records the terminal + * glyph on success, so the next tick's query no longer returns the row; the + * `ackInFlight` guard stops a slow ack re-firing before its glyph is recorded. + */ + function reconcileStatusAcks(): void { + for (const row of listApprovalsNeedingStatusAck(db, TERMINAL_STATUS_GLYPHS, now(), RECONCILE_WINDOW_MS)) { + emitStatusReaction(row.omniMessageId, row.status === 'approved' ? STATUS_APPROVED : STATUS_DENIED); + } + } + return { tick(): void { + // Expire stale rows, then let reconciliation set the terminal ✅/❌ on any + // row closed here OR by the hook fork's own self-timeout expiry. expireStale(db, config.approvals.pollBudgetMs, now()); + reconcileStatusAcks(); announce(); }, @@ -563,11 +738,12 @@ export function createOmniRunner(deps: OmniRunnerDeps): OmniRunner { }, async whenIdle(): Promise { - // Drain both one-shot runs and announce sends in a loop: a settling task - // must not add new work, but snapshot-await is cheap insurance against - // future callers that fan out mid-drain. - while (pending.size > 0 || inFlightSends.size > 0) { - await Promise.allSettled([...pending, ...inFlightSends]); + // Drain one-shot runs, announce sends, AND status reactions in a loop: a + // send settles by firing the ⏳ status reaction (adds to inFlightReactions + // mid-drain), and a resolve fires ✅/❌, so the loop must keep going until + // every set has quiesced. + while (pending.size > 0 || inFlightSends.size > 0 || inFlightReactions.size > 0) { + await Promise.allSettled([...pending, ...inFlightSends, ...inFlightReactions]); } }, }; diff --git a/src/lib/v5/global-db.test.ts b/src/lib/v5/global-db.test.ts index fb75fd6b1..e2ef4b7ea 100644 --- a/src/lib/v5/global-db.test.ts +++ b/src/lib/v5/global-db.test.ts @@ -6,10 +6,12 @@ import { join } from 'node:path'; import { ForeignDbError, GLOBAL_SCHEMA_VERSION, + MIGRATED_STATUS_SENTINEL, MalformedDbError, openGlobalDb, resolveGlobalDbPath, } from './global-db.js'; +import { listApprovalsNeedingStatusAck } from './omni-queue.js'; let dir: string; const originalGenieHome = process.env.GENIE_HOME; @@ -78,6 +80,91 @@ describe('openGlobalDb schema init', () => { }); }); +describe('openGlobalDb additive column backfill (last_status_glyph)', () => { + /** The pre-column v1 approvals schema, as an earlier build stamped it. */ + const OLD_APPROVALS_SQL = ` + CREATE TABLE approvals ( + id TEXT PRIMARY KEY, repo TEXT NOT NULL, session_hint TEXT, tool TEXT NOT NULL, + input_summary TEXT NOT NULL, + status TEXT NOT NULL CHECK (status IN ('pending','approved','denied','expired')), + omni_message_id TEXT, requested_by TEXT, resolved_by TEXT, + created_at INTEGER NOT NULL, resolved_at INTEGER + ); + CREATE TABLE inbound_messages ( + id TEXT PRIMARY KEY, instance TEXT NOT NULL, chat TEXT NOT NULL, sender TEXT NOT NULL, + body TEXT NOT NULL, received_at INTEGER NOT NULL, handled_at INTEGER + );`; + + function columns(db: Database, table: string): Set { + return new Set((db.query(`PRAGMA table_info(${table})`).all() as Array<{ name: string }>).map((c) => c.name)); + } + + test('backfills the column on an existing v1 DB without bumping user_version or losing data', () => { + const path = join(dir, 'legacy.db'); + const seed = new Database(path); + seed.exec(OLD_APPROVALS_SQL); + seed.exec(`PRAGMA user_version = ${GLOBAL_SCHEMA_VERSION}`); + // A pre-existing approval row must survive the additive migration untouched. + seed + .query( + "INSERT INTO approvals (id, repo, tool, input_summary, status, created_at) VALUES ('appr_old','/r','Bash','x','pending',1)", + ) + .run(); + seed.close(); + expect(columns(new Database(path, { readonly: true }), 'approvals').has('last_status_glyph')).toBe(false); + + // Opening runs the additive ALTER (schemaIsCurrent sees the missing column). + const db = openGlobalDb({ path }); + try { + expect(columns(db, 'approvals').has('last_status_glyph')).toBe(true); + const row = db.query("SELECT last_status_glyph FROM approvals WHERE id = 'appr_old'").get() as { + last_status_glyph: string | null; + }; + expect(row.last_status_glyph).toBeNull(); // new column, back-filled NULL + } finally { + db.close(); + } + // Same version — additive backfill stays within v1 (no destructive migration). + expect(userVersion(path)).toBe(GLOBAL_SCHEMA_VERSION); + }); + + test('one-time backfill stamps pre-upgrade CLOSED history as MIGRATED so reconcile ignores it', () => { + const path = join(dir, 'legacy-history.db'); + const seed = new Database(path); + seed.exec(OLD_APPROVALS_SQL); + seed.exec(`PRAGMA user_version = ${GLOBAL_SCHEMA_VERSION}`); + // Historical announced+closed rows (approved/denied/expired) — the kind that, + // pre-fix, the first post-upgrade tick would fire a reaction POST for. + const insert = seed.query( + 'INSERT INTO approvals (id, repo, tool, input_summary, status, omni_message_id, created_at, resolved_at) VALUES (?,?,?,?,?,?,?,?)', + ); + insert.run('appr_a', '/r', 'Bash', 'x', 'approved', 'stanza-a', 1, 100); + insert.run('appr_d', '/r', 'Bash', 'x', 'denied', 'stanza-d', 2, 200); + insert.run('appr_e', '/r', 'Bash', 'x', 'expired', 'stanza-e', 3, 300); + // A still-pending row must stay reconcilable (not stamped). + insert.run('appr_p', '/r', 'Bash', 'x', 'pending', 'stanza-p', 4, null); + seed.close(); + + const db = openGlobalDb({ path }); + try { + const closed = db + .query("SELECT id, last_status_glyph AS g FROM approvals WHERE status != 'pending'") + .all() as Array<{ id: string; g: string | null }>; + // Every historical closed row is stamped with the sentinel. + expect(closed.every((r) => r.g === MIGRATED_STATUS_SENTINEL)).toBe(true); + // Pending row untouched. + expect( + (db.query("SELECT last_status_glyph AS g FROM approvals WHERE id = 'appr_p'").get() as { g: string | null }).g, + ).toBeNull(); + // The reconciliation query returns NONE — no reaction fires on the first tick. + const now = 1_000_000; + expect(listApprovalsNeedingStatusAck(db, ['\u{2705}', '\u{274C}'], now, 24 * 60 * 60 * 1000)).toEqual([]); + } finally { + db.close(); + } + }); +}); + describe('openGlobalDb refusal (typed errors from sqlite-open)', () => { test('refuses a malformed (non-sqlite) file with MalformedDbError', () => { const path = join(dir, 'garbage.db'); diff --git a/src/lib/v5/global-db.ts b/src/lib/v5/global-db.ts index 584e1e638..c51c8b491 100644 --- a/src/lib/v5/global-db.ts +++ b/src/lib/v5/global-db.ts @@ -28,6 +28,16 @@ export { BusyDbError, ForeignDbError, GenieDbError, isBusyError, MalformedDbErro /** Schema revision for the GLOBAL database. Independent of the per-repo version. */ export const GLOBAL_SCHEMA_VERSION = 1; +/** + * Sentinel written into `last_status_glyph` for pre-existing closed approvals at + * the moment the column is first added (the one-time upgrade backfill). It is + * NOT a real reaction emoji, so it can never collide with a live status glyph; + * `listApprovalsNeedingStatusAck` treats it as terminal so the runner's + * reconciliation pass NEVER touches historical rows on the first post-upgrade + * tick (which would otherwise fire a reaction HTTP POST for the entire history). + */ +export const MIGRATED_STATUS_SENTINEL = 'migrated'; + /** * Base directory for all machine-scope genie state. Read from `GENIE_HOME` on * every call (not cached) so env overrides in tests take effect. @@ -76,7 +86,10 @@ function schemaIsCurrent(db: Database): boolean { ).map((r) => r.name), ); for (const t of EXPECTED_TABLES) if (!tables.has(t)) return false; - return true; + // A pre-column v1 DB (missing the additive last_status_glyph) returns false so + // ensureSchema runs and backfills — mirrors genie-db.ts's ensureTaskColumns. + const cols = new Set((db.query('PRAGMA table_info(approvals)').all() as Array<{ name: string }>).map((c) => c.name)); + return cols.has('last_status_glyph'); } const SCHEMA_SQL = ` @@ -91,7 +104,8 @@ CREATE TABLE IF NOT EXISTS approvals ( requested_by TEXT, resolved_by TEXT, created_at INTEGER NOT NULL, - resolved_at INTEGER + resolved_at INTEGER, + last_status_glyph TEXT ); CREATE TABLE IF NOT EXISTS inbound_messages ( @@ -111,4 +125,25 @@ CREATE INDEX IF NOT EXISTS idx_inbound_handled ON inbound_messages(handled_at); /** Create every table/index if absent. Idempotent — pure `IF NOT EXISTS`. */ 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); + } } diff --git a/src/lib/v5/omni-queue.test.ts b/src/lib/v5/omni-queue.test.ts index 219dc5b43..85ed904f7 100644 --- a/src/lib/v5/omni-queue.test.ts +++ b/src/lib/v5/omni-queue.test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { openGlobalDb } from './global-db.js'; +import { MIGRATED_STATUS_SENTINEL, openGlobalDb } from './global-db.js'; import { ApprovalConflictError, UnknownApprovalError, @@ -12,13 +12,23 @@ import { enqueueApproval, expireStale, getApproval, + listApprovalsNeedingStatusAck, listInbox, listPendingApprovals, markHandled, recordInbound, + recordStatusGlyph, resolveApproval, } from './omni-queue.js'; +const HOURGLASS = '\u{23F3}'; // ⏳ +const CHECK = '\u{2705}'; // ✅ +const CROSS = '\u{274C}'; // ❌ +const TERMINAL = [CHECK, CROSS]; +// Deterministic clock + recency window for the reconciliation-query tests. +const NOW_Q = 5_000_000; +const WINDOW = 1_000_000; + let dir: string; let db: Database; const originalGenieHome = process.env.GENIE_HOME; @@ -122,6 +132,90 @@ describe('approvals', () => { }); }); +// --------------------------------------------------------------------------- +// Status-ack bookkeeping + reconciliation query +// --------------------------------------------------------------------------- +describe('status-ack glyph + listApprovalsNeedingStatusAck', () => { + test('enqueue defaults lastStatusGlyph to null', () => { + const id = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'x' }); + expect(getApproval(db, id)?.lastStatusGlyph).toBeNull(); + }); + + test('recordStatusGlyph sets the glyph keyed by omni_message_id; no-op for an unknown id', () => { + const id = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'x' }); + attachOmniMessageId(db, id, 'stanza-1'); + recordStatusGlyph(db, 'stanza-1', HOURGLASS); + expect(getApproval(db, id)?.lastStatusGlyph).toBe(HOURGLASS); + // No row carries this message id → silent no-op, never throws. + expect(() => recordStatusGlyph(db, 'stanza-absent', CHECK)).not.toThrow(); + expect(getApproval(db, id)?.lastStatusGlyph).toBe(HOURGLASS); + }); + + test('reconciliation query returns only done + announced + non-terminal + recent rows', () => { + // (1) pending + announced → excluded (still awaiting a decision) + const pending = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'p', now: 1 }); + attachOmniMessageId(db, pending, 'stanza-p'); + + // (2) resolved but NEVER announced (no omni_message_id) → excluded + const noId = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'n', now: 2 }); + resolveApproval(db, noId, 'approved', 'x', NOW_Q); + + // (3) approved + announced + still ⏳ → INCLUDED (needs ✅ swap) + const stuck = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 's', now: 3 }); + attachOmniMessageId(db, stuck, 'stanza-s'); + recordStatusGlyph(db, 'stanza-s', HOURGLASS); + resolveApproval(db, stuck, 'approved', 'x', NOW_Q); + + // (4) expired + announced + glyph NULL (hook-fork race) → INCLUDED (needs ❌). + // Faithful to the hook's single-row expireOwnRow: expire ONLY this row, no + // status glyph — so the runner never acked it and a ⏳ would be stuck. + const raced = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'r', now: 4 }); + attachOmniMessageId(db, raced, 'stanza-r'); + db.query("UPDATE approvals SET status = 'expired', resolved_at = ? WHERE id = ?").run(NOW_Q, raced); + + // (5) denied + announced + already ❌ → excluded (terminal) + const done = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'd', now: 6 }); + attachOmniMessageId(db, done, 'stanza-d'); + recordStatusGlyph(db, 'stanza-d', CROSS); + resolveApproval(db, done, 'denied', 'x', NOW_Q); + + const need = listApprovalsNeedingStatusAck(db, TERMINAL, NOW_Q, WINDOW); + expect(need.map((r) => r.id).sort()).toEqual([stuck, raced].sort()); + }); + + test('the migration sentinel is treated as terminal — historical rows are excluded', () => { + const migrated = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'h', now: 1 }); + attachOmniMessageId(db, migrated, 'stanza-h'); + resolveApproval(db, migrated, 'approved', 'x', NOW_Q); + recordStatusGlyph(db, 'stanza-h', MIGRATED_STATUS_SENTINEL); // as the upgrade backfill stamps + expect(listApprovalsNeedingStatusAck(db, TERMINAL, NOW_Q, WINDOW)).toEqual([]); + }); + + test('recency window: a row resolved before the window is excluded, a recent one included', () => { + // Resolved WELL before the window → excluded even though its glyph is ⏳. + const old = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'old', now: 1 }); + attachOmniMessageId(db, old, 'stanza-old'); + recordStatusGlyph(db, 'stanza-old', HOURGLASS); + resolveApproval(db, old, 'approved', 'x', NOW_Q - WINDOW - 1); + + // Resolved just inside the window → included. + const recent = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'new', now: 2 }); + attachOmniMessageId(db, recent, 'stanza-new'); + recordStatusGlyph(db, 'stanza-new', HOURGLASS); + resolveApproval(db, recent, 'approved', 'x', NOW_Q - WINDOW + 1); + + const need = listApprovalsNeedingStatusAck(db, TERMINAL, NOW_Q, WINDOW); + expect(need.map((r) => r.id)).toEqual([recent]); + }); + + test('empty terminal set returns nothing (guard against invalid NOT IN ())', () => { + const id = enqueueApproval(db, { repo: '/r', tool: 'Bash', inputSummary: 'x' }); + attachOmniMessageId(db, id, 'stanza-x'); + resolveApproval(db, id, 'approved', 'x', NOW_Q); + expect(listApprovalsNeedingStatusAck(db, [], NOW_Q, WINDOW)).toEqual([]); + }); +}); + // --------------------------------------------------------------------------- // Expiry // --------------------------------------------------------------------------- diff --git a/src/lib/v5/omni-queue.ts b/src/lib/v5/omni-queue.ts index 2a6dcf31d..bafe558a0 100644 --- a/src/lib/v5/omni-queue.ts +++ b/src/lib/v5/omni-queue.ts @@ -22,6 +22,7 @@ import type { Database } from 'bun:sqlite'; import { randomBytes } from 'node:crypto'; +import { MIGRATED_STATUS_SENTINEL } from './global-db.js'; // ============================================================================ // Types @@ -38,6 +39,10 @@ export interface ApprovalRow { inputSummary: string; status: ApprovalStatus; omniMessageId: string | null; + /** Last status-ack glyph the runner successfully set on the Omni message + * (⏳ on announce, ✅/❌ once terminal). null until the first ack lands. Drives + * the runner's reconciliation pass so a stuck ⏳ is never left on the phone. */ + lastStatusGlyph: string | null; requestedBy: string | null; resolvedBy: string | null; createdAt: number; @@ -141,6 +146,7 @@ interface RawApproval { input_summary: string; status: string; omni_message_id: string | null; + last_status_glyph: string | null; requested_by: string | null; resolved_by: string | null; created_at: number; @@ -156,6 +162,7 @@ function mapApproval(r: RawApproval): ApprovalRow { inputSummary: r.input_summary, status: r.status as ApprovalStatus, omniMessageId: r.omni_message_id, + lastStatusGlyph: r.last_status_glyph, requestedBy: r.requested_by, resolvedBy: r.resolved_by, createdAt: r.created_at, @@ -219,6 +226,61 @@ export function attachOmniMessageId(db: Database, id: string, omniMessageId: str if (res.changes !== 1) throw new UnknownApprovalError(id); } +/** + * Record the status-ack glyph the runner just SUCCESSFULLY set on an approval's + * Omni message, keyed by the message id (the stanza id both the ack and inbound + * reactions correlate on). Advisory bookkeeping — a no-op when no row carries + * that message id (e.g. the row was pruned), so it never throws and never wedges + * the runner. This is what makes the reconciliation query + * ({@link listApprovalsNeedingStatusAck}) idempotent: a recorded terminal glyph + * stops the pass re-firing every tick. + */ +export function recordStatusGlyph(db: Database, omniMessageId: string, glyph: string): void { + db.query('UPDATE approvals SET last_status_glyph = ? WHERE omni_message_id = ?').run(glyph, omniMessageId); +} + +/** + * Rows whose status-ack is stale: RESOLVED or EXPIRED (no longer pending), + * already ANNOUNCED (an omni_message_id to react on), yet whose last recorded + * glyph is not terminal (still ⏳, or never acked). These are the rows the runner + * must swap to a terminal ✅/❌ — the hook-fork-expiry race and any + * transport-dropped swap both land here. Oldest first. + * + * Two bounds keep this from ever sweeping history: + * - the `MIGRATED_STATUS_SENTINEL` is always excluded, so the one-time upgrade + * backfill's pre-upgrade closed rows are ignored; + * - `resolved_at >= now - windowMs` caps the pass to RECENTLY-closed rows, so + * per-tick work is bounded and long-dead history is never re-acked. The + * window is deliberately generous (see the runner) so a legitimately recent + * row is not dropped just because `omni serve` was briefly down. + * + * `terminalGlyphs` must be non-empty (the runner passes [✅, ❌]). + */ +export function listApprovalsNeedingStatusAck( + db: Database, + terminalGlyphs: string[], + now: number, + windowMs: number, +): ApprovalRow[] { + if (terminalGlyphs.length === 0) return []; + // The migration sentinel is terminal for reconciliation purposes — a pre-upgrade + // closed row must never be re-acked, regardless of what the runner passes. + const excluded = [...terminalGlyphs, MIGRATED_STATUS_SENTINEL]; + const placeholders = excluded.map(() => '?').join(', '); + const cutoff = now - windowMs; + const rows = db + .query( + `SELECT * FROM approvals + WHERE status != 'pending' + AND omni_message_id IS NOT NULL + AND (last_status_glyph IS NULL OR last_status_glyph NOT IN (${placeholders})) + AND resolved_at >= ? + ORDER BY created_at ASC`, + ) + .all(...excluded, cutoff) as RawApproval[]; + return rows.map(mapApproval); +} + /** * Atomically resolve a pending approval to `approved`/`denied`. The conditional * UPDATE (`WHERE id = ? AND status = 'pending'`) runs in an IMMEDIATE diff --git a/src/term-commands/omni.test.ts b/src/term-commands/omni.test.ts index 8a9ff2e60..c04278679 100644 --- a/src/term-commands/omni.test.ts +++ b/src/term-commands/omni.test.ts @@ -280,6 +280,32 @@ describe('transport is not initialized without `omni serve`', () => { }); }); +describe('omni test-approval — fake round-trip (no network)', () => { + /** Capture stdout during `fn`, restoring the real writer afterwards. */ + async function captureStdout(fn: () => Promise): Promise { + const realWrite = process.stdout.write.bind(process.stdout); + let buffer = ''; + process.stdout.write = ((chunk: string) => { + buffer += chunk; + return true; + }) as typeof process.stdout.write; + try { + await fn(); + return buffer; + } finally { + process.stdout.write = realWrite; + } + } + + test('drives one clean ⏳→✅ round-trip and prints a success line', async () => { + const output = await captureStdout(() => omniTest.testApprovalCommand({})); + expect(output).toMatch(/round-trip OK/); + expect(output).toMatch(/approved/); + // The fake path is fully offline — no NATS transport ever opened. + expect(natsConnectionCount()).toBe(0); + }); +}); + describe('omni handshake keypair provisioning', () => { test('generateAndPersistKeypair writes a 0600 private key and a raw base64url pubkey', () => { const home = mkdtempSync(join(tmpdir(), 'omni-hs-')); diff --git a/src/term-commands/omni.ts b/src/term-commands/omni.ts index 76175d42b..651d1584f 100644 --- a/src/term-commands/omni.ts +++ b/src/term-commands/omni.ts @@ -17,11 +17,26 @@ import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'n import { homedir, hostname as osHostname } from 'node:os'; import { dirname, join, resolve as resolvePath } from 'node:path'; import type { Command } from 'commander'; -import { isOmniApprovalEnabled, resolveOmniRuntimeConfig } from '../lib/omni-config.js'; +import { + DEFAULT_APPROVE_REACTIONS, + DEFAULT_APPROVE_TOKENS, + DEFAULT_DENY_REACTIONS, + DEFAULT_DENY_TOKENS, + type OmniRuntimeConfig, + isOmniApprovalEnabled, + resolveOmniRuntimeConfig, +} from '../lib/omni-config.js'; import { resolveOmniApiKey, resolveOmniApiUrl } from '../lib/omni-registration.js'; -import { runOmniServe } from '../lib/omni-runner.js'; +import { + type OmniSend, + type OmniSetReaction, + createOmniRunner, + makeDefaultOmniSend, + makeDefaultOmniSetReaction, + runOmniServe, +} from '../lib/omni-runner.js'; import { openGlobalDb } from '../lib/v5/global-db.js'; -import { listInbox } from '../lib/v5/omni-queue.js'; +import { type ApprovalRow, enqueueApproval, getApproval, listInbox } from '../lib/v5/omni-queue.js'; function out(line = ''): void { process.stdout.write(`${line}\n`); @@ -159,6 +174,136 @@ async function inboxCommand(opts: { } } +// ============================================================================ +// omni test-approval — one deliberate approval round-trip (fake by default) +// ============================================================================ + +const TEST_STANZA_ID = 'test-approval-stanza'; +const THUMBS_UP = '\u{1F44D}'; + +/** A minimal fully-populated runtime config for the CI-safe fake round-trip. */ +function fakeApprovalConfig(): OmniRuntimeConfig { + return { + natsUrl: 'localhost:4222', + instance: 'test-instance', + approvalChat: 'test-chat', + approveTokens: DEFAULT_APPROVE_TOKENS, + denyTokens: DEFAULT_DENY_TOKENS, + approveReactions: DEFAULT_APPROVE_REACTIONS, + denyReactions: DEFAULT_DENY_REACTIONS, + approvals: { enabled: true, toolMatcher: '^Bash$', pollBudgetMs: 60_000, pollIntervalMs: 400 }, + }; +} + +/** + * Drive ONE approval round-trip against the given transport: enqueue → announce + * (send + ⏳) → a 👍 reaction on the stored stanza id → resolve (✅). Shared by + * the fake and `--live` paths; the only difference is which `sendApproval` / + * `setReaction` are injected. Returns the resolved row so the caller can report. + */ +async function driveApprovalRoundTrip( + db: ReturnType, + config: OmniRuntimeConfig, + sendApproval: OmniSend, + setReaction: OmniSetReaction, +): Promise { + const runner = createOmniRunner({ db, config, publish: () => {}, sendApproval, setReaction }); + const id = enqueueApproval(db, { + repo: process.cwd(), + tool: 'TestApproval', + inputSummary: 'genie omni test-approval', + }); + + runner.tick(); // announce → send returns the stanza id → ⏳ status reaction + await runner.whenIdle(); + + const stanza = getApproval(db, id)?.omniMessageId; + if (stanza) { + // Simulate the human's 👍 on the approval message (correlated by stanza id). + runner.handleMessage( + `omni.message.${config.instance}.${config.approvalChat}`, + JSON.stringify({ + content: `[Reaction: ${THUMBS_UP} on message ${stanza}]`, + messageId: stanza, + chatId: config.approvalChat, + instanceId: config.instance, + sender: 'test-approval', + }), + ); + await runner.whenIdle(); // let the ✅ swap fire + } + return getApproval(db, id); +} + +async function testApprovalCommand(opts: { live?: boolean }): Promise { + if (opts.live) { + await runLiveTestApproval(); + return; + } + + // Fake, CI-safe path: in-memory DB + recording transports, zero network. + const db = openGlobalDb({ path: ':memory:' }); + try { + const reactions: Array<{ messageId: string; emoji: string }> = []; + const sent: string[] = []; + const sendApproval: OmniSend = async ({ text }) => { + sent.push(text); + return { success: true, messageId: TEST_STANZA_ID }; + }; + const setReaction: OmniSetReaction = async ({ messageId, emoji }) => { + reactions.push({ messageId, emoji }); + return { success: true }; + }; + const row = await driveApprovalRoundTrip(db, fakeApprovalConfig(), sendApproval, setReaction); + + const pending = reactions.find((r) => r.emoji === '\u{23F3}'); + const approved = reactions.find((r) => r.emoji === '\u{2705}'); + const ok = row?.status === 'approved' && Boolean(pending) && Boolean(approved) && sent.length === 1; + if (!ok) { + fail( + `test-approval round-trip did not complete cleanly (status=${row?.status ?? 'none'}, ` + + `sends=${sent.length}, ⏳=${Boolean(pending)}, ✅=${Boolean(approved)})`, + ); + } + out('genie omni test-approval (fake transport, no network)'); + out(` send: 1 approval message → stanza ${TEST_STANZA_ID}`); + out(' status: ⏳ set on announce → ✅ swapped on approve'); + out(' resolved: approved — round-trip OK'); + } finally { + db.close(); + } +} + +/** + * ONE real approval round-trip against the configured instance/approvalChat. + * Emits exactly ONE real WhatsApp message plus its ⏳→✅ status reactions, then + * resolves it locally so the operator can eyeball the in-place swap (the sole + * SPIKE-c empirical unknown). Deliberate manual escape hatch — NEVER called by + * tests. + */ +async function runLiveTestApproval(): Promise { + const rt = await resolveOmniRuntimeConfig(); + if (!isOmniApprovalEnabled(rt)) { + fail( + 'Omni approvals are not enabled. `--live` needs omni.approvals.enabled=true + omni.instance + omni.approvalChat.', + ); + } + out('genie omni test-approval --live — sending ONE real WhatsApp approval message.'); + const db = openGlobalDb(); + try { + const row = await driveApprovalRoundTrip(db, rt, makeDefaultOmniSend(rt), makeDefaultOmniSetReaction(rt)); + if (row?.status !== 'approved') { + fail( + `live round-trip did not resolve to approved (status=${row?.status ?? 'none'}, omniId=${row?.omniMessageId ?? 'none'})`, + ); + } + out(` sent + resolved on instance ${rt.instance} chat ${rt.approvalChat} (omni id ${row?.omniMessageId ?? '?'})`); + out(' Check WhatsApp: the approval message should show ⏳ swapped to ✅ in place.'); + } finally { + db.close(); + } +} + // ============================================================================ // omni handshake — ported from origin/v4:src/term-commands/omni/handshake.ts // ============================================================================ @@ -374,6 +519,14 @@ export function registerOmniCommands(program: Command): void { await inboxCommand(opts); }); + omni + .command('test-approval') + .description('Drive one approval round-trip (enqueue → ⏳ → approve → ✅). Fake transport by default.') + .option('--live', 'Send ONE real WhatsApp approval to the configured instance/approvalChat (deliberate)') + .action(async (opts: { live?: boolean }) => { + await testApprovalCommand(opts); + }); + omni .command('handshake') .description('Register this genie host with the omni server (ed25519 keypair, idempotent)') @@ -396,4 +549,5 @@ export const __test__ = { loadHostJson, writeHostJson, generateAndPersistKeypair, + testApprovalCommand, }; diff --git a/src/types/genie-config.ts b/src/types/genie-config.ts index 98e068ed9..8487c7c3c 100644 --- a/src/types/genie-config.ts +++ b/src/types/genie-config.ts @@ -90,10 +90,16 @@ const OmniApprovalsConfigSchema = z.object({ pollBudgetMs: z.number().default(110_000), /** Poll interval while waiting for a resolution (ms). */ pollIntervalMs: z.number().default(400), - /** Approve/deny text tokens (case-insensitive). Empty → runner defaults. */ + /** + * Approve/deny TEXT tokens (case-insensitive). Empty → runner defaults. + * Put words here (`y`, `sim`), NOT emoji — an emoji placed in a token list + * would be echoed back by WhatsApp's bare-emoji dual-emit and could + * double-resolve. Emoji belong in `approveReactions`/`denyReactions` below. + */ approveTokens: z.array(z.string()).optional(), denyTokens: z.array(z.string()).optional(), - /** Approve/deny reaction emoji. Empty → runner defaults. */ + /** Approve/deny REACTION emoji (👍/👎). Empty → runner defaults. Emoji go + * here, never in the `*Tokens` lists above. */ approveReactions: z.array(z.string()).optional(), denyReactions: z.array(z.string()).optional(), }); From 301d5b226284ff2a66b7c624e58b8021317f81cf Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 03:26:45 -0300 Subject: [PATCH 12/18] docs(omni-approval-ux): record the LOW upgrade-instant residual --- .genie/wishes/omni-approval-ux/WISH.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.genie/wishes/omni-approval-ux/WISH.md b/.genie/wishes/omni-approval-ux/WISH.md index adaf3e334..80b11a0e6 100644 --- a/.genie/wishes/omni-approval-ux/WISH.md +++ b/.genie/wishes/omni-approval-ux/WISH.md @@ -146,3 +146,7 @@ bun dist/genie.js omni test-approval 2>&1 | grep -qiE 'approved|allow|round-trip - **Builds on** the live-validated `omni-runner-port` bridge + the `omni-hardening` (PR #2507) instance-scope + approval-budget-doc fixes. - **Records** the 2026-07-03 live QA outcome (allow path proven end-to-end; deny code-verified; personal instance offline; reactions unconfirmed) — this wish closes the reaction + correlation + feedback gaps that QA exposed. + +## Known residual (LOW) + +- A row that is still *pending* at the exact instant of the one-time `last_status_glyph` upgrade keeps its bogus self-ref `omni_message_id`; after it resolves, the reconciliation pass makes a few failed reaction attempts to a non-existent stanza id until the 24h recency window ages it out. Self-limiting and harmless — `emitStatusReaction` never throws, records the glyph only on confirmed success, and a bogus id matches no real message (nothing wrong resolves, nothing is spammed). Blast radius: the 0–few approvals outstanding during the ~110s upgrade window. Noted in the `ensureApprovalColumns` code comment. From bb7cd7cc26a7720fb9d9d17f1610a9357f1dca8f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 3 Jul 2026 07:00:38 +0000 Subject: [PATCH 13/18] chore(version): bump to 5.260703.4 [auto-version] --- .claude-plugin/marketplace.json | 2 +- package.json | 2 +- plugins/genie/.claude-plugin/plugin.json | 2 +- plugins/genie/package.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 48d0f1081..4eb352bf3 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "genie", - "version": "5.260703.3", + "version": "5.260703.4", "source": "./plugins/genie", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, wish them into plans, make with parallel agents, ship as one team. A coding genie that grows with your project." } diff --git a/package.json b/package.json index c6ea8276e..1d5f82bd4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@automagik/genie", - "version": "5.260703.3", + "version": "5.260703.4", "description": "Collaborative terminal toolkit for human + AI workflows. NOTE: npm distribution discontinued 2026-05-09 — install via `curl -fsSL https://raw.githubusercontent.com/automagik-dev/genie/main/install.sh | bash` (cosign + SLSA verified). See https://automagik.dev/genie/release-process", "type": "module", "bin": { diff --git a/plugins/genie/.claude-plugin/plugin.json b/plugins/genie/.claude-plugin/plugin.json index 9b8f25afb..b226a902d 100644 --- a/plugins/genie/.claude-plugin/plugin.json +++ b/plugins/genie/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "genie", - "version": "5.260703.3", + "version": "5.260703.4", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, turn them into wishes, execute with /work, validate with /review, and ship as one team.", "author": { "name": "Namastex Labs" diff --git a/plugins/genie/package.json b/plugins/genie/package.json index da4b0e217..36cb4ddc0 100644 --- a/plugins/genie/package.json +++ b/plugins/genie/package.json @@ -1,6 +1,6 @@ { "name": "genie-plugin", - "version": "5.260703.3", + "version": "5.260703.4", "private": true, "description": "Runtime dependencies for genie bundled CLIs", "type": "module", From 6592980e1ce454b93deac9164591da60d0cb0305 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 04:08:34 -0300 Subject: [PATCH 14/18] fix(mcp): drop non-object JSON-RPC lines so a bare 'null' can't crash the server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/term-commands/mcp.test.ts | 27 +++++++++++++++++++++++++++ src/term-commands/mcp.ts | 5 +++++ 2 files changed, 32 insertions(+) diff --git a/src/term-commands/mcp.test.ts b/src/term-commands/mcp.test.ts index 972f133ef..aa741e394 100644 --- a/src/term-commands/mcp.test.ts +++ b/src/term-commands/mcp.test.ts @@ -79,6 +79,25 @@ async function driveMcp(cwd: string, requests: Record[]): Promi .map((l) => JSON.parse(l) as RpcResponse); } +/** Like driveMcp but sends raw (already-serialized) lines — for malformed input. */ +async function driveMcpRaw(cwd: string, rawLines: string[]): Promise { + const proc = Bun.spawn(['bun', GENIE, 'mcp'], { + cwd, + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + env: { ...process.env, NO_COLOR: '1', GENIE_TEST_SKIP_PGSERVE: '1' }, + }); + proc.stdin.write(`${rawLines.join('\n')}\n`); + await proc.stdin.end(); + const stdout = await new Response(proc.stdout).text(); + await proc.exited; + return stdout + .split('\n') + .filter((l) => l.trim().length > 0) + .map((l) => JSON.parse(l) as RpcResponse); +} + const INIT = { jsonrpc: '2.0', id: 1, @@ -108,6 +127,14 @@ function seed(cwd: string): { taskId: string } { // ============================================================================ describe('mcp handshake', () => { + test('a bare `null` / primitive line is dropped and does not crash the server', async () => { + // `JSON.parse('null')` is valid JSON but not a JSON-RPC object; without the + // non-object guard, dispatch(null) throws on null.id and the server crashes. + const res = await driveMcpRaw(repo, ['null', '5', 'true', '"str"', JSON.stringify(INIT)]); + // The malformed lines are silently dropped; the server survives + answers initialize. + expect(res.some((r) => r.id === 1 && (r.result as { serverInfo?: unknown })?.serverInfo)).toBe(true); + }); + test('initialize replies with protocolVersion, tools capability, and serverInfo', async () => { const [res] = await driveMcp(repo, [INIT]); expect(res.id).toBe(1); diff --git a/src/term-commands/mcp.ts b/src/term-commands/mcp.ts index 9439b078f..c61eda75e 100644 --- a/src/term-commands/mcp.ts +++ b/src/term-commands/mcp.ts @@ -144,6 +144,11 @@ export async function runMcpServer(): Promise { // Unparseable line → cannot attribute an id; drop it (no id to reply to). return; } + // A JSON-RPC request must be a non-null object. `JSON.parse('null')` and bare + // primitives are valid JSON but carry no id to attribute — drop them like an + // unparseable line. Without this, `dispatch(null)` (and the catch below) throw + // on `null.id`, and the uncaught error crashes the server on a `null` line. + if (req === null || typeof req !== 'object') return; try { dispatch(req); } catch (e) { From c60e1e39b6ee02a71dba06d5025e72731ed3180d Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 04:18:57 -0300 Subject: [PATCH 15/18] fix(mcp): db-backed branch resolution + reopen db created mid-session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 + '-'; 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. --- src/lib/v5/mcp-tools.ts | 34 ++++++++++++++----- src/lib/v5/task-state.ts | 17 ++++++++++ src/term-commands/mcp.test.ts | 64 +++++++++++++++++++++++++++++++++++ src/term-commands/mcp.ts | 4 +++ 4 files changed, 110 insertions(+), 9 deletions(-) diff --git a/src/lib/v5/mcp-tools.ts b/src/lib/v5/mcp-tools.ts index d74342189..9265576d3 100644 --- a/src/lib/v5/mcp-tools.ts +++ b/src/lib/v5/mcp-tools.ts @@ -24,6 +24,7 @@ import { getTask, getWishGroups, listTasks, + listWishSlugs, } from './task-state.js'; // ============================================================================ @@ -97,17 +98,30 @@ function currentBranch(cwd: string): string | null { } /** - * Parse a `wish/-` branch into `{ wish, group }`. The group is the - * final `-`-delimited segment; the slug is everything before it (slugs may - * themselves contain hyphens, e.g. `wish/genie-mcp-g2` → `genie-mcp` / `g2`). - * Returns `null` when the branch is not a wish worktree branch. + * Resolve a `wish/[-]` branch into `{ wish, group }`. Both slug and + * group may contain hyphens, so a raw last-dash split is ambiguous + * (`wish/genie-mcp` is the `genie-mcp` wish with no group, NOT a `genie` wish + * with an `mcp` group). Disambiguate against the KNOWN wish slugs in the db: + * 1. exact match → top-level branch, group = null; + * 2. longest known slug that is a prefix followed by `-`; + * 3. no known wish (e.g. a brand-new branch with no tasks yet) → best-effort + * last-dash split, else the whole rest as the wish. + * Returns `null` only when the branch is not a `wish/…` branch. */ -function parseWishBranch(branch: string): { wish: string; group: string } | null { +function resolveWishBranch(db: Database | null, branch: string): { wish: string; group: string | null } | null { const rest = branch.startsWith('wish/') ? branch.slice('wish/'.length) : null; if (!rest) return null; + const known = db ? listWishSlugs(db) : []; // longest-first + if (known.includes(rest)) return { wish: rest, group: null }; + for (const slug of known) { + if (rest.startsWith(`${slug}-`)) { + const group = rest.slice(slug.length + 1); + if (group) return { wish: slug, group }; + } + } const dash = rest.lastIndexOf('-'); - if (dash <= 0 || dash === rest.length - 1) return null; - return { wish: rest.slice(0, dash), group: rest.slice(dash + 1) }; + if (dash > 0 && dash < rest.length - 1) return { wish: rest.slice(0, dash), group: rest.slice(dash + 1) }; + return { wish: rest, group: null }; } // ============================================================================ @@ -190,10 +204,12 @@ interface WorktreeContextPayload { function genieWorktreeContext(ctx: ToolContext, args: Record): WorktreeContextPayload { const branch = argString(args, 'branch') ?? currentBranch(ctx.cwd); - const parsed = branch ? parseWishBranch(branch) : null; + const parsed = branch ? resolveWishBranch(ctx.db, branch) : null; if (parsed) { - const tasks = ctx.db ? listTasks(ctx.db, { wish: parsed.wish }).filter((t) => t.group === parsed.group) : []; + const wishTasks = ctx.db ? listTasks(ctx.db, { wish: parsed.wish }) : []; + // Top-level wish branch (no group) → all of the wish's tasks; a group branch → just that group. + const tasks = parsed.group === null ? wishTasks : wishTasks.filter((t) => t.group === parsed.group); return { branch, resolved: true, wish: parsed.wish, group: parsed.group, tasks: tasks.map(toSummary) }; } diff --git a/src/lib/v5/task-state.ts b/src/lib/v5/task-state.ts index f1782277d..96c736468 100644 --- a/src/lib/v5/task-state.ts +++ b/src/lib/v5/task-state.ts @@ -611,6 +611,23 @@ export function getWishGroups(db: Database, wish: string): WishGroupRow[] { return rows.map(mapWishGroup); } +/** + * Distinct known wish slugs (from tasks + wish_groups), longest first. Used to + * disambiguate a `wish/-` branch when the slug itself contains + * hyphens (`genie-mcp` vs a `genie` wish with an `mcp` group). + */ +export function listWishSlugs(db: Database): string[] { + const rows = db + .query( + `SELECT wish FROM ( + SELECT wish FROM tasks WHERE wish IS NOT NULL + UNION SELECT wish FROM wish_groups WHERE wish IS NOT NULL + ) GROUP BY wish ORDER BY LENGTH(wish) DESC`, + ) + .all() as Array<{ wish: string }>; + return rows.map((r) => r.wish); +} + function getWishGroup(db: Database, wish: string, name: string): WishGroupRow | null { const row = db.query('SELECT * FROM wish_groups WHERE wish = ? AND name = ?').get(wish, name) as | Parameters[0] diff --git a/src/term-commands/mcp.test.ts b/src/term-commands/mcp.test.ts index aa741e394..29495f585 100644 --- a/src/term-commands/mcp.test.ts +++ b/src/term-commands/mcp.test.ts @@ -331,6 +331,70 @@ describe('mcp absent-db degrade', () => { expect(payload.counts.total).toBe(0); expect(payload.tasks).toEqual([]); }); + + test('reopens a db created AFTER the server started (no stale empty board)', async () => { + // Server starts against a repo with no genie.db (null handle), THEN the db + // is created mid-session — a per-call reopen must pick it up, not serve empty. + const proc = Bun.spawn(['bun', GENIE, 'mcp'], { + cwd: repo, + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + env: { ...process.env, NO_COLOR: '1', GENIE_TEST_SKIP_PGSERVE: '1' }, + }); + proc.stdin.write(`${JSON.stringify(INIT)}\n`); + seed(repo); // create .genie/genie.db + tasks AFTER the server opened (null) at startup + proc.stdin.write( + `${JSON.stringify({ jsonrpc: '2.0', id: 20, method: 'tools/call', params: { name: 'genie_board', arguments: {} } })}\n`, + ); + await proc.stdin.end(); + const stdout = await new Response(proc.stdout).text(); + await proc.exited; + const responses = stdout + .split('\n') + .filter((l) => l.trim().length > 0) + .map((l) => JSON.parse(l) as RpcResponse); + const res = responses.find((r) => r.id === 20); + const payload = toolPayload<{ counts: { total: number } }>(res!); + expect(payload.counts.total).toBeGreaterThan(0); // saw the db created mid-session + }); +}); + +describe('mcp worktree branch resolution', () => { + test('disambiguates a hyphenated wish slug against known wishes', async () => { + seed(repo); // wish 'genie-mcp' (slug has a hyphen), groups g1/g2, a task in g2 + // Top-level `wish/genie-mcp` must resolve to the genie-mcp wish (group null), + // NOT a mis-split `genie` wish with an `mcp` group. + const top = await driveMcp(repo, [ + INIT, + INITIALIZED, + { + jsonrpc: '2.0', + id: 30, + method: 'tools/call', + params: { name: 'genie_worktree_context', arguments: { branch: 'wish/genie-mcp' } }, + }, + ]); + const topP = toolPayload<{ resolved: boolean; wish: string; group: string | null }>(top.find((r) => r.id === 30)!); + expect(topP.resolved).toBe(true); + expect(topP.wish).toBe('genie-mcp'); + expect(topP.group).toBeNull(); + + // A group branch `wish/genie-mcp-g2` → genie-mcp / g2 (last-dash is correct here). + const grp = await driveMcp(repo, [ + INIT, + INITIALIZED, + { + jsonrpc: '2.0', + id: 31, + method: 'tools/call', + params: { name: 'genie_worktree_context', arguments: { branch: 'wish/genie-mcp-g2' } }, + }, + ]); + const grpP = toolPayload<{ wish: string; group: string | null }>(grp.find((r) => r.id === 31)!); + expect(grpP.wish).toBe('genie-mcp'); + expect(grpP.group).toBe('g2'); + }); }); // ============================================================================ diff --git a/src/term-commands/mcp.ts b/src/term-commands/mcp.ts index c61eda75e..92967a615 100644 --- a/src/term-commands/mcp.ts +++ b/src/term-commands/mcp.ts @@ -91,6 +91,10 @@ export async function runMcpServer(): Promise { }); return; } + // The db may have been absent when the server started (null handle → empty + // board). Re-attempt the read-only open per call so a db created mid-session + // (e.g. a fresh `genie init`) is picked up instead of serving empty forever. + if (!ctx.db) ctx.db = openReadonlyDb(cwd); const payload = tool.handler(ctx, args); ok(id, { content: [{ type: 'text', text: JSON.stringify(payload) }], From 9dedcd7915fb55f9d87b2ffc687c37a3a0092688 Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 04:28:48 -0300 Subject: [PATCH 16/18] =?UTF-8?q?fix(mcp):=20address=20#2512=20review=20?= =?UTF-8?q?=E2=80=94=20db-handle=20leak,=20verified=20branch=20resolution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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. --- src/lib/v5/mcp-tools.ts | 22 ++++++++++++---- src/lib/v5/task-state.ts | 3 ++- src/term-commands/mcp.test.ts | 47 ++++++++++++++++++++++++++++++++--- src/term-commands/mcp.ts | 8 +++--- 4 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/lib/v5/mcp-tools.ts b/src/lib/v5/mcp-tools.ts index 9265576d3..e41745a12 100644 --- a/src/lib/v5/mcp-tools.ts +++ b/src/lib/v5/mcp-tools.ts @@ -101,24 +101,36 @@ function currentBranch(cwd: string): string | null { * Resolve a `wish/[-]` branch into `{ wish, group }`. Both slug and * group may contain hyphens, so a raw last-dash split is ambiguous * (`wish/genie-mcp` is the `genie-mcp` wish with no group, NOT a `genie` wish - * with an `mcp` group). Disambiguate against the KNOWN wish slugs in the db: - * 1. exact match → top-level branch, group = null; - * 2. longest known slug that is a prefix followed by `-`; - * 3. no known wish (e.g. a brand-new branch with no tasks yet) → best-effort - * last-dash split, else the whole rest as the wish. + * with an `mcp` group). Disambiguate against the db, most-authoritative first: + * 1. a `-` where BOTH the slug is known AND `` is a real + * group of it → a launch worktree (beats a same-named top-level slug); + * 2. exact known slug → top-level branch, group = null; + * 3. longest known slug that is a prefix + `-` (group unverified); + * 4. no known wish (brand-new branch) → last-dash heuristic, else whole rest. * Returns `null` only when the branch is not a `wish/…` branch. */ function resolveWishBranch(db: Database | null, branch: string): { wish: string; group: string | null } | null { const rest = branch.startsWith('wish/') ? branch.slice('wish/'.length) : null; if (!rest) return null; const known = db ? listWishSlugs(db) : []; // longest-first + // 1. Verified launch worktree: the group actually exists on the prefix wish. + if (db) { + for (const slug of known) { + if (!rest.startsWith(`${slug}-`)) continue; + const group = rest.slice(slug.length + 1); + if (group && getWishGroups(db, slug).some((g) => g.name === group)) return { wish: slug, group }; + } + } + // 2. Exact known slug → top-level branch (no group). if (known.includes(rest)) return { wish: rest, group: null }; + // 3. Longest known slug that is a prefix (group unverified) → best guess. for (const slug of known) { if (rest.startsWith(`${slug}-`)) { const group = rest.slice(slug.length + 1); if (group) return { wish: slug, group }; } } + // 4. No known wish yet → last-dash heuristic, else the whole rest as the wish. const dash = rest.lastIndexOf('-'); if (dash > 0 && dash < rest.length - 1) return { wish: rest.slice(0, dash), group: rest.slice(dash + 1) }; return { wish: rest, group: null }; diff --git a/src/lib/v5/task-state.ts b/src/lib/v5/task-state.ts index 96c736468..180c4c870 100644 --- a/src/lib/v5/task-state.ts +++ b/src/lib/v5/task-state.ts @@ -619,10 +619,11 @@ export function getWishGroups(db: Database, wish: string): WishGroupRow[] { export function listWishSlugs(db: Database): string[] { const rows = db .query( + // UNION already de-duplicates; order longest-first for prefix disambiguation. `SELECT wish FROM ( SELECT wish FROM tasks WHERE wish IS NOT NULL UNION SELECT wish FROM wish_groups WHERE wish IS NOT NULL - ) GROUP BY wish ORDER BY LENGTH(wish) DESC`, + ) ORDER BY LENGTH(wish) DESC`, ) .all() as Array<{ wish: string }>; return rows.map((r) => r.wish); diff --git a/src/term-commands/mcp.test.ts b/src/term-commands/mcp.test.ts index 29495f585..1ac71a689 100644 --- a/src/term-commands/mcp.test.ts +++ b/src/term-commands/mcp.test.ts @@ -342,15 +342,30 @@ describe('mcp absent-db degrade', () => { stderr: 'pipe', env: { ...process.env, NO_COLOR: '1', GENIE_TEST_SKIP_PGSERVE: '1' }, }); + const reader = proc.stdout.getReader(); + const decoder = new TextDecoder(); + let out = ''; + // Synchronize: wait for the initialize reply BEFORE seeding, which proves the + // server's startup read-only open already ran against an absent db (null) — + // otherwise the test could seed first and pass without exercising the reopen. proc.stdin.write(`${JSON.stringify(INIT)}\n`); - seed(repo); // create .genie/genie.db + tasks AFTER the server opened (null) at startup + while (!out.includes('"serverInfo"')) { + const { value, done } = await reader.read(); + if (done) break; + out += decoder.decode(value, { stream: true }); + } + seed(repo); // create .genie/genie.db + tasks AFTER the startup open saw nothing proc.stdin.write( `${JSON.stringify({ jsonrpc: '2.0', id: 20, method: 'tools/call', params: { name: 'genie_board', arguments: {} } })}\n`, ); await proc.stdin.end(); - const stdout = await new Response(proc.stdout).text(); + while (true) { + const { value, done } = await reader.read(); + if (done) break; + out += decoder.decode(value, { stream: true }); + } await proc.exited; - const responses = stdout + const responses = out .split('\n') .filter((l) => l.trim().length > 0) .map((l) => JSON.parse(l) as RpcResponse); @@ -395,6 +410,32 @@ describe('mcp worktree branch resolution', () => { expect(grpP.wish).toBe('genie-mcp'); expect(grpP.group).toBe('g2'); }); + + test('a launch group branch beats a same-named top-level wish slug', async () => { + // Ambiguous collision: a `genie` wish WITH a real `mcp` group, AND a separate + // `genie-mcp` wish. `wish/genie-mcp` must resolve to the verified launch + // worktree (genie / mcp), not the exact-slug top-level `genie-mcp` wish. + const db = openDb({ cwd: repo }); + const board = createBoard(db, 'repo'); + createTask(db, { title: 'a', boardId: board.id, wish: 'genie', group: 'mcp' }); + createWishGroups(db, 'genie', [{ name: 'mcp' }]); + createTask(db, { title: 'b', boardId: board.id, wish: 'genie-mcp', group: 'g1' }); + createWishGroups(db, 'genie-mcp', [{ name: 'g1' }]); + db.close(); + const res = await driveMcp(repo, [ + INIT, + INITIALIZED, + { + jsonrpc: '2.0', + id: 32, + method: 'tools/call', + params: { name: 'genie_worktree_context', arguments: { branch: 'wish/genie-mcp' } }, + }, + ]); + const p = toolPayload<{ wish: string; group: string | null }>(res.find((r) => r.id === 32)!); + expect(p.wish).toBe('genie'); + expect(p.group).toBe('mcp'); + }); }); // ============================================================================ diff --git a/src/term-commands/mcp.ts b/src/term-commands/mcp.ts index 92967a615..f91543083 100644 --- a/src/term-commands/mcp.ts +++ b/src/term-commands/mcp.ts @@ -74,8 +74,10 @@ export async function runMcpServer(): Promise { // Lazy: the read-only bun:sqlite open + tools load here, not at genie startup. const { MCP_TOOLS, openReadonlyDb } = await import('../lib/v5/mcp-tools.js'); const cwd = process.cwd(); - const db = openReadonlyDb(cwd); - const ctx: ToolContext = { db, cwd }; + // Single source of truth for the handle: the per-call reopen below writes back + // to ctx.db, and close() reads ctx.db — so a mid-session reopen is always the + // one that gets closed (no stale/leaked handle). + const ctx: ToolContext = { db: openReadonlyDb(cwd), cwd }; const toolByName = new Map(MCP_TOOLS.map((t) => [t.name, t] as const)); @@ -166,7 +168,7 @@ export async function runMcpServer(): Promise { await new Promise((resolve) => { rl.on('close', () => { - db?.close(); + ctx.db?.close(); // Flush stdout BEFORE exiting: the empty-write callback fires after the // stream's buffer (including all prior response writes) drains to the OS, // so the final response is never truncated. See SPIKE.md flush note. From 3fc8b901e43ccc0d94b7759280746125de3f7950 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 3 Jul 2026 07:30:48 +0000 Subject: [PATCH 17/18] chore(version): bump to 5.260703.5 [auto-version] --- .claude-plugin/marketplace.json | 2 +- package.json | 2 +- plugins/genie/.claude-plugin/plugin.json | 2 +- plugins/genie/package.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 4eb352bf3..c95becfd6 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "genie", - "version": "5.260703.4", + "version": "5.260703.5", "source": "./plugins/genie", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, wish them into plans, make with parallel agents, ship as one team. A coding genie that grows with your project." } diff --git a/package.json b/package.json index 1d5f82bd4..d15850e0c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@automagik/genie", - "version": "5.260703.4", + "version": "5.260703.5", "description": "Collaborative terminal toolkit for human + AI workflows. NOTE: npm distribution discontinued 2026-05-09 — install via `curl -fsSL https://raw.githubusercontent.com/automagik-dev/genie/main/install.sh | bash` (cosign + SLSA verified). See https://automagik.dev/genie/release-process", "type": "module", "bin": { diff --git a/plugins/genie/.claude-plugin/plugin.json b/plugins/genie/.claude-plugin/plugin.json index b226a902d..eb7051080 100644 --- a/plugins/genie/.claude-plugin/plugin.json +++ b/plugins/genie/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "genie", - "version": "5.260703.4", + "version": "5.260703.5", "description": "Human-AI partnership for Claude Code. Share a terminal, orchestrate workers, evolve together. Brainstorm ideas, turn them into wishes, execute with /work, validate with /review, and ship as one team.", "author": { "name": "Namastex Labs" diff --git a/plugins/genie/package.json b/plugins/genie/package.json index 36cb4ddc0..01a4ffe26 100644 --- a/plugins/genie/package.json +++ b/plugins/genie/package.json @@ -1,6 +1,6 @@ { "name": "genie-plugin", - "version": "5.260703.4", + "version": "5.260703.5", "private": true, "description": "Runtime dependencies for genie bundled CLIs", "type": "module", From 633bfa7736f4aa61892a8fd2db8594a0e4018c9a Mon Sep 17 00:00:00 2001 From: Felipe Rosa Date: Fri, 3 Jul 2026 04:33:31 -0300 Subject: [PATCH 18/18] fix(doctor): enforce strictly-below hook-timeout contract at the boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/genie-commands/doctor.test.ts | 13 +++++++++++-- src/genie-commands/doctor.ts | 18 +++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/genie-commands/doctor.test.ts b/src/genie-commands/doctor.test.ts index d02021c63..94dae5a5a 100644 --- a/src/genie-commands/doctor.test.ts +++ b/src/genie-commands/doctor.test.ts @@ -102,19 +102,28 @@ describe('omni hook-timeout guardrail', () => { test('warns when the hook timeout is below pollBudgetMs', () => { const res = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: 110_000, timeoutSec: 5 }); expect(res?.status).toBe('warn'); - expect(res?.detail).toContain('< pollBudget'); + expect(res?.detail).toContain('≤ pollBudget'); expect(res?.suggestion).toContain('120'); }); + test('warns at the exact boundary (timeoutMs === pollBudgetMs — strictly-below contract)', () => { + // 110s → 110_000ms === pollBudget: no margin, so the strict contract must warn. + const res = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: 110_000, timeoutSec: 110 }); + expect(res?.status).toBe('warn'); + // smallest safe whole-second timeout strictly exceeds the budget → 111s. + expect(res?.suggestion).toContain('111'); + }); + test('warns when approvals are enabled but no dispatch timeout is found', () => { const res = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: 110_000, timeoutSec: null }); expect(res?.status).toBe('warn'); expect(res?.detail).toContain('no `genie hook dispatch`'); }); - test('passes when the hook timeout meets or exceeds pollBudgetMs', () => { + test('passes when the hook timeout strictly exceeds pollBudgetMs', () => { const res = evaluateOmniHookTimeout({ enabled: true, pollBudgetMs: 110_000, timeoutSec: 120 }); expect(res?.status).toBe('pass'); + expect(res?.detail).toContain('> pollBudget'); }); test('emits no check when omni approvals are disabled', () => { diff --git a/src/genie-commands/doctor.ts b/src/genie-commands/doctor.ts index d892294d5..fd011b6e8 100644 --- a/src/genie-commands/doctor.ts +++ b/src/genie-commands/doctor.ts @@ -235,8 +235,10 @@ export function evaluateOmniHookTimeout(params: { timeoutSec: number | null; }): CheckResult | null { if (!params.enabled) return null; - const name = 'omni hook timeout ≥ pollBudget'; - const needSec = Math.ceil(params.pollBudgetMs / 1000); + const name = 'omni hook timeout > pollBudget'; + // pollBudgetMs MUST stay STRICTLY below the hook timeout (genie-config.ts), so + // the smallest safe whole-second timeout is the first that exceeds pollBudgetMs. + const needSec = Math.floor(params.pollBudgetMs / 1000) + 1; if (params.timeoutSec === null) { return { name, @@ -246,15 +248,21 @@ export function evaluateOmniHookTimeout(params: { }; } const timeoutMs = params.timeoutSec * 1000; - if (timeoutMs < params.pollBudgetMs) { + // At timeoutMs === pollBudgetMs there is no margin — CC can kill the hook the + // instant the poll budget expires — so the strict contract warns on equal too. + if (timeoutMs <= params.pollBudgetMs) { return { name, status: 'warn', - detail: `hook timeout ${params.timeoutSec}s (${timeoutMs}ms) < pollBudget ${params.pollBudgetMs}ms — CC will kill the hook before it can allow/deny or reach its ask fail-safe`, + detail: `hook timeout ${params.timeoutSec}s (${timeoutMs}ms) ≤ pollBudget ${params.pollBudgetMs}ms — CC may kill the hook before it can allow/deny or reach its ask fail-safe`, suggestion: `Raise the PreToolUse \`genie hook dispatch\` timeout to ≥ ${needSec}s (e.g. 120) in ~/.claude/settings.json.`, }; } - return { name, status: 'pass', detail: `hook timeout ${params.timeoutSec}s ≥ pollBudget ${params.pollBudgetMs}ms` }; + return { + name, + status: 'pass', + detail: `hook timeout ${params.timeoutSec}s (${timeoutMs}ms) > pollBudget ${params.pollBudgetMs}ms`, + }; } async function checkOmniHookTimeout(): Promise {