feat(omni): approval UX — correlated identity, reactions, ⏳→✅ acks - #2509
Conversation
Rebased onto current dev (was based on a pre-omni-hardening commit). Wish:
correlated identity, reaction approve/deny, two-state ⏳→✅ acks; sequential
G1->G2->G3. SPIKE.md (0 live msgs, source-proven): reactions arrive on
omni.message.{instance}.{chatId} (omni.event.> has zero publishers), send
returns the stanza id, outbound set-reaction GO, text quoted-id unavailable
(bare text stays oldest fallback).
Group 2 of omni-approval-ux. announce() now sends via an injectable OmniSend seam (default: signed POST to omni /api/v2/messages) and stores the REAL WhatsApp stanza id via attachOmniMessageId — retiring the self-referential genId() ref that matched nothing inbound. Inbound reactions parse off omni.message.* as [Reaction: <emoji> on message <id>], correlate 👍/👎 to the exact approval by omni_message_id (oldest only when no id), and the dual-emit bare-emoji echo drops structurally (reaction-vs-text split). Retired the dead omni.event.> path/handleEvent. Bare text stays oldest fallback; PR#2507 instance-scope guard kept. Reviewed: SHIP. 28 runner/queue tests + typecheck.
Group 3 (final) of omni-approval-ux. genie sets ⏳ on the approval message the moment it's sent (the G2 stanza id), swapping in place to ✅ (approved) / ❌ (denied/expired) — via an injectable OmniSetReaction seam (default: signed omni --reaction POST; fallback message-edit/status-reply is a one-seam swap). A tick reconciliation pass (reconcileStatusAcks) makes the runner the authoritative acker regardless of which process expired the row, curing a stuck-⏳ hook-fork race AND transport-dropped swaps. Reactions targeting an unknown id no-op (not resolveOldest). Adds genie omni test-approval (fake default, --live) + a doctor hook-timeout guardrail. New last_status_glyph column (additive, no user_version bump; one-time migration backfill + 24h recency cap so upgrade never sweeps history). Reviewed: SHIP (2 HIGHs found+fixed). 77 omni tests.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the Omni Approval UX wish, introducing correlated approval identity, reaction-based approvals, and a two-state status-reaction lifecycle (⏳→✅/❌). Key changes include capturing and storing the real Omni message ID (stanza ID) on send, correlating inbound reactions and quoted replies to specific approvals, and updating the approval message's status reaction. Additionally, a genie doctor guardrail check was added to warn if the Claude Code hook timeout is below the poll budget, and a genie omni test-approval command was introduced to drive a single round-trip (fake or live). Database schema updates add a last_status_glyph column to the approvals table with a one-time backfill sentinel for historical rows to prevent redundant status updates. There are no review comments to assess, so no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 483df2af34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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; |
There was a problem hiding this comment.
When the live runner uses this default send path, the repo's own spike notes that the persisted POST /messages REST route returns messageId: message.id (the Omni UUID), while inbound WhatsApp reactions target the stanza/external id (.genie/wishes/omni-approval-ux/SPIKE.md lines 65-68). Storing this response field therefore makes resolveReaction() compare an external id from [Reaction: … on message <id>] against a UUID and miss; because explicit reaction targets intentionally do not fall back to oldest, 👍/👎 reactions on real approvals will be ignored and the approval will time out instead of resolving.
Useful? React with 👍 / 👎.
1 participant
Summary
The v5 omni WhatsApp approval bridge is live-proven, but its live QA exposed three UX/correctness gaps. This wish (
omni-approval-ux) closes them: approvals are now correlated (a reaction resolves the exact one it targets, via the real WhatsApp stanza id), reaction-driven (👍/👎), and self-updating via a two-state ⏳→✅ acknowledgement — genie sets ⏳ on its approval message the moment it's sent, swapping in place to ✅/❌ once you answer.Spike-first (0 live messages)
G1 grounded four unknowns against the running Omni build with zero live traffic (all source-proven): reactions arrive on
omni.message.{instance}.{chatId}(the assumedomni.event.>has zero publishers — the QA disproof); the send returns the stanza id; outbound set-reaction is GO (with a documented fallback); text replies carry no quoted id (so bare text stays an honest oldest-fallback).What shipped
announce()now sends via an injectable seam and stores the real stanza id (retiring a self-referentialgenId()ref that matched nothing); reactions parse offomni.message.*, correlate 👍/👎 to the exact approval, dedupe the dual-emit echo; deadomni.event.>retired.genie omni test-approval(fake default,--live) and agenie doctorhook-timeout guardrail. Newlast_status_glyphcolumn (additive, nouser_versionbump — a bump would brick existing DBs; one-time migration backfill + 24h recency cap so upgrade never sweeps history).Review rigor
Every group was independently reviewed; G3's reviews found and closed two real HIGHs (a stuck ⏳ on the timeout path, and a reconcile sweep of historical rows on upgrade) before ship — both would have silently violated the wish's own promises (live-state accuracy, anti-spam). One LOW residual noted (a pending-with-bogus-id row caught at the exact upgrade instant — 0–few rows, self-limiting, no real traffic).
Anti-spam guarantee
The automated suite emits zero live WhatsApp traffic (injectable fakes;
--liveis a documented manual escape hatch never called by tests). The G1 spike sent 0 messages.Needs your eyes — one live confirmation
Everything is source-proven, but the ⏳→✅ in-place render (does WhatsApp swap the emoji visually?) is the sole empirical unknown. I'll run one labelled round-trip between your own numbers to confirm — the fallback (message-edit / status-reply) covers it if the swap doesn't render.
Gates
Full
bun run check628 pass / 0; build green;omni test-approvalprints the ⏳→✅ round-trip; complexity budget + biome clean.Merge decision: Felipe.