Skip to content

feat: add Messages Name & Photo sharing#173

Merged
steipete merged 5 commits into
mainfrom
agent/rebuild-name-photo-sharing
Jul 11, 2026
Merged

feat: add Messages Name & Photo sharing#173
steipete merged 5 commits into
mainfrom
agent/rebuild-name-photo-sharing

Conversation

@steipete

@steipete steipete commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • supersedes feat: add guarded contact-card probes #166 and builds on @omarshahine's implementation, fail-closed runtime proof, and bridge-liveness finding
  • fixes nickname lookup to use the current IMNicknameController singleton and an IMHandle
  • adds canonical imsg name-photo status|share commands plus BlueBubbles-compatible bridge actions and Photon-compatible RPC method names
  • accepts only iMessage/iMessageLite chats, requires participants and a chat-bound local sender identity, and supports only the explicit four-argument force-send selector
  • reports request submission rather than claiming delivery, and documents the explicit-consent requirement

The watcher change from #166 remains intentionally excluded. The two policy-oriented fallback selectors are also excluded because receiver-visible send behavior is unproven; unsupported runtimes fail closed.

Validation

Exact head: 7bf716fb756c38948f751188baf03269a013199d

  • make test — 477 tests passed
  • make lint — passed; existing non-serious warnings only
  • make build — universal CLI (arm64/x86_64) and helper (arm64e/arm64/x86_64) built and signed
  • make build-dylib — passed with one existing deprecation warning
  • focused Name & Photo suite — 12 tests passed after the iMessage service guard
  • live regression fix autoreview — clean, no actionable findings
  • exact-head GitHub CI — macOS and Linux passed in run 29166535110

Live proof

Validated in a macOS 26.5 Parallels VM with SIP disabled and the exact helper injected. Bridge v2 reported both Name & Photo selectors, read-only status returned in 0.21 seconds, an SMS chat failed closed, and an explicitly approved direct iMessage chat invoked the four-argument force-send selector in 0.20 seconds while Messages remained responsive.

Live validation also found and fixed a false-success edge: when no usable personal profile exists, the helper now waits for controller initialization, reports personal_nickname_loaded and has_personal_nickname, and refuses the share rather than returning requested: true. Messages setup produced the profile locally, but Apple's CloudKit upload failed in the VM; after that failure the exact-head build correctly reported available: false and rejected the share. Receiver delivery therefore remains unclaimed, matching the command contract.

Credit

This replacement preserves Omar's authorship in the commit trailer and thanks @omarshahine in the changelog.

Co-authored-by: Omar Shahine <10343873+omarshahine@users.noreply.github.com>
@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: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 11, 2026
@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed July 11, 2026, 3:06 PM ET / 19:06 UTC.

Summary
Adds Messages Name & Photo inspection and explicit sharing through the CLI, bridge actions, compatibility RPC methods, documentation, and tests.

Reproducibility: not applicable. This PR adds a new optional private-API capability rather than fixing a defined current-main regression.

Review metrics: 4 noteworthy metrics.

  • Patch surface: 14 files, +756/-18. The change spans public interfaces, injected Objective-C runtime code, documentation, and tests.
  • New contracts: 1 CLI command, 2 RPC methods, 2 bridge actions. These identifiers and output semantics require intentional compatibility ownership.
  • Regression coverage: 364 new test lines across 2 files. The branch has substantial deterministic coverage, but it cannot establish Messages.app liveness or receiver-visible delivery behavior.
  • Exact-head CI: 2 jobs passed. Successful macOS and Linux checks support build and test quality but do not exercise the injected profile-sharing mutation.

Root-cause cluster
Relationship: canonical
Canonical: #173
Summary: This PR is the active canonical replacement for the closed unmerged contact-card proposal.

Members:

  • superseded: feat: add guarded contact-card probes #166 - The predecessor supplied the initial implementation and live hang evidence; this PR replaces its controller and selector path, tightens service and sender guards, and intentionally omits its watcher change.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
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:

  • Remove the CHANGELOG.md edit.
  • [P1] Add redacted exact-head liveness and receiver-visible proof from an explicitly consented iMessage chat.
  • Obtain explicit bridge-owner approval for the permanent CLI and RPC surface.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The exact head has build, test, and CI evidence but no live share; provide redacted runtime liveness plus sender and receiver evidence from an explicitly consented chat, then update the PR body to trigger a fresh review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] The exact private selector remains unproven in a live receiver-visible run and could hang or behave differently across supported macOS versions.
  • [P1] The new CLI, bridge-action, and RPC identifiers would become compatibility commitments for users and external integrations.
  • [P1] Incorrect chat, participant, or sender resolution could disclose the local Messages profile to unintended recipients.

Maintainer options:

  1. Complete proof and owner approval (recommended)
    Remove the changelog edit, collect redacted consented live liveness and sender/receiver proof, then explicitly approve the public API contract.
  2. Narrow the compatibility surface
    Drop the public CLI and RPC names while retaining only internal implementation that maintainers are ready to validate and support.
  3. Pause unsupported private API
    Keep the PR draft or close it if the selector cannot be safely proven on the intended macOS support boundary.

Next step before merge

  • [P2] The contributor must remove the changelog edit and supply real-environment proof; the remaining public API and support-boundary decision belongs to the shared bridge owners.

Maintainer decision needed

  • Question: After exact-head live proof, should imsg adopt name-photo and contacts.shouldShareContact/contacts.shareContactCard as supported public contracts?
  • Rationale: The change commits the project to undocumented, privacy-sensitive private-API behavior and external compatibility vocabulary; tests and source inspection cannot determine that product intent.
  • Likely owner: steipete — The collaborator author owns the active replacement and current bridge baseline, with Omar Shahine providing complementary bridge and runtime context.
  • Options:
    • Adopt proven guarded APIs (recommended): Approve the public surface after live evidence confirms liveness, iMessage-only targeting, participant scope, sender identity, and receiver-visible behavior.
    • Keep capability internal: Retain only validated bridge internals and defer the public CLI and compatibility RPC commitments.
    • Decline private sharing: Do not support the mutation if maintainers do not want to own its runtime, privacy, and compatibility contract.

Security
Needs attention: The source guards are conservative, but the live profile-disclosure boundary and Messages.app liveness remain unverified.

Review findings

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:16
Review details

Best possible solution:

Keep the branch draft until it removes the changelog edit, demonstrates consented exact-head liveness and receiver-visible targeting, and receives explicit shared-owner approval for the supported public API and macOS boundary.

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

Not applicable: this PR adds a new optional private-API capability rather than fixing a defined current-main regression.

Is this the best way to solve the issue?

Unclear: the narrowed fail-closed design is appropriately conservative, but its exact runtime behavior and permanent compatibility contract remain unproven and unapproved.

Full review comments:

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:16
    CHANGELOG.md is release-owned; keep this release-note context in the PR body or commit message instead. This finding from every prior review cycle remains present on the current head.
    Confidence: 1

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: This optional bridge feature has bounded default exposure but meaningful privacy, compatibility, and runtime validation needs.
  • merge-risk: 🚨 compatibility: The patch establishes lasting CLI, bridge-action, and BlueBubbles/Photon-compatible RPC contracts.
  • merge-risk: 🚨 security-boundary: The operation sends the user’s personal Messages name and photo to every participant in the selected chat.
  • merge-risk: 🚨 availability: The predecessor demonstrated a private controller path blocking Messages.app, while this replacement selector lacks exact-head live liveness proof.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The exact head has build, test, and CI evidence but no live share; provide redacted runtime liveness plus sender and receiver evidence from an explicitly consented chat, then update the PR body to trigger a fresh review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

Security concerns:

  • [medium] Verify the private mutation and audience — Sources/IMsgHelper/IMsgInjected.m:6294
    Use an explicitly consented iMessage test chat to prove the selected chat, participant set, sender identity, force-send selector, unsupported-service failure, and receiver-visible result.
    Confidence: 0.99

What I checked:

  • Current main lacks the feature: Current main contains none of the proposed Name & Photo CLI, bridge-action, private-selector, or compatibility RPC identifiers, so the central capability is not already implemented. (10c91b10924e)
  • Latest release lacks the feature: The same identifiers are absent from release v0.12.3, confirming that no shipped implementation supersedes this PR. (dea78a9e9c49)
  • Guarded mutation implementation: The exact head restricts mutation to the explicit four-argument force-send selector after checking an iMessage service, non-empty participants, and a chat-bound sender identity. (Sources/IMsgHelper/IMsgInjected.m:6257, 22ccf98cb317)
  • Guards improved after earlier reviews: Later commits removed fallback mutation selectors, required the force-send signature, narrowed direct chat materialization, and added rejection coverage for non-iMessage services. (Sources/IMsgHelper/IMsgInjected.m:6093, 22ccf98cb317)
  • Live proof remains pending: The PR body and latest collaborator comment explicitly state that no exact-head profile share occurred and receiver-visible liveness/share proof remains the gate, despite successful macOS and Linux CI. (22ccf98cb317)
  • Prior changelog finding remains: The release-owned Name & Photo changelog entry is still present on the current head and traces to the initial feature commit. (CHANGELOG.md:16, 39c82acfe54f)

Likely related people:

  • steipete: Peter Steinberger carried the current release baseline and recent bridge hardening, then authored the active replacement and its force-send and iMessage-only guard commits. (role: bridge baseline owner and current decision owner; confidence: high; commits: dea78a9e9c49, 0b5dea7efdca, 3eabfc90cf62; files: Sources/IMsgHelper/IMsgInjected.m, Sources/IMsgCore/IMsgBridgeProtocol.swift, Sources/imsg/RPCServer.swift)
  • omarshahine: Omar Shahine introduced the BlueBubbles private-API bridge, authored substantial recent bridge/RPC features, co-authored this replacement, and supplied the predecessor’s live controller-hang evidence. (role: private-API bridge feature owner and runtime investigator; confidence: high; commits: c56c24d488ef, 5667b5ceb78f, 8e3964f0640e; files: Sources/IMsgHelper/IMsgInjected.m, Sources/IMsgCore/IMsgBridgeProtocol.swift, Sources/imsg/RPCServer.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 (3 earlier review cycles)
  • reviewed 2026-07-11T05:04:32.466Z sha 39c82ac :: found issues before merge. :: [P2] Remove the release-owned changelog edit
  • reviewed 2026-07-11T05:11:14.756Z sha 39c82ac :: found issues before merge. :: [P2] Remove the release-owned changelog edit
  • reviewed 2026-07-11T18:59:48.207Z sha 22ccf98 :: found issues before merge. :: [P2] Remove the release-owned changelog edit

steipete added 3 commits July 11, 2026 19:38
…oto-sharing

# Conflicts:
#	.agents/skills/imsg/SKILL.md
#	CHANGELOG.md
#	README.md
#	Sources/IMsgCore/IMsgBridgeProtocol.swift
#	Sources/IMsgHelper/IMsgInjected.m
@steipete

Copy link
Copy Markdown
Collaborator Author

Exact-head proof for 22ccf98cb317c02c22f34a4ca291c6254a01ec64:

  • merged current main and resolved the bridge/docs conflicts additively
  • removed the two unproven policy-oriented fallback selectors; sharing now requires the explicit four-argument force-send API
  • rejects SMS/unsupported chats before status inspection or mutation; only iMessage and iMessageLite proceed
  • requires non-empty participants and a chat-bound local sender identity; reports request submission, not delivery
  • make test: 477 tests passed
  • make lint: passed with 12 existing non-serious warnings
  • make build: universal CLI and helper built and signed
  • focused Name & Photo tests: 12 passed
  • final whole-branch autoreview: clean, no actionable findings
  • exact-head CI: macOS and Linux green in https://github.com/openclaw/imsg/actions/runs/29164354743

Remaining gate: receiver-visible liveness/share proof on a SIP-disabled Messages host. The available Mac has SIP enabled, the prepared SIP-off host is unavailable, and no recipient was explicitly confirmed for this privacy-sensitive share. No profile was sent. Keeping the PR draft rather than overriding that proof gate.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 11, 2026
@steipete
steipete marked this pull request as ready for review July 11, 2026 20:10
@steipete
steipete merged commit f6cc39e into main Jul 11, 2026
2 checks passed
@steipete
steipete deleted the agent/rebuild-name-photo-sharing branch July 11, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. 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