Skip to content

feat: add poll unvote updates#162

Open
omarshahine wants to merge 11 commits into
openclaw:mainfrom
omarshahine:feat/poll-unvote-add-option
Open

feat: add poll unvote updates#162
omarshahine wants to merge 11 commits into
openclaw:mainfrom
omarshahine:feat/poll-unvote-add-option

Conversation

@omarshahine

@omarshahine omarshahine commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add bridge action for native poll unvote updates
  • expose imsg poll unvote
  • expose JSON-RPC poll.unvote, polls.unvote, and messages.poll.unvote
  • unvote now reads the sender's latest native full-selection snapshot, removes only the requested option, and sends the remaining selected option identifiers
  • fix native empty-vote readback so unvote rows decode as poll vote/update evidence
  • add status selector reporting, docs, and command/RPC coverage
  • remove the unproven add-option surface from this PR; it needs a separate follow-up after visual append behavior is proven

Redacted exact-head proof

Proof collected on lobster against exact head 1d38dc1 in the real Lobster Test Group iMessage group with the branch-built universal CLI/helper. Raw logs are local under /Users/lobster/imsg-pr-proofs/2026-07-10/162-selective-unvote-* because they include chat GUIDs, message GUIDs, handles, and participant identifiers.

Setup note: Messages.app has no public imsg command to create a multi-select poll snapshot. To create the required precondition, I used the branch helper's raw v2 send-poll-unvote bridge action once with remainingOptionIdentifiers=[Alpha,Beta]. The behavior under review was then exercised through the public imsg poll unvote command.

Fresh poll created on exact head:

{
  "messageGuid": "CF492830-8495-4620-9F0A-A3DDCF3F5880",
  "poll": {
    "question": "PR162 A+B unvote proof 20260710-115344",
    "options": [
      { "id": "1D130315-134B-4C52-8487-FB48A5F08E85", "text": "Alpha" },
      { "id": "7E83A9E9-A665-4793-A734-F5F4BD2A9897", "text": "Beta" }
    ]
  }
}

Before state, read back from imsg history --chat-id 22 --json, showed both selections in the native full snapshot:

{
  "guid": "41FA9252-E3E9-4563-8B7F-DDDEB984DCC7",
  "poll_guid": "CF492830-8495-4620-9F0A-A3DDCF3F5880",
  "poll": {
    "votes": [
      { "option_id": "1D130315-134B-4C52-8487-FB48A5F08E85", "option_text": "Alpha", "event_type": "selected" },
      { "option_id": "7E83A9E9-A665-4793-A734-F5F4BD2A9897", "option_text": "Beta", "event_type": "selected" }
    ]
  }
}

Public command under test:

./bin/imsg poll unvote \
  --chat 'any;+;<redacted-group-id>' \
  --poll CF492830-8495-4620-9F0A-A3DDCF3F5880 \
  --option-id 1D130315-134B-4C52-8487-FB48A5F08E85 \
  --json

Command output showed only Beta carried forward:

{
  "removed": true,
  "optionIdentifier": "1D130315-134B-4C52-8487-FB48A5F08E85",
  "messageGuid": "D55975CD-5FF6-4976-92C6-6D812CE5ABFA",
  "remainingOptionIdentifiers": ["7E83A9E9-A665-4793-A734-F5F4BD2A9897"]
}

After state, read back from imsg history --chat-id 22 --json, showed Beta remained and Alpha was absent:

{
  "guid": "D55975CD-5FF6-4976-92C6-6D812CE5ABFA",
  "poll_guid": "CF492830-8495-4620-9F0A-A3DDCF3F5880",
  "poll": {
    "votes": [
      { "option_id": "7E83A9E9-A665-4793-A734-F5F4BD2A9897", "option_text": "Beta", "event_type": "selected" }
    ]
  }
}

SQLite proof for the public unvote row:

{
  "ROWID": 6897,
  "guid": "D55975CD-5FF6-4976-92C6-6D812CE5ABFA",
  "associated_message_guid": "CF492830-8495-4620-9F0A-A3DDCF3F5880",
  "associated_message_type": 4000,
  "balloon_bundle_id": "com.apple.messages.MSMessageExtensionBalloonPlugin:0000000000:com.apple.messages.Polls",
  "payload_bytes": 670,
  "summary_bytes": 201,
  "chat_guid": "any;+;<redacted-group-id>"
}

Verification

  • make build passed on 1d38dc1
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make lint passed; existing warnings only, 0 serious
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make test passed: 409 tests
  • git diff --check passed

@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 10, 2026, 3:11 PM ET / 19:11 UTC.

Summary
The PR adds selective native poll unvote support across the CLI, three JSON-RPC aliases, the injected bridge, poll-history decoding, documentation, skill guidance, and regression tests.

Reproducibility: not applicable. as a current-main bug because this PR introduces a new capability; the exact-head live A+B→B run and deterministic fixtures provide a high-confidence feature verification path.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 16 files, +361/-36. The feature spans public CLI and RPC contracts, database reconstruction, native delivery, decoding, docs, skill guidance, and tests.
  • Public API additions: 1 CLI action, 3 RPC aliases. The patch establishes a durable user-facing poll mutation contract that maintainers should recognize before merge.
  • Behavior validation: A+B → B; 409 tests reported; 2 CI jobs passed. The central selective-unvote invariant has both real Messages evidence and automated coverage.

Root-cause cluster
Relationship: canonical
Canonical: #162
Summary: This PR remains the canonical selective-unvote implementation after its shared native vote-metadata prerequisite was extracted and merged separately.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The feature uses private Messages framework behavior, so the exact-head proof cannot guarantee identical native behavior on every supported macOS release.
  • [P1] Native poll mutations replace a full selection snapshot; future regressions in latest-row selection or payload serialization could preserve or clear the wrong choices.

Maintainer options:

  1. Merge the proven selective scope (recommended)
    Accept the private-framework risk after normal maintainer review while retaining the A+B→B regression and exact-head live proof as merge safeguards.
  2. Request broader macOS proof
    Pause only if maintainers require live compatibility evidence on additional supported macOS versions before accepting the native delivery path.

Next step before merge

  • [P2] No concrete automated repair remains; this proven implementation PR should proceed through ordinary maintainer merge review.

Security
Cleared: The diff adds no dependency, workflow, permission, secret, downloaded-code, publishing, generated-vendor, or supply-chain surface.

Review details

Best possible solution:

Merge the focused selective-unvote implementation with its exact-head A+B→B proof and full-snapshot regression coverage intact, while continuing to exclude add-option and other unproven poll mutation surfaces.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a current-main bug because this PR introduces a new capability; the exact-head live A+B→B run and deterministic fixtures provide a high-confidence feature verification path.

Is this the best way to solve the issue?

Yes. Native poll mutations are full-selection snapshots, so reading the sender's latest snapshot, removing one validated option, and resending the remainder through a shared CLI and RPC path is the narrowest maintainable solution.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 39c205860904.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted exact-head live CLI output, before-and-after native history readback, and SQLite evidence showing Alpha removed while Beta remains.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides redacted exact-head live CLI output, before-and-after native history readback, and SQLite evidence showing Alpha removed while Beta remains.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a bounded user-facing messaging feature with strong validation and limited blast radius.
  • merge-risk: 🚨 message-delivery: The new native full-selection mutation path could deliver an incorrect poll state if snapshot reconstruction or payload serialization regresses.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides redacted exact-head live CLI output, before-and-after native history readback, and SQLite evidence showing Alpha removed while Beta remains.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted exact-head live CLI output, before-and-after native history readback, and SQLite evidence showing Alpha removed while Beta remains.
Evidence reviewed

What I checked:

  • Current main lacks unvote: Current main supports poll creation and voting but has no poll-unvote CLI action, RPC method, or bridge action, so the proposed capability is not already implemented. (Sources/imsg/RPCServer.swift:42, 39c205860904)
  • Selective snapshot reconstruction: The PR reads the latest outbound full-selection snapshot, validates that the requested option is selected, removes only that option, and passes the remaining identifiers to the bridge. (Sources/IMsgCore/MessageStore+Polls.swift:65, 1d38dc1cc61a)
  • Shared CLI and RPC semantics: Both public entry points resolve options through local history and use the same selective mutation contract, including rejection when the option is not currently selected. (Sources/imsg/RPCServer+BridgeMessageHandlers.swift:151, 1d38dc1cc61a)
  • Native bridge payload: The helper serializes the remaining identifiers as the native full-selection votes array and dispatches a distinct send-poll-unvote action without the removed or add-option fields deliberately excluded from scope. (Sources/IMsgHelper/IMsgInjected.m:2797, 1d38dc1cc61a)
  • Regression coverage: Tests cover A+B selected followed by unvote A producing remaining B, rejection of an unselected option, and decoding empty native votes arrays as poll-mutation evidence. (Tests/imsgTests/RPCBridgeMessageHandlersTests.swift:31, 1d38dc1cc61a)
  • Exact-head live proof: The PR body records a real Messages run on exact head 1d38dc1: Alpha and Beta were selected before the public command, unvoting Alpha returned only Beta as remaining, history showed Beta only afterward, and chat.db contained the native associated row. (1d38dc1cc61a)

Likely related people:

  • steipete: Merged the immediate native vote-envelope prerequisite, identified the full-selection semantic blocker, and has the strongest recent current-main history on the native helper path. (role: recent area contributor and reviewer; confidence: high; commits: 2ea890ff8172, dea78a9e9c49; files: Sources/IMsgHelper/IMsgInjected.m, Sources/IMsgCore/MessageStore+Polls.swift)
  • omarshahine: Introduced native poll voting in merged commit 617d941 and has repeated history across the poll CLI, RPC, database, and helper surfaces beyond this proposal. (role: feature owner and prior merged contributor; confidence: high; commits: 617d941f7b1e; files: Sources/IMsgHelper/IMsgInjected.m, Sources/IMsgCore/MessageStore+Polls.swift, Sources/imsg/Commands/PollCommand.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (15 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-09T00:50:48.207Z sha 3af173b :: needs changes before merge. :: [P3] Remove the changelog entry | [P3] Update the payload-title docs
  • reviewed 2026-07-09T01:17:25.729Z sha 9bf040d :: needs real behavior proof before merge. :: [P3] Remove the changelog entry
  • reviewed 2026-07-09T03:52:10.628Z sha b693285 :: needs changes before merge. :: [P2] Treat empty-votes rows as poll evidence | [P3] Update the question option help
  • reviewed 2026-07-09T04:08:39.089Z sha ff5860c :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T04:13:07.442Z sha ff5860c :: needs maintainer review before merge. :: none
  • reviewed 2026-07-10T09:10:28.851Z sha 34f0b45 :: needs real behavior proof before merge. :: [P1] Preserve other selected poll options on unvote | [P3] Remove the PR-owned changelog entry
  • reviewed 2026-07-10T15:28:43.502Z sha 4d85dc2 :: needs real behavior proof before merge. :: [P3] Remove the PR-owned changelog entry
  • reviewed 2026-07-10T19:01:31.530Z sha 1d38dc1 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 8, 2026
@omarshahine

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 9, 2026
@omarshahine

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@omarshahine

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 9, 2026
@omarshahine omarshahine changed the title feat: add poll unvote and option updates feat: add poll unvote updates Jul 9, 2026
@omarshahine

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 9, 2026
@omarshahine

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@omarshahine omarshahine force-pushed the feat/poll-unvote-add-option branch from ff5860c to 34f0b45 Compare July 10, 2026 01:07
@omarshahine

Copy link
Copy Markdown
Contributor Author

@steipete rebased #162 onto current openclaw/main as requested.

What changed:

  • the fix: match native poll vote metadata #169 native vote metadata slice is now in the base instead of carried as review noise in this PR
  • removed the unproven add-option surface from CLI/RPC/docs/skill/tests
  • removed the unrelated poll creation/title rendering change so this diff stays focused on unvote
  • kept the remaining PR surface to imsg poll unvote plus poll.unvote / polls.unvote / messages.poll.unvote
  • unvote now uses the native empty votes array shape while preserving the fix: match native poll vote metadata #169 participant-handle/envelope behavior for normal votes

Verification:

  • make build passed locally
  • make test was attempted locally but this host has only Command Line Tools active and fails before tests run with no such module Testing; GitHub CI is running on the pushed head 34f0b453cf6ffa8cd6c2e0daefd574c9d803bb0d.

@omarshahine

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@steipete

Copy link
Copy Markdown
Collaborator

Thanks for narrowing this after #169. One semantic blocker remains: the requested option is validated but discarded for unvote; every option produces the identical item.votes = [] payload. That cannot implement “remove this option” when the sender has multiple selections—it clears the whole selection snapshot.

Please reconstruct the active sender’s selected options, remove only the requested one, and send the remaining votes, or explicitly narrow the API to “clear all votes.” Required regression: A+B selected, unvote A, B remains; then exact-head sender/recipient proof.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 10, 2026
@omarshahine

Copy link
Copy Markdown
Contributor Author

Addressed the selective unvote blocker in 4d85dc2.

What changed:

  • polls.unvote / poll unvote now reads the sender's latest outbound poll vote snapshot from chat.db.
  • Unvoting option A validates that A is currently selected, removes only A, and passes remainingOptionIdentifiers to the bridge.
  • The injected helper now serializes those remaining option identifiers back into the native item.votes array instead of always sending votes = [].
  • Added regression coverage for A+B selected, unvote A, B remains, plus rejection for unvoting an unselected option.

Local verification:

  • make build passed.
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make test passed: 409 tests.
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make lint exited 0. It still reports existing warnings only.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@omarshahine

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants