Skip to content

feat(photon): add native message effects - #43718

Closed
arnoldfrancisca wants to merge 1 commit into
NousResearch:mainfrom
arnoldfrancisca:feat/photon-message-effects
Closed

feat(photon): add native message effects#43718
arnoldfrancisca wants to merge 1 commit into
NousResearch:mainfrom
arnoldfrancisca:feat/photon-message-effects

Conversation

@arnoldfrancisca

Copy link
Copy Markdown

Summary

  • Adds a Photon sidecar /send-effect endpoint backed by spectrum-ts/providers/imessage's native effect(...) builder
  • Adds PhotonAdapter.send_effect(...) for text with native iMessage bubble/screen effects
  • Updates Photon README and website docs to mark message effects as supported
  • Adds regression coverage for the effect endpoint body shape and validation

Why

spectrum-ts already exposes iMessage effects such as slam, loud, gentle, invisible, confetti, fireworks, balloons, heart, lasers, celebration, sparkles, spotlight, and echo. This exposes the sidecar/adapter layer without adding a new model tool or changing the global tool schema.

Verification

  • node --check plugins/platforms/photon/sidecar/index.mjs
  • Local effect-builder smoke test with spectrum-ts/providers/imessageeffect builder ok: com.apple.messages.effect.CKConfettiEffect
  • venv/bin/python -m pytest tests/plugins/platforms/photon/test_outbound_media.py -q -o 'addopts=' → 10 passed
  • venv/bin/python -m pytest tests/plugins/platforms/photon -q -o 'addopts=' → 68 passed

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels Jun 10, 2026
@arnoldfrancisca

Copy link
Copy Markdown
Author

Local verification from fork branch: uv run --extra dev pytest tests/plugins/platforms/photon -q passed (68 passed). GitHub reports mergeable=true, mergeable_state=blocked, no CI check rollup present.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for exposing the Spectrum effect capability without adding a new core tool.

Problems

  • PhotonAdapter.send_effect() is currently unreachable from Hermes's outbound interface. send_message accepts only send, list, react, and unreact in tools/send_message_tool.py:210-250; its live route calls adapter.send() at tools/send_message_tool.py:713-734, and the Photon standalone sender only posts to /send at plugins/platforms/photon/adapter.py:1662-1739. The README/docs therefore overstate current user-visible support.
  • The added test stubs _sidecar_call, so it cannot validate the new JavaScript handler or its spectrum-ts builder/import. Current main pins spectrum-ts 8.0.0 in plugins/platforms/photon/sidecar/package.json:15-17.

Suggested changes

  • Wire effects through a deliberate supported invocation path, or scope this as internal and adjust the docs accordingly.
  • Add sidecar-level coverage for a supported and unsupported effect against the pinned dependency.

This PR is substantially stale relative to the current Photon sidecar, so the implementation should be salvaged onto its current structure rather than applied verbatim. This is an automated hermes-sweeper review.

chat_id, animation_url, caption, reply_to, metadata,
)

async def send_effect(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send_effect() has no caller in the existing delivery surface: send_message dispatches only send/list/react/unreact and its send path invokes adapter.send(). Please wire this through a deliberate supported path (including standalone behavior), or keep it internal and avoid documenting Hermes-level effect support.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #73614 — cherry-picked with authorship preserved (trivially rebased over your poll PR; a follow-up hardened MESSAGE_EFFECTS against SDK builds lacking the effect surface).

@teknium1 teknium1 closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants