Skip to content

fix(gateway): deliver ics files as attachments - #38455

Open
saralilyb wants to merge 1 commit into
NousResearch:mainfrom
saralilyb:fix/ics-media-delivery
Open

fix(gateway): deliver ics files as attachments#38455
saralilyb wants to merge 1 commit into
NousResearch:mainfrom
saralilyb:fix/ics-media-delivery

Conversation

@saralilyb

@saralilyb saralilyb commented Jun 3, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes .ics calendar invite delivery through the shared gateway media
extraction path.

Before this change, a response like MEDIA:/tmp/event.ics could remain visible
as literal text instead of being extracted and uploaded as a native document
attachment, because .ics was missing from the gateway document MIME map and
outbound media-delivery extension allowlist.

This is gateway-wide rather than Signal-specific: BasePlatformAdapter.extract_media()
runs before platform-specific dispatch, and non-image media is then routed
through send_document().

Related Issue

N/A — no issue was filed. Related prior closed PR: #33454.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/platforms/base.py — adds .ics to SUPPORTED_DOCUMENT_TYPES as
    text/calendar.
  • gateway/platforms/base.py — adds .ics to MEDIA_DELIVERY_EXTS so calendar
    files are extracted for document delivery.
  • tests/gateway/test_media_extraction.py — adds regression coverage for both
    explicit MEDIA:/.../event.ics tags and bare local .ics paths.

How to Test

  1. Create a local .ics file.
  2. Have an agent response include MEDIA:/path/to/event.ics.
  3. Verify the gateway extracts the file and sends it as a document attachment
    instead of leaving the MEDIA: tag as visible text.

Verification run on this branch:

  • python3 -m pytest tests/gateway/test_media_extraction.py -q → 11 passed
  • python3 scripts/check-windows-footguns.py gateway/platforms/base.py tests/gateway/test_media_extraction.py → clean
  • git diff --check main..HEAD → clean

I also attempted the full suite with python3 -m pytest tests/ -q; it did not
complete in this checkout, timing out after 600s with failures outside the
targeted gateway extraction tests before reaching 40%.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: WSL2 / Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

N/A — regression tests cover the shared extraction behavior.

@alt-glitch alt-glitch added comp/gateway Gateway runner, session dispatch, delivery type/bug Something isn't working P2 Medium — degraded but workaround exists labels Jun 3, 2026
@saralilyb
saralilyb force-pushed the fix/ics-media-delivery branch from aa85f26 to 79eed8b Compare June 17, 2026 19:00
@saralilyb

Copy link
Copy Markdown
Author

Refreshed this branch against current origin/main.

Current head: 79eed8bec6a643f597cf35cb907de579fc1271ba

Verification from the refreshed branch:

  • uv run --with pytest python -m pytest tests/gateway/test_media_extraction.py -q → 15 passed
  • python3 -m py_compile gateway/platforms/base.py tests/gateway/test_media_extraction.py → ok
  • direct smoke test for BasePlatformAdapter.extract_media('MEDIA:/tmp/event.ics') and extract_local_files() with a real temporary .ics file → ok
  • python3 scripts/check-windows-footguns.py gateway/platforms/base.py tests/gateway/test_media_extraction.py → no Windows footguns found
  • git diff --check origin/main...HEAD → ok

The diff is still only the small gateway-wide .ics document delivery allowlist fix plus regression coverage.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current main still omits .ics from MEDIA_DELIVERY_EXTS at gateway/platforms/base.py:1455; that tuple builds MEDIA_TAG_CLEANUP_RE (:1478-1497), which extract_media() uses (:3641-3655), and also drives extract_local_files() (:3745-3755). The reported extraction failure therefore remains present.

The change adds .ics to that shared set and to SUPPORTED_DOCUMENT_TYPES, while the common response dispatcher sends extracted non-image/non-video files through send_document() (gateway/platforms/base.py:5110-5132). The added explicit-tag and real bare-file tests cover both affected extractor paths.

No correctness or completeness issue was found in the two-file diff. This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
@saralilyb
saralilyb force-pushed the fix/ics-media-delivery branch from 79eed8b to a32072f Compare July 30, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants