Skip to content

Feature/subscriptions phase 3 Email Channel#75

Merged
smunini merged 21 commits into
mainfrom
feature/subscriptions
Apr 26, 2026
Merged

Feature/subscriptions phase 3 Email Channel#75
smunini merged 21 commits into
mainfrom
feature/subscriptions

Conversation

@aacruzgon
Copy link
Copy Markdown
Contributor


Summary

Implements Phase 3 of the FHIR Topic-Based Subscriptions roadmap: the email
channel. Builds on top of the Phase 1 / Phase 2 infrastructure without any
breaking changes.

Clients subscribe with channel.type = "email" and a mailto: endpoint; the
engine delivers the FHIR notification Bundle as an SMTP email — short
human-readable summary in the body, full Bundle JSON attached as
notification.json. Server defaults (From, Subject) come from
HFS_SUBSCRIPTION_SMTP_* env vars; per-subscription header entries override
them. R4 uses the Subscriptions R5 Backport IG; R4B / R5 use the native
channel; R6 is compiled but not exercised at runtime in smoke (matches the
existing convention).

Related: discussion #59, Phase 1 PR #61, Phase 2 PR #62, smoke workflow PR
#63.

What changed

helios-subscriptions crate

New files:

  • src/channels/email.rs — EmailChannel implementing ChannelDispatcher. Wraps
    an AsyncSmtpTransport from lettre behind a small
    EmailTransport trait so unit tests can inject a capturing stub. Builds a
    multipart/mixed MIME message: text/plain summary body + notification.json
    attachment for FHIR JSON payloads. Includes 13 unit tests using a
    CapturingTransport stub.
  • tests/email_integration.rs — 5 integration tests against a real
    axllent/mailpit container via testcontainers: handshake + event-notification
    end-to-end, empty-payload no-attachment, full-resource-over-plain-SMTP
    rejection, custom-Subject override, non-mailto: rejection at registration.
    Tests skip cleanly when Docker isn't available.

Modified files:

  • Cargo.toml — add lettre 0.11 (smtp-transport, tokio1, rustls-tls, builder);
    dev-deps testcontainers 0.27 and reqwest (for polling the mailpit HTTP API)
  • src/channels/mod.rs — declare the new email module
  • src/config.rs — add SmtpSettings { host, port, username, password,
    encryption, from_address, default_subject, timeout_secs } and SmtpEncryption {
    None, StartTls, Tls }; add smtp: Option to SubscriptionConfig
    (defaults to None so the channel is opt-in)
  • src/engine/mod.rs — add email_channel: Option<Arc> field;
    build it in SubscriptionEngine::new from config.smtp; add ChannelType::Email
    arms to activate_subscription() and dispatch_with_retry()
  • src/manager/mod.rs — validate that email subscriptions carry a non-empty
    mailto: endpoint at register() time (parallel to the existing rest-hook check)
  • README.md — flip the channel-support row to "Implemented", add the SMTP
    env-var table, add a worked email Subscription example

helios-rest crate

Modified files:

  • src/lib.rs — new build_smtp_settings_from_env() helper reads
    HFS_SUBSCRIPTION_SMTP_HOST, _PORT, _USERNAME, _PASSWORD, _ENCRYPTION, _FROM,
    _DEFAULT_SUBJECT, _TIMEOUT_SECS. Email is advertised in
    supported_channel_types only when both HOST and FROM are set; otherwise any
    inbound email Subscription is rejected with UnsupportedChannel.

Smoke + CI

Modified files:

  • crates/hfs/tests/subscriptions/run_external_subscriptions_smoke.sh — new
    EMAIL_DIR block keyed off MAILPIT_HTTP_URL. Creates an email Subscription,
    polls mailpit's HTTP API for the handshake email, posts an Encounter, polls
    for the event-notification email, fetches the notification.json attachment,
    and runs the same NOTIFICATION_TYPE_JQ / EXPECTED_BUNDLE_TYPE assertions used
    for rest-hook and websocket. Skips with a pass: ... line when MAILPIT_HTTP_URL
    is unset.
  • .github/workflows/subscriptions-smoke.yml — new "Start mailpit" step
    (axllent/mailpit:latest, exposes 1025/8025 on ephemeral host ports), exports
    HFS_SUBSCRIPTION_SMTP_* and MAILPIT_HTTP_URL to $GITHUB_ENV, captures
    container logs on failure, cleans up on exit.
  • .github/workflows/subscriptions-channels.yml — new email-integration job
    mirroring rest-hook-integration and websocket-integration across the 15-config
    × FHIR-version matrix.

Design decisions

Body shape: summary + JSON attachment. Real-world FHIR email integrations
(Epic, IRIS, public-health workflows) typically attach the bundle rather than
inline it. We render a short plain-text summary (topic URL, notification type,
event number, focus references) in the body and attach the full Bundle as
notification.json with the subscription's payload MIME type
(application/fhir+json by default). For content=empty we skip the attachment
entirely; for text/plain MIME we skip the JSON attachment and put only the
summary in the body.

Header precedence: server default, subscription override.
HFS_SUBSCRIPTION_SMTP_FROM and _DEFAULT_SUBJECT set the server-wide defaults;
subscription channel.header entries (R4 backport) and Subscription.parameter
entries (native) named Subject, From, Reply-To, or Cc override per
subscription. Other header names are ignored — SMTP doesn't have an HTTP-style
"any header" contract.

TLS gating for full-resource payloads. Subscriptions requesting
content=full-resource over an encryption=none transport are rejected at
dispatch time with a PermanentError. This mirrors the HTTPS requirement on the
rest-hook channel (channels/rest_hook.rs:51) — full-resource payloads should
never traverse a plaintext SMTP hop.

Email is opt-in. With HFS_SUBSCRIPTION_SMTP_HOST or HFS_SUBSCRIPTION_SMTP_FROM
unset, "email" is omitted from supported_channel_types and any inbound email
Subscription is rejected with UnsupportedChannel — consistent with how the
engine already gates websocket support.

Transport abstraction for testability. The dispatcher takes Arc (a small async trait) rather than a concrete
AsyncSmtpTransport. Production uses a LettreSmtpTransport wrapper; unit tests
use a CapturingTransport that records Messages in a Mutex<Vec> so we
can assert on subject/from/multipart shape without a real SMTP server.
Integration tests use a real mailpit container for the SMTP wire-format
guarantee.

FHIR XML payload deferred. payload_mime_type = "application/fhir+xml" returns
a PermanentError for now. Easy follow-up; not a Phase-3 requirement.

Test plan

  • cargo test -p helios-subscriptions — 141 tests pass (125 unit + 13 new email
    channel unit tests + 2 rest-hook integration + 9 websocket integration + 5
    email integration gracefully skipped without Docker)
  • cargo test -p helios-subscriptions --no-default-features --features "R4,R4B"
    --test email_integration --no-run — compiles
  • cargo test -p helios-subscriptions --no-default-features --features "R4,R5"
    --test email_integration --no-run — compiles
  • cargo fmt --all -- --check — clean
  • cargo check -p helios-hfs --features subscriptions — clean
  • Manual end-to-end:

Shell A

docker run --rm -p 1025:1025 -p 8025:8025 axllent/mailpit

Shell B

HFS_SUBSCRIPTIONS_ENABLED=true
HFS_SUBSCRIPTION_SMTP_HOST=localhost HFS_SUBSCRIPTION_SMTP_PORT=1025
HFS_SUBSCRIPTION_SMTP_ENCRYPTION=none
HFS_SUBSCRIPTION_SMTP_FROM=hfs@local.test
cargo run --bin hfs --features subscriptions

Shell C

MAILPIT_HTTP_URL=http://localhost:8025 FHIR_VERSION=R4
./crates/hfs/tests/subscriptions/run_external_subscriptions_smoke.sh

→ summary.md shows PASS for rest-hook, websocket, and email

http://localhost:8025 inbox shows handshake + event-notification emails

with notification.json attached

smunini
smunini previously approved these changes Apr 25, 2026
cargo llvm-cov --workspace was failing because the email integration
tests need a mailpit Docker container that times out on the coverage
runner. The same tests run successfully in subscriptions-channels.yml
where mailpit is provisioned. Filter them out so the coverage job can
generate lcov.info and publish to Codecov.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2026

Codecov Report

❌ Patch coverage is 79.96390% with 111 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/subscriptions/src/channels/email.rs 84.36% 81 Missing ⚠️
crates/subscriptions/src/engine/mod.rs 14.28% 18 Missing ⚠️
crates/subscriptions/src/manager/mod.rs 14.28% 12 Missing ⚠️

📢 Thoughts on this report? Let us know!

smunini added 3 commits April 25, 2026 10:38
Several pieces of fhirpath hardcoded R4 in lib code or tests, breaking
the subscriptions-channels matrix slices that build without R4 (single-
R4B, single-R5, single-R6, R4B-R5, etc.):

- crates/fhirpath/src/cli.rs:129 used `default_value_t = FhirVersion::R4`
  in the CLI args struct, so the lib failed to compile when fhirpath had
  only R4B/R5/R6 enabled. Replaced with a `default_fhir_version()` helper
  using the same cfg-ladder pattern as evaluator.rs and handlers.rs.
- The cli test helper hardcoded R4 too; switched to the same helper so
  the generic Patient-JSON tests work under any single-version build.
- not_function.rs::test_not_integer asserts R4-specific integer-falsy
  semantics; gated the whole test mod on `feature = "R4"`.
- terminology_client.rs tests passed FhirVersion::R4 just to exercise URL
  normalization; replaced with an `any_version()` cfg-ladder helper.
- handlers.rs::test_evaluator_format_r4 was ungated; gated on R4 to match
  its R5 sibling, plus gated the imports it consumes accordingly.
- 14 fhirpath integration test files import from `helios_fhir::r4::*`
  unconditionally; added a top-level `#![cfg(feature = "R4")]` to each
  so they're skipped (rather than failing) under non-R4 builds.

Workflows:
- subscriptions-channels.yml unit-tests step uses `--tests` to skip
  doctests (many fhirpath doctests use `helios_fhir::r4::*` as examples
  and don't compile in non-R4 single-version builds; doctest coverage is
  preserved in the main ci.yml run).
- Bumped `actions/checkout@v4` -> `@v5` in subscriptions-channels.yml
  and hts-benchmark.yml to clear the Node 20 deprecation warning.
…ating

Revert most of 70a466c. The matrix should not run with
--no-default-features, because helios-audit (a transitive default of
helios-rest) is structurally R4-only — dropping defaults forces R4 off
in helios-fhirpath while audit's transitive R4 stays on, creating a
runtime dispatch mismatch. Keep R4 + sqlite + audit in the matrix and
add the slice's extra FHIR versions on top.

With defaults retained, the previous fhirpath cfg-ladder defaults and
test gating become unnecessary, so revert those (cli.rs, handlers.rs,
not_function.rs, terminology_client.rs, and the 14 tests/* gates).

Workflow action upgrades (actions/checkout@v4 -> @v5 in
hts-benchmark.yml and subscriptions-channels.yml) from 70a466c are
preserved.
clippy 0.1.95 (CI) flags `map: _, ..` as
clippy::unneeded_wildcard_pattern; the `..` already covers it.
smunini
smunini previously approved these changes Apr 25, 2026
@smunini
Copy link
Copy Markdown
Contributor

smunini commented Apr 25, 2026

@claude the ci.yml is failing because we need to have that step load a docker image for the mail tests

@claude
Copy link
Copy Markdown

claude Bot commented Apr 25, 2026

Claude finished @smunini's task in 2m 6s —— View job


Fixing CI for email/mailpit Docker image

  • Read ci.yml and subscriptions-channels.yml to understand what's missing
  • Identified the root cause from the failing job log
  • Provide the fix

Root cause

The Test Rust job in ci.yml runs on the Windows self-hosted runner. When cargo test --workspace --all-features reaches the email_integration tests, testcontainers tries to pull axllent/mailpit:latest on demand. The Docker image pull consumes most of the 60-second WaitContainer timeout, so the container never reaches the "accessible via" ready message in time:

start mailpit container: WaitContainer(StartupTimeout)

The fix is to pre-pull the image before tests run.

Fix needed in .github/workflows/ci.yml

I'm unable to modify workflow files (GitHub App permissions don't allow it), so you'll need to apply this manually. Add the following step to the test-rust job, between "Configure Rust to use LLD (Linux)" and "Run tests with all features" (around line 68):

      - name: Pre-pull mailpit Docker image
        run: docker pull axllent/mailpit:latest

The full relevant block should look like:

      - name: Configure Rust to use LLD (Linux)
        if: runner.os == 'Linux'
        run: |
          mkdir -p ~/.cargo
          ...

      - name: Pre-pull mailpit Docker image
        run: docker pull axllent/mailpit:latest

      - name: Run tests with all features
        run: |
          cargo test --workspace --all-features

Note: The same pre-pull step should also be added to subscriptions-channels.yml's email-integration job for consistency — those jobs use the same image via testcontainers. The subscriptions-channels.yml jobs run on Linux where Docker Hub pulls are typically fast enough not to hit the timeout, but adding the pre-pull there too avoids any flakiness under load.

View failing job |

smunini added 4 commits April 26, 2026 08:32
`testcontainers` 0.27's watchdog spawns a signal-hook background thread
on first container start and removes registered containers when the
test process receives SIGTERM/SIGINT/SIGQUIT. Without it, GitHub
Actions cancellations (which send SIGTERM) bypass `Drop` on
`ContainerAsync` and `#[ctor::dtor]` hooks, leaking postgres
testcontainers on every cancelled or timed-out run.
Avoids the testcontainers 60s startup timeout on the Windows
self-hosted runner, where the on-demand Docker Hub pull consumes most
of the budget before mailpit can signal ready.
Mailpit emits all of its log lines (including the "[http] accessible
via …" readiness message) on stdout. The test was waiting for that
substring on stderr, so testcontainers never saw it and every email
integration test hit WaitContainer(StartupTimeout) after 60s.
@smunini smunini merged commit 05cec2c into main Apr 26, 2026
16 checks passed
@aacruzgon aacruzgon deleted the feature/subscriptions branch April 28, 2026 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants