Skip to content

feat(outbound): core contract, store resolution, mock/SMS migration (COD-398) - #21

Merged
shivros merged 4 commits into
mainfrom
runner/COD-398-outbound-core
Aug 19, 2026
Merged

feat(outbound): core contract, store resolution, mock/SMS migration (COD-398)#21
shivros merged 4 commits into
mainfrom
runner/COD-398-outbound-core

Conversation

@shivros

@shivros shivros commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

IRIS-11 PR1: Outbound attachments — core contract + store resolution + Mock/SMS migration (T2–T4)

Closes COD-398. First implementation slice of the add-outbound-attachments OpenSpec change (spec merged via #19 / COD-395).

What this does

  • Breaking contract change: MessageProvider::send_message now takes &OutboundMessage { body, attachments } instead of &str.
  • New iris-core::outbound module: OutboundMessage, OutboundAttachment (Bytes { mime_type, filename, bytes } | Stored(Uuid)), ResolvedAttachment, plus resolve_attachments and enforce_capability helpers.
  • New ProviderCapability::SendAttachments: providers without it reject non-empty attachment lists with UnsupportedCapability before any external request. Text-only sends are behavior-identical to main.
  • Store resolution: stored refs resolve via Arc<dyn AttachmentStore> at the provider boundary; a missing stored ID (or missing store) fails the send with no partial dispatch. Inline attachments validate (non-empty MIME, non-empty bytes) and never require a store.
  • Mock: advertises SendAttachments, records complete outbound sends (body + resolved attachments) via recorded_sends() for deterministic round-trip tests; audit metadata stays content-free (mime/filename/byte_count only).
  • SMS: rejects attachments (no SendAttachments); text behavior unchanged.
  • Telegram/Email: compile under the new signature, text-only until their multipart slices (T5/T6 are PR2/PR3).
  • Surfaces (CLI/MCP/HTTP): construct OutboundMessage::text(...) — attachment input expansion is T8–T10, a later PR.

Review panel findings (fixed)

  • Reviewer B (GPT-5.5) caught that the mock rejected inline attachment sends when no store was configured — inline attachments don't need a store. Fixed in b7e6304: resolve_attachments now takes Option<&Arc<dyn AttachmentStore>>; stored refs fail fast, inline always resolve.
  • Reviewer B also noted text-only audit metadata now includes attachment_count: 0 (and attachments: [] for mock). This is deliberate per the frozen design ("audit metadata contains attachment summaries only (count, MIME type, filename, and byte count)") and content-free. Flagging for reviewer awareness.
  • Reviewer A (independent gate re-run) approved clean.

Verification

All five gates pass on this branch:

  • cargo build --all-targets
  • cargo test --all-targets — 94 tests, 0 failures ✅
  • cargo clippy --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • cargo run -p iris-codegen --bin iris-codegen -- check ✅ (generated artifacts untouched by this PR)

Out of scope (follow-up slices)

  • T5 Telegram multipart media sends (PR2)
  • T6 Email MIME multipart sends (PR3)
  • T7 partial-dispatch audit outcomes (lands with Telegram PR2)
  • T8–T10 generated surface attachment inputs

Draft — awaiting Siobhan's review. Do not merge automatically.

@shivros

shivros commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Automated Review Panel

Dual-model review before opening this PR. Reviewer A re-ran the verification gates independently; Reviewer B performed a diff-only second opinion.


Reviewer A (frontier reasoning model, gate re-run)

Verdict: ✅ APPROVE

All frozen contract requirements for PR1 are correctly implemented, and I independently re-ran every gate on the branch (runner/COD-398-outbound-core, clean tree): cargo test --all-targets (all suites green), cargo clippy --all-targets -- -D warnings (clean), iris-codegen check (OK).

Contract compliance verified:

  • Core signature change (send_message(&OutboundMessage)) applied across trait, all four providers, and all three surfaces
  • Capability gating rejects attachments pre-dispatch for providers without SendAttachments
  • Stored-reference resolution failures abort the send with no partial dispatch
  • Audit metadata stays content-free (mime/filename/byte_count summaries only)
  • Text-only send paths behavior-identical to main

Reviewer B (second-opinion diff review)

Verdict: ⛔ BLOCK → resolved

Finding (fixed in b7e6304): MockProvider advertised SendAttachments but rejected all non-text-only sends when no attachment store was configured — including inline Bytes attachments, which never need a store. The doc comment even claimed "inline attachments still work" while the implementation returned IrisError::Config for them. Also prevented inline validation from running on that path.

Fix: resolve_attachments now takes Option<&Arc<dyn AttachmentStore>>. Inline variants validate in place and always resolve; only Stored(Uuid) references require a store and fail fast with a config error when absent. New test mock_without_store_accepts_inline_attachments covers the gap.

Noted, accepted: text-only audit metadata now includes attachment_count: 0 (mock also adds attachments: []). Deliberate per the frozen design's audit-summary requirements; content-free. Flagged in the PR description for reviewer awareness.


Both reviewers' blocking findings are addressed; all five gates green at b7e6304 (94 tests).

@shivros
shivros force-pushed the runner/COD-398-outbound-core branch 2 times, most recently from 08127bd to 5653a42 Compare August 18, 2026 13:39
…COD-398)

- OutboundMessage/OutboundAttachment/ResolvedAttachment in iris-core
- send_message takes &OutboundMessage across trait and all providers
- SendAttachments capability with pre-dispatch gating helper
- resolve_attachments validates inline and resolves stored refs via AttachmentStore
- Mock records complete outbound sends; SMS/email/telegram text-only under new contract
- Surfaces construct text-only OutboundMessage until T8-T10 input expansion
Review panel finding: mock rejected ALL attachment sends when no store was
configured, but inline Bytes attachments never need a store — only Stored(Uuid)
references do. resolve_attachments now takes Option<&Arc<dyn AttachmentStore>>:
inline variants validate in place, stored refs fail fast with a config error
when no store exists. Added mock test covering inline-without-store.
@shivros
shivros force-pushed the runner/COD-398-outbound-core branch from 5653a42 to 0cc3ada Compare August 19, 2026 03:14
@shivros
shivros marked this pull request as ready for review August 19, 2026 03:42
@shivros
shivros merged commit e6c9be7 into main Aug 19, 2026
5 checks passed
@shivros
shivros deleted the runner/COD-398-outbound-core branch August 19, 2026 03:43
shivros added a commit that referenced this pull request Aug 19, 2026
Rebase onto main picked up the outbound send_message signature change
from COD-398. Update the three test provider stubs (sse.rs x2, watch.rs)
to take &OutboundMessage and import it.

Co-authored-by: Archon <archon@purelymail.com>
shivros added a commit that referenced this pull request Aug 19, 2026
…D-400, T9-T12) (#23)

* feat(realtime): SSE subscribe_events handler + iris watch runtime (COD-400, T9-T12)

- Real GET /v1/events handler replacing the 501 stub: provider/thread
  exact-match filters, 422 unsupported_realtime_provider (filtered) /
  503 no_realtime_provider (aggregate-empty) selection statuses, SSE-only
  response headers, per-provider branch tasks fanning into one wire driver.
- Five public terminal error codes (slow_consumer, telegram_conflict,
  audit_failed, retry_exhausted, provider_failed) with URL/bot-token
  sanitization of public messages.
- Aggregate semantics: one branch's terminal error keeps others alive;
  filtered stream closes after its error; aggregate closes after the last
  branch ends.
- Wire-idle comment heartbeat (validated SseSettings, 15s default,
  test-shrinkable); filtered-out events never reset it.
- Disconnect cleanup: dropping the HTTP body closes the frame channel,
  branch tasks drop their provider streams, hubs prune subscriptions.
- iris watch: IRIS_SERVER_URL env (default 127.0.0.1:3000), --provider /
  --thread-id options, spec-compliant SSE frame parser (LF/CRLF/CR, split
  chunks, multiline data, EOF flush), JSONL stdout, stderr diagnostics,
  non-zero exit on filtered error or aggregate end-in-error.
- serve: graceful shutdown now awaits shutdown_realtime() for every
  instantiated provider (SIGINT/SIGTERM).
- 28 new tests: 15 HTTP SSE behavior (statuses, schemas, filters,
  heartbeat, no-reset, disconnect) + 13 CLI watch (parser, e2e against an
  in-process server, exit policies). Binary smoke test: message JSONL +
  stderr diagnostic + exit 1 verified against a live local endpoint.

* fix(realtime): address review panel findings on watch/SSE semantics

- Aggregate exit policy: exit non-zero only when the aggregate stream
  ENDS in error (last event frame was terminal error); a later message
  from a surviving branch clears the flag. Regression test uses a second
  provider branch whose message outlives the first branch's error.
- SSE parser: treat CR as an immediate line terminator; a CRLF split
  across chunks degrades to CR + blank line, dispatching the frame at the
  same point instead of stalling a paused peer.
- IRIS_SERVER_URL: trim trailing '/' before joining /v1/events.
- telegram_conflict classification matches any 'HTTP 409' status text,
  not just the parenthesized form.
- Bare-token redaction accepts any-length numeric bot IDs before ':'.

Both reviewers' findings resolved; 139/139 tests, clippy/fmt/codegen green.

* fix(realtime): adapt test mocks to OutboundMessage send contract (#21)

Rebase onto main picked up the outbound send_message signature change
from COD-398. Update the three test provider stubs (sse.rs x2, watch.rs)
to take &OutboundMessage and import it.

Co-authored-by: Archon <archon@purelymail.com>

---------

Co-authored-by: Archon <archon@purelymail.com>
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.

1 participant