Skip to content

fix: support group participant changes on macOS 26#178

Open
markxiong0122 wants to merge 1 commit into
openclaw:mainfrom
markxiong0122:fix/macos26-participant-selectors
Open

fix: support group participant changes on macOS 26#178
markxiong0122 wants to merge 1 commit into
openclaw:mainfrom
markxiong0122:fix/macos26-participant-selectors

Conversation

@markxiong0122

Copy link
Copy Markdown

Summary

  • vend add-participant handles through the existing candidate/service-aware resolver instead of bare IMHandleWithID:
  • prefer Tahoe selectors inviteParticipants:reason: and removeParticipants:reason: with the older selectors as fallbacks
  • expose selector availability in status --json

Verification

  • make build-dylib
  • make test (477 tests)
  • live macOS 26 Messages.app gate: add +13129001215 to a disposable 3-member group, verify via a fresh imsg chats, remove it, verify the original participant set is restored

@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. labels Jul 15, 2026
@clawsweeper

clawsweeper Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 4:24 AM ET / 08:24 UTC.

Summary
The PR updates group participant add/remove calls to resolve handles through the existing service-aware path, prefer macOS 26 participant selectors with legacy fallbacks, and report selector availability in status --json.

Reproducibility: no. high-confidence live reproduction was established in this review because the reported behavior needs a macOS 26 Messages.app account and disposable group. The source path is clear, and the PR describes the required add/list/remove round trip, but no inspectable after-fix artifact was supplied.

Review metrics: 1 noteworthy metric.

  • Native runtime surface: 1 file affected; 31 added, 15 removed. The small, focused private-API compatibility change is reviewable, but its behavior requires real macOS validation rather than CI-only confidence.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P1] Attach redacted live macOS 26 evidence showing add, fresh listing, remove, and restored group membership.
  • Include any relevant selector/status output while removing phone numbers, account details, and other private identifiers.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR claims a successful live macOS 26 round trip but provides no inspectable after-fix evidence; attach redacted terminal output, runtime logs, or a recording and update the PR body for re-review (or ask a maintainer to comment @clawsweeper re-review).

Risk before merge

  • [P2] Merging changes private runtime selector routing for existing macOS users; selector-availability checks reduce downgrade risk, but only a redacted live macOS 26 add/list/remove round trip can establish that the Tahoe path and legacy fallback preserve real group membership behavior.

Maintainer options:

  1. Require macOS 26 round-trip proof (recommended)
    Before merge, ask for redacted live output, logs, or a recording showing participant add, fresh listing, remove, and restored membership on macOS 26.
  2. Accept runtime-selector uncertainty
    Merge based on the guarded selector fallback without external proof, accepting that the private macOS 26 runtime path has not been independently demonstrated.

Next step before merge

  • [P1] The remaining blocker is contributor-supplied real behavior proof from macOS 26, not a concrete automated code repair.

Security
Cleared: The one-file native selector and handle-resolution change introduces no concrete supply-chain, secret, permission, or authorization regression in the reviewed diff.

Review details

Best possible solution:

Keep the service-aware resolver and selector-fallback implementation, then attach redacted evidence from a disposable macOS 26 group showing add, fresh chat listing, remove, and restoration of the original participant set before merge.

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

No high-confidence live reproduction was established in this review because the reported behavior needs a macOS 26 Messages.app account and disposable group. The source path is clear, and the PR describes the required add/list/remove round trip, but no inspectable after-fix artifact was supplied.

Is this the best way to solve the issue?

Yes, conditionally: preferring the macOS 26 selectors behind runtime availability checks and retaining legacy fallbacks is the narrowest maintainable repair. It should not merge until real macOS 26 evidence confirms both the selected path and observed membership result.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: The PR changes runtime selector selection for existing macOS group participant operations, so an unproven selector contract could affect established user workflows after upgrade.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: This is a bounded compatibility repair for group-participant changes rather than an emergency availability or data-loss incident.
  • merge-risk: 🚨 compatibility: The PR changes runtime selector selection for existing macOS group participant operations, so an unproven selector contract could affect established user workflows after upgrade.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR claims a successful live macOS 26 round trip but provides no inspectable after-fix evidence; attach redacted terminal output, runtime logs, or a recording and update the PR body for re-review (or ask a maintainer to comment @clawsweeper re-review).
Evidence reviewed

What I checked:

  • Focused patch surface: The branch changes one native injection file, adding 31 lines and removing 15 lines to select inviteParticipants:reason: and removeParticipants:reason: when available while retaining the legacy selectors. (Sources/IMsgHelper/IMsgInjected.m:5786, 71ae5b2a59b2)
  • Current-main gap: The current-main participant path still exposes the legacy inviteParticipantsToiMessageChat:reason: and removeParticipantsFromiMessageChat:reason: behavior, so the macOS 26 compatibility portion is not already implemented on main. (Sources/IMsgHelper/IMsgInjected.m:5786, ca91c41b47e0)
  • Proof remains absent: The PR body describes a live macOS 26 add/list/remove round trip, but the supplied PR discussion has no attached terminal output, runtime logs, recording, screenshot, or linked artifact demonstrating the after-fix result; the existing proof-request label and prior ClawSweeper review remain applicable. (71ae5b2a59b2)
  • Feature-history routing: The proposed compatibility work is concentrated in the longstanding native Messages injection surface; repository history available in the supplied context identifies the current patch author but does not provide sufficient independent merged-history metadata to name a higher-confidence mainline code owner. (Sources/IMsgHelper/IMsgInjected.m:5786, ca91c41b47e0)

Likely related people:

  • steipete: The native Messages helper is part of the repository’s central implementation surface; available review context does not provide enough untruncated file-history attribution to make this routing stronger. (role: likely adjacent code owner; confidence: low; files: Sources/IMsgHelper/IMsgInjected.m)
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 (2 earlier review cycles)
  • reviewed 2026-07-15T06:01:22.666Z sha 71ae5b2 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-15T07:12:22.792Z sha 71ae5b2 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: support group participant changes on macOS 26 This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant