feat(extension-runtime): P5 — delivery coordinator + Slack/Telegram outbound (runtime PR 6/8)

[BenKurrek]

P5 — delivery coordinator + Slack/Telegram outbound (Workstream F)

Fifth PR of the extension-runtime train (design: docs/reborn/extension-runtime/, Workstream F). Base: nea25/13-ingress-router (P4). Everything below is green locally: workspace tests, --features integration (both DBs — Postgres via testcontainers, skip = failure), CI-exact clippy (raw RC 0), architecture + specificity + boundary + no-panics gates.

What this builds (§5.4: the two-halves split)

DeliveryCoordinator (ironclaw_product_workflow::delivery_coordinator) — delivery semantics, one instance per composition, the sole delivery-state writer:

• The nine DeliveryIntents split policy-class (FinalReply/Progress/GatePrompt/AuthPrompt/TriggeredDelivery → deliver(), full outbound-policy pipeline) vs notice-class (FailureNotice/ConnectRequired/Working/Cleanup → deliver_notice(), source-routed on the originating conversation). Cross-class calls are typed errors.
• Persist-before-egress: Prepared → Sending persisted before any vendor call; terminal Delivered/Failed; interrupted Sending from a prior lifetime settles Unknown — never blindly resent (OUT-6). Partial multipart is terminal after any Sent part (OUT-7). Bounded retry, drain, per-delivery-id single-flight.
• ING-11 read half: the coordinator resolves stored reply_context through DeliveryReplyContextSource and hands it to the adapter on the OutboundEnvelope — wired in the factory over the SAME store the ingress router writes.
• CoordinatedDeliveryOutcome::Delivered { conversation, vendor_message_refs } replaces vendor-response sniffing (the old TrackingSlackPostEgress ts-sniffing and resolved_space_id mutex are gone).

Generic run-delivery components (ironclaw_product_workflow::run_delivery) — the watching-and-emitting logic that is identical for every channel, ported from the Slack host:

• RunDeliveryObserver (live source-route path: final reply, gate/auth prompts, working indicator, busy-hint FIFO dedupe, failure notices, retract-on-cleanup) and TriggeredRunDeliveryDriver (preference-target path with pending/delivery semaphores + OAuth-not-DM backstop).
• Vendor residue enters ONLY through small ports: ApprovalPromptContextSource, BlockedAuthPromptSource, BlockedAuthFlowCancel, PreferenceTargetCodec. Composition implements them once, ungated (extension_host/run_delivery_ports.rs).

Adapters (ChannelAdapter::deliver — vendor mechanics only): Slack deliver/list_targets (+ Retract → chat.delete), Telegram deliver (+ Retract → deleteMessage) — no store, no policy, egress via RestrictedEgress only.

The cutover deletes the direct Slack send lane: slack_delivery.rs, slack_egress.rs, slack_dm_open.rs deleted; slack_outbound_targets.rs → slack_preference_targets.rs (provisioner re-pointed over ChannelDeliveryResolver + adapter list_targets); all 24 slack_serve/e2e_tests.rs scenarios re-pointed through the coordinator (24/24 green).

Earlier slices in this PR

• Slice 1–2: ChannelEgressDescriptor.injection (header/query_param/path_placeholder), the policy half of channel RestrictedEgress (OUT-8, reject-before-transport), EgressFactory::egress_for_channel so activate() has working egress pre-publication.
• Slice 3: HostRuntimeChannelEgressTransport (material resolution, declared injection incl. path placeholders for Telegram's token-in-path Bot API, policy pinned to the approved host, private-IP denial, response caps).
• Slice 4: OutboundDeliveryStatus += Prepared/Sending/Unknown, both-DB store contract.
• TEST-1: exported channel-adapter conformance suite (ironclaw_product_adapters::conformance), run by Slack, Telegram, and the acme fixture.

Integration proofs (tests/integration/extension_delivery.rs, both DBs)

• Slack outbound through the REAL coordinator: signed DM event on the production ingress mount → manifest-recipe verification → real workflow admission → real turn → generic observer → factory-built coordinator → chat.postMessage on the wire recorder with the bridged bot token injected host-side + terminal Delivered attempt in the coordinator's own outbound store.
• DEL-10 Telegram (the addition test): bundled package (manifest + adapter crate, zero bespoke host code) installs → activates (setWebhook over recorded egress, token substituted host-side into the URL path placeholder) → signed update through the production router → turn → coordinated sendMessage. Bad shared secret rejected on the wire.

Owner calls / deltas to review (made autonomously, flagged per mission rules)

1. Lazy per-scope crash recovery: the outbound store enumerates attempts per scope only, so interrupted-Sending reconciliation runs lazily before a scope's first delivery (recovered_scopes set) instead of a startup scan. Fail-closed either way (Unknown, no resend).
2. Host-runtime/network egress widening (slice 5): HostRuntimeHttpEgressPort accepts TrustClass::System channel calls with a per-request pinned NetworkPolicy (single approved host, private-IP denial) — review the security posture.
3. §11 credential bridging: BridgedChannelEgressCredentials consults registered bridges (Slack beta bot token from setup config) before the scoped secret store; removed when credentials migrate to [channel.config] storage (P6).
4. §11 sliver: slack_preference_targets.rs (preference-target provisioner glue) stays in composition until P6.
5. Telegram install config rides a test-support seam (publish_bundled_extension_with_config_for_test): the production configure surface for [channel.config] operator values is a P6/H deliverable; activation itself is the real host.activate hook.
6. DirectChat-trigger gating replaces 'D'-prefix channel-id checks (connect nudge, OAuth-URL privacy wording); Slack is_dm_channel uses channel_type == "im" with a 'D' fallback, so behavior is faithful.
7. Prompt bodies are markdown-authored in crates/ironclaw_product_workflow/prompts/*.md (per repo prompt-template rule); gate/auth prompt wording normalized to channel-neutral phrasing.
8. Projection-ref prefix: notice-class attempts persist system-notice:{intent}:{notice_ref} projection refs (new vocabulary; policy-class refs unchanged).
9. Auth-block cancel idempotency key is now channel-auth-block:{run_id} (shared by live + triggered paths so the cancellation contract cannot drift).
10. mrkdwn flag omission: the generic path posts without an explicit mrkdwn flag (Slack defaults it true); asserted absent in fixtures.
11. Allowlist 277 vs the mission's <268 estimate: every entry freed by this PR is retired (281 → 277, shrink-only scanner enforces); the remaining delta was estimation error in the mission brief, not unretired entries. ~20 telegram-crate entries added by slice 8c are inventory-derived and die with P6's crate fold-in.
12. CLI: ironclaw-reborn extension --channel assembles the telegram native factory (gated on slack-v2-host-beta until P6); CLI dep pins updated accordingly.
13. Transport-port fix: the channel egress transport is built from store_graph.local_runtime.host_runtime_http_egress (the override-honoring port) — building from services.host_runtime_http_egress_port() bypassed test recorders and hit real vendors.
14. De-flake: the trigger hook wiring test captures active_run_ref on sight with 5 ms polls (was: 100 ms polls racing run settle).
15. Coverage floor: this PR deletes well-covered slack files and adds heavily-tested generic code; if the CI ratchet flags the denominator shift, the floor gets the documented same-PR recapture (tests/integration/coverage-floor.toml).
16. Test-support seams (zero production bytes): publish_bundled_extension_with_config_for_test, register_static_channel_egress_credentials_for_test, outbound_delivery_stores_for_test.

Checklist

docs/reborn/extension-runtime/checklist.md updated with named evidence: ING-11, ING-12, OUT-1..10, OUT-12, DEL-10, TEST-1, TEST-5 ticked; OUT-11 annotated as the deliberately-remaining presentation-policy plumbing (P6).

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)

• staging
• reborn-integration

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c8e13d05-2805-4f0d-8e04-fc9b04d9a55d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gemini-code-assist]

Code Review

This pull request introduces channel-restricted egress policy enforcement and transport execution for channel adapters, supporting credential injection via headers, query parameters, and path placeholders. It also enhances the outbound delivery lifecycle with new coordinator-visible states (Prepared, Sending, Unknown) to improve crash recovery safety. The review feedback highlights several key improvements: ensuring secret store errors are not silently swallowed in channel_secret, preserving missing credential handle context in egress errors, tightening query parameter validation to prevent injection vulnerabilities, and accounting for percent-encoding of curly braces by url::Url::parse during placeholder substitution.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

In SecretStoreChannelEgressCredentials::channel_secret, any error returned by self.store.lease_once is swallowed and mapped to Ok(None). If the secret store is temporarily unavailable (e.g., due to a database connection issue or decryption failure), this will be incorrectly treated as if the credential does not exist, leading to a misleading AuthRequired error or unauthenticated requests being sent to the vendor. Instead, transient or system errors from lease_once should be propagated as Err(ChannelEgressCredentialError::Unavailable). This aligns with the rule to avoid silently swallowing database errors and instead handle them explicitly.

References

1. Avoid silently swallowing database errors by using .ok().flatten() on results from database operations. Instead, handle errors explicitly using .map_err()? or transpose() to ensure failures are properly surfaced.

[gemini-code-assist]

When mapping runtime HTTP egress errors to RestrictedEgressError, if the error is CredentialUnavailable, we return RestrictedEgressError::AuthRequired with an empty required_secrets list. This prevents the extension host or adapter from knowing which credential handle is missing. Since we have access to the approved.credential context before executing the request, we can extract the required secret handle and pass it to map_runtime_http_error to preserve this context.

        let required_secrets = approved
            .credential
            .as_ref()
            .map(|c| vec![c.handle.clone()])
            .unwrap_or_default();

        let response = self
            .host_egress
            .execute(HostRuntimeHttpEgressRequest {
                extension_id,
                trust: TrustClass::System,
                request: RuntimeHttpEgressRequest {
                    runtime: RuntimeKind::FirstParty,
                    scope: self.request_scope(),
                    capability_id,
                    method: approved.method,
                    url: approved.url,
                    headers: approved.headers,
                    body: approved.body,
                    network_policy: NetworkPolicy {
                        allowed_targets: vec![NetworkTargetPattern {
                            scheme: Some(NetworkScheme::Https),
                            host_pattern: approved.host,
                            port: None,
                        }],
                        deny_private_ip_ranges: true,
                        max_egress_bytes: None,
                    },
                    credential_injections: Vec::new(),
                    response_body_limit: Some(approved.response_body_limit),
                    save_body_to: None,
                    timeout_ms: Some(u32::try_from(approved.timeout_ms).unwrap_or(u32::MAX)),
                },
                credentials,
            })
            .await
            .map_err(|err| map_runtime_http_error(err, required_secrets))?;

[gemini-code-assist]

Update map_runtime_http_error to accept the required_secrets list so it can be populated when returning RestrictedEgressError::AuthRequired.

fn map_runtime_http_error(
    error: ironclaw_host_api::RuntimeHttpEgressError,
    required_secrets: Vec<SecretHandle>,
) -> RestrictedEgressError {
    match error.reason_code() {
        RuntimeHttpEgressReasonCode::PolicyDenied | RuntimeHttpEgressReasonCode::RequestDenied => {
            RestrictedEgressError::PolicyDenied
        }
        RuntimeHttpEgressReasonCode::ResponseBodyLimitExceeded => {
            RestrictedEgressError::ResponseTooLarge
        }
        RuntimeHttpEgressReasonCode::CredentialUnavailable => RestrictedEgressError::AuthRequired {
            required_secrets,
            credential_requirements: Vec::new(),
        },
        RuntimeHttpEgressReasonCode::NetworkError | RuntimeHttpEgressReasonCode::ResponseError => {
            RestrictedEgressError::Transport {
                reason: error.stable_runtime_reason().to_string(),
            }
        }
    }
}

[gemini-code-assist]

The validation for RuntimeCredentialTarget::QueryParam only checks that the name is not empty and does not contain whitespace. However, if the query parameter name contains delimiters like & or =, it can be exploited to inject arbitrary query parameters or corrupt the query string structure (e.g., name = "foo&bar" would inject a bar parameter). We should restrict query parameter names to a safe set of characters, such as ASCII alphanumerics, underscores, and hyphens.

                    crate::RuntimeCredentialTarget::QueryParam { name } => {
                        !name.trim().is_empty()
                            && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
                    }

[gemini-code-assist]

Note that url::Url::parse automatically percent-encodes curly braces { and } in the URL path to %7B and %7D. If the host-side transport or substitution logic performs placeholder replacement (e.g., looking for {token}) on a parsed Url object or after re-serializing it, it may fail to find the placeholder. Ensure that any substitution logic either operates on the raw string before parsing, or is prepared to handle percent-encoded placeholders like %7Btoken%7D.

[BenKurrek]

Superseded by #6090 — the Train B runtime rollup (P0–P7b squashed into one tree-identical commit on the docs bridge codex/nea25-generic-extension-runtime; tree byte-identical to nea25/17-finalize). This branch is untouched and remains the review record, so reopening is possible. Follow-up work (P6 owner-call fixes + finalization) continues on #6090.