feat(reborn): project Slack ingress routes from the manifest, delete the Rust policy literals - #5626
Conversation
…dential coherence Re-derives the one still-valuable idea from the superseded earliest-reborn ingress stack (#5072/#5093/#5100/#5107, closed) fresh atop main's current `ironclaw_host_api::ingress` contract — as a single small, self-contained registry-only change, instead of rebasing a ~250-commit-stale 4-deep stack built on a `host_ingress_registry` crate main never adopted. A ProductAdapter manifest section may now declare `[[...host_ingress]]` routes, each carrying a full host-owned `IngressRouteDescriptor` (validated by host_api's own Deserialize — dotted route id, absolute path, and every policy invariant including the fail-closed floor that a `public_webhook` listener MUST require `webhook_signature`) plus the `credential_handles` that verify it. The registry does NOT re-own ingress route/policy vocabulary; it projects the descriptor and adds the binding host_api deliberately lacks — ingress credential coherence, enforced fail-closed: - every credential handle must be declared in `required_credentials` (mirrors the egress rule; ingress handles flow into the same declared set installation bindings validate against), - an auth-required route must name at least one verifying credential handle (no route nothing can authenticate), - route ids stay distinct within a section. This is exactly the seam PR #5107's review flagged as fail-open/TODO'd. The serve-layer generic mount (descriptor -> axum route + reused SharedSecretHeader/Hmac verifier) is the deliberate follow-up; main's PublicRouteMount + descriptor-fold middleware already generalize, so it is a small wiring change with no new crate. Tests: ingress credential-coherence matrix (undeclared handle, auth-required route missing credential, duplicate route id, happy projection) as focused unit tests in src/lib.rs; wire-path projection, the inherited fail-closed floor, and coherence-over-the-wire as integration tests in tests/manifest_ingestion.rs. fmt + clippy (all-features, --tests) clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ChangesManifest-driven Slack host-ingress route projection
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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. Comment |
github-actions
Bot
added
size: L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the Slack ingress route definitions (events and commands) from hardcoded Rust literals in slack_serve.rs to declarative data in the bundled Slack extension manifest (manifest.toml). It introduces a helper module host_ingress to project these route descriptors from the manifest at runtime, and updates the tests to verify that the projected descriptors match the expected pre-migration configurations. The reviewer suggests caching the projected descriptors for both the Slack events and commands routes using std::sync::LazyLock to avoid parsing the manifest TOML repeatedly on every call.
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.
| pub fn slack_events_route_descriptors() -> Vec<IngressRouteDescriptor> { | ||
| let descriptor = IngressRouteDescriptor::new( | ||
| SLACK_EVENTS_ROUTE_ID, | ||
| NetworkMethod::Post, | ||
| SLACK_EVENTS_PATH, | ||
| slack_events_policy(), | ||
| ) | ||
| .expect("Slack events route descriptor must validate at startup"); // safety: route id/path are crate-local literals and policy is built by sibling helper. | ||
| vec![descriptor] | ||
| vec![bundled_slack_ingress_descriptor(SLACK_EVENTS_ROUTE_ID)] | ||
| } |
There was a problem hiding this comment.
To avoid redundant parsing of the Slack manifest TOML on every call to slack_events_route_descriptors, we should cache the projected descriptor using std::sync::LazyLock. This aligns with the general rule to avoid redundant parsing of configuration files by projecting them exactly once.
| pub fn slack_events_route_descriptors() -> Vec<IngressRouteDescriptor> { | |
| let descriptor = IngressRouteDescriptor::new( | |
| SLACK_EVENTS_ROUTE_ID, | |
| NetworkMethod::Post, | |
| SLACK_EVENTS_PATH, | |
| slack_events_policy(), | |
| ) | |
| .expect("Slack events route descriptor must validate at startup"); // safety: route id/path are crate-local literals and policy is built by sibling helper. | |
| vec![descriptor] | |
| vec![bundled_slack_ingress_descriptor(SLACK_EVENTS_ROUTE_ID)] | |
| } | |
| pub fn slack_events_route_descriptors() -> Vec<IngressRouteDescriptor> { | |
| static DESCRIPTOR: std::sync::LazyLock<IngressRouteDescriptor> = std::sync::LazyLock::new(|| { | |
| bundled_slack_ingress_descriptor(SLACK_EVENTS_ROUTE_ID) | |
| }); | |
| vec![DESCRIPTOR.clone()] | |
| } |
References
- Avoid redundant parsing and iteration of configuration files (e.g., TOML manifests) by projecting them into typed entries exactly once, validating the set, and replaying the pre-parsed entries.
| pub fn slack_commands_route_descriptors() -> Vec<IngressRouteDescriptor> { | ||
| let descriptor = IngressRouteDescriptor::new( | ||
| SLACK_COMMANDS_ROUTE_ID, | ||
| NetworkMethod::Post, | ||
| SLACK_COMMANDS_PATH, | ||
| slack_commands_policy(), | ||
| ) | ||
| .expect("Slack commands route descriptor must validate at startup"); // safety: route id/path are crate-local literals and policy is built by sibling helper. | ||
| vec![descriptor] | ||
| } | ||
|
|
||
| fn slack_commands_policy() -> IngressPolicy { | ||
| IngressPolicy::new(IngressPolicyParts { | ||
| listener_class: ListenerClass::PublicWebhook, | ||
| auth: IngressAuthPolicy::Required { | ||
| schemes: vec![IngressAuthScheme::WebhookSignature], | ||
| }, | ||
| scope_source: IngressScopeSource::HostResolved, | ||
| body_limit: BodyLimitPolicy::Limited { | ||
| max_bytes: SLACK_COMMANDS_BODY_LIMIT_BYTES, | ||
| }, | ||
| rate_limit: RateLimitPolicy::Limited { | ||
| // Coarse pre-auth abuse guard, mirroring the events route. A second | ||
| // post-verification bucket keyed by the resolved installation is | ||
| // applied inside `SlackIngressService::resolve_command`. | ||
| scope: RateLimitScope::Global, | ||
| max_requests: SLACK_COMMANDS_MAX_REQUESTS, | ||
| window_seconds: SLACK_COMMANDS_RATE_WINDOW_SECONDS, | ||
| }, | ||
| cors: CorsPolicy::NotApplicable, | ||
| websocket_origin: WebSocketOriginPolicy::NotApplicable, | ||
| streaming: StreamingMode::None, | ||
| audit: AuditTraceClass::PublicCallback, | ||
| effect_path: AllowedEffectPath::ProductWorkflow, | ||
| }) | ||
| .expect("Slack commands ingress policy must validate") // safety: policy combines validated constants and host-resolved webhook-signature scope. | ||
| vec![bundled_slack_ingress_descriptor(SLACK_COMMANDS_ROUTE_ID)] | ||
| } |
There was a problem hiding this comment.
To avoid redundant parsing of the Slack manifest TOML on every call to slack_commands_route_descriptors, we should cache the projected descriptor using std::sync::LazyLock. This aligns with the general rule to avoid redundant parsing of configuration files by projecting them exactly once.
pub fn slack_commands_route_descriptors() -> Vec<IngressRouteDescriptor> {
static DESCRIPTOR: std::sync::LazyLock<IngressRouteDescriptor> = std::sync::LazyLock::new(|| {
bundled_slack_ingress_descriptor(SLACK_COMMANDS_ROUTE_ID)
});
vec![DESCRIPTOR.clone()]
}References
- Avoid redundant parsing and iteration of configuration files (e.g., TOML manifests) by projecting them into typed entries exactly once, validating the set, and replaying the pre-parsed entries.
|
🚅 Deployed to the ironclaw-pr-5626 environment in ironclaw-ci-preview
|
…obustness + type nits Addresses bot review feedback on #5625: - gemini (security): a public (no-auth) host-ingress route declaring a credential handle is incoherent and misleading (a reader would assume it is authenticated). Add PublicIngressRouteHasCredential and reject it, making the coherence rule symmetric with the auth-required-needs-credential rule. Adds two unit tests (rejection + the valid no-credential complement). - gemini (types): store &IngressRouteId in the route-id dedup set instead of downgrading to &str. - copilot/coderabbit (tests): the fail-closed-floor wire test now accepts both RegistryError::Manifest(_) and ManifestSectionParse, so it pins the fail-closed behavior rather than the error-routing path. - copilot (naming): documented on HostIngressRoute::credential_handles why the shared EgressCredentialHandle newtype is reused for ingress (type-placement rule) and that its Display leaks no "egress" wording. cargo fmt + clippy (all-features, --tests) clean; registry tests green (6 unit incl. the 2 new public-route cases + 9 manifest + 13 contract). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…the Rust policy literals Makes the manifest-driven ingress contract from #5625 load-bearing: Slack's two inbound routes are now declared as data in the bundled extension manifest and projected into descriptors at serve time, instead of being hand-written Rust policy literals. This is the real example of usage the mechanism needed. - assets/slack/manifest.toml declares `[[product_adapter.inbound.host_ingress]]` for `slack.events` and `slack.commands`, each naming `slack_bot_token` as its verifying credential (fail-closed credential coherence, enforced by the registry). - slack_serve::slack_events_route_descriptors / slack_commands_route_descriptors now project their descriptor from the bundled manifest via a new generic helper (composition::host_ingress::bundled_host_ingress_descriptor), and the two hardcoded slack_events_policy() / slack_commands_policy() literals are deleted. - Only the declarative descriptor moved to the manifest. The axum handler and the HMAC verifier (behavior) stay in Rust — a manifest cannot carry behavior. The descriptor is validated by ironclaw_host_api on deserialize (dotted route id, absolute path, and the fail-closed floor that a public_webhook listener MUST require webhook_signature) and by ironclaw_product_adapter_registry for ingress credential coherence — so a manifest cannot declare a weaker route than the Rust literal did. Stacked on #5625 (needs ProductAdapterHostApiSection::host_ingress()). This also answers the "unused API" question on #5625: the accessor now has a production consumer that deletes per-channel Rust. Tests: two behavior-preserving equivalence guards assert the manifest-projected descriptor is byte-for-byte identical to the pre-migration literal (slack_{events,commands}_route_descriptor_matches_manifest_projection); the existing 377 Slack serve/e2e/handler tests still drive real signed and forged webhooks through the projected mounts (all pass). fmt + clippy (slack-v2-host-beta,webui-v2-beta,libsql; --tests) clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses gemini review feedback on #5626: project each Slack route descriptor from the bundled manifest exactly once (LazyLock) instead of re-parsing the manifest TOML on every slack_{events,commands}_route_descriptors() call. The manifest is a compile-time constant, so the projection is deterministic and safe to memoize for the process lifetime. Behavior-preserving: the equivalence guards and the full 377-test Slack serve suite still pass; clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
82f5a58 to
fd152f9
Compare
IronLoop Review StatusHead: Current reviewers:
Recent activity:
Commands:
|
|
Thanks for the review — addressed (pushed in
Behavior-preserving: the two equivalence guards and the full 377-test Slack serve/e2e/handler suite still pass; |
serrrfirat
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Review summary for fd152f940babd8a4a82f969a967c86ff8af28eda:
- Security: clean
- Bugs: reviewer timed out; not included
- Performance/concurrency: clean
- Tests: 1 low-confidence gap worth adding
- Conventions/API contract: 1 medium finding
- Thermo-nuclear maintainability: 3 findings, with the two higher-signal ones inlined
The main issue is that the new manifest contract says credential_handles are the credentials that verify the ingress route, but the Slack runtime verifier is built from the Slack signing secret, not the bot token declared in the manifest.
| # handler + HMAC verifier stay in Rust. Credential coherence: each route is | ||
| # verified by `slack_bot_token`, which is declared in required_credentials above. | ||
| [[product_adapter.inbound.host_ingress]] | ||
| credential_handles = ["slack_bot_token"] |
There was a problem hiding this comment.
The new contract says these credential_handles are the credentials that verify the ingress route, but Slack verification is not done with slack_bot_token. The runtime builds WebhookAuth::Hmac from config.signing_secret (crates/ironclaw_reborn_composition/src/slack_host_beta.rs:956-961), and setup stores that under a separate signing_secret_handle (slack_setup.rs:37-38, 304-309). With this manifest, future registry/installation checks will treat the outbound bot token as the inbound verifier even though the actual HMAC path uses the signing secret. Either declare the signing-secret credential handle here, or rename/soften this manifest field/docs so it represents required installation credentials rather than verifier credentials.
There was a problem hiding this comment.
Good catch — fixed in 27731cb by declaring the real verifier. The manifest now declares slack_signing_secret in required_credentials and both routes' credential_handles name it instead of slack_bot_token (which stays as the egress credential only). This matches the runtime: WebhookAuth::Hmac is built from config.signing_secret. Safe for existing installs because registry binding validation is "bindings ⊆ declared" (validate_installation), so declaring an extra handle never invalidates an installation.
| ) | ||
| .expect("Slack events route descriptor must validate at startup"); // safety: route id/path are crate-local literals and policy is built by sibling helper. | ||
| vec![descriptor] | ||
| vec![SLACK_EVENTS_DESCRIPTOR.clone()] |
There was a problem hiding this comment.
This still leaves the mount path/method duplicated outside the manifest descriptor. The descriptor now owns route_pattern and method, but slack_events_route_mount still independently mounts SLACK_EVENTS_PATH with post(...) above, and the commands route repeats the same split. That means the manifest can drift from what Axum actually mounts, with equality tests catching drift after the fact instead of the construction making drift impossible. Consider projecting a typed Slack ingress route spec once and building both the router and descriptor list from that same descriptor, failing closed if the descriptor method is not POST.
There was a problem hiding this comment.
Fixed in 27731cb. Both mount functions now build their axum route from the projected descriptor (descriptor.route_pattern().as_str()), and the projection fails closed at startup if a route ever declares a non-POST method (bundled_slack_post_descriptor), since the handlers are wired with post(...). The SLACK_*_PATH consts remain only as the pub API other modules use for URL display, with the existing equality tests pinning const ↔ manifest — but what axum mounts now comes from the manifest descriptor, so mount/manifest drift is structurally impossible.
| manifest_toml: &str, | ||
| route_id: &str, | ||
| ) -> Result<IngressRouteDescriptor, HostIngressProjectionError> { | ||
| let record = parse_product_adapter_manifest_record( |
There was a problem hiding this comment.
This helper introduces a second manifest-ingestion path for serve descriptors: it reparses the bundled manifest with HostPortCatalog::empty() and only the product-adapter parser, then calls product_adapter_sections even though parse_product_adapter_manifest_record already validated those sections. That makes future manifest changes harder to reason about because bundled extension installation and serve-time descriptor projection can stop using the same parsing context. A cleaner shape would reuse the same parsed bundled package/manifest path as available_extensions, or add a registry API that returns the already-projected ProductAdapter sections and project all Slack ingress routes once.
There was a problem hiding this comment.
Fixed in 27731cb. host_ingress no longer parses with HostPortCatalog::empty() + a product-adapter-only contract registry — it now uses the exact same parsing context as bundled extension installation (default_host_port_catalog() + default_host_api_contract_registry() + from_toml_with_contracts, the same calls bundled_extension_package makes), so a bundled manifest cannot be installable yet fail serve-time projection or vice versa. It also projects all routes in one pass (bundled_host_ingress_descriptors) with a separate id selector, and slack_serve projects both Slack routes from one parse in a single LazyLock.
| .flat_map(|section| section.host_ingress()) | ||
| .find(|route| route.descriptor().route_id().as_str() == route_id) | ||
| .map(|route| route.descriptor().clone()) | ||
| .ok_or_else(|| HostIngressProjectionError::RouteNotDeclared { |
There was a problem hiding this comment.
This new RouteNotDeclared branch is not directly tested. The Slack tests only request the two routes present in the manifest, and the registry tests cover manifest projection rather than this selector's missing-route behavior. Please add a focused test like host_ingress::tests::bundled_host_ingress_descriptor_rejects_missing_route_id that passes a valid bundled manifest but asks for an absent route id.
There was a problem hiding this comment.
Added in 27731cb: host_ingress::tests::bundled_host_ingress_descriptor_rejects_missing_route_id projects a valid bundled manifest and asserts an absent route id yields RouteNotDeclared (plus a happy-path projection test on the same fixture).
| # verified by `slack_bot_token`, which is declared in required_credentials above. | ||
| [[product_adapter.inbound.host_ingress]] | ||
| credential_handles = ["slack_bot_token"] | ||
| descriptor = { route_id = "slack.events", method = "post", route_pattern = "/webhooks/slack/events", policy = { listener_class = "public_webhook", auth = { type = "required", schemes = ["webhook_signature"] }, scope_source = "host_resolved", body_limit = { type = "limited", max_bytes = 1048576 }, rate_limit = { type = "limited", scope = "global", max_requests = 12000, window_seconds = 60 }, cors = "not_applicable", websocket_origin = "not_applicable", streaming = "none", audit = "public_callback", effect_path = { type = "product_workflow" } } } |
There was a problem hiding this comment.
Now that the manifest is the canonical home for this route policy, the deeply nested one-line inline table is harder to review than the Rust literal it replaces. Future auth/body-limit/rate-limit/audit/effect-path changes will be buried in a long single-line diff. Please expand the descriptor into multiline TOML tables, or introduce a narrower manifest route shape that keeps each policy field reviewable.
There was a problem hiding this comment.
Fixed in 27731cb. Each descriptor and policy is now a multiline TOML table ([product_adapter.inbound.host_ingress.descriptor] / .policy), one field per line, so auth/body-limit/rate-limit/audit/effect-path changes diff line-by-line. Only the short leaf values (e.g. auth, body_limit) remain as one-line inline tables.
…gle manifest parse path Address PR #5626 review feedback: - manifest.toml: the ingress routes' credential_handles now name slack_signing_secret (declared in required_credentials) — the secret the runtime's HMAC webhook verifier is actually built from — instead of the outbound slack_bot_token. Descriptors expanded from one-line inline tables into multiline TOML tables so policy changes diff field-by-field. - slack_serve: both descriptors project from one manifest parse via a single LazyLock, and the axum mounts now build their route path from the projected descriptor (failing closed at projection time if a route ever declares a non-POST method), so what axum mounts cannot drift from what the manifest declares. - host_ingress: parsing reuses the same context as bundled extension installation (default host-port catalog + default host-API contract registry) instead of a second ingestion path with an empty catalog; projects all routes in one pass with a separate route-id selector. - host_ingress tests: cover the happy projection and the previously untested RouteNotDeclared branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
scope: docs
There was a problem hiding this comment.
Pull request overview
This PR makes Slack’s Reborn host-ingress routes manifest-driven: the serve layer now projects IngressRouteDescriptors from the bundled Slack extension manifest instead of maintaining hand-written Rust policy literals, while keeping the actual axum handlers + HMAC verification logic in Rust.
Changes:
- Declare Slack’s
slack.events/slack.commandshost-ingress routes in the bundled Slack extension manifest and add the inbound verifying credential (slack_signing_secret). - Update
slack_serveto mount routes using manifest-projected descriptors (via a cached, single-parse projection) and delete the old Rust policy literal builders. - Add a composition helper to project host-ingress descriptors from bundled manifests; extend registry parsing/validation + tests to support
host_ingressroutes with fail-closed credential coherence.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ironclaw_reborn_composition/src/slack_serve.rs | Switch Slack route mounting/descriptors to use manifest-projected IngressRouteDescriptors; remove Rust policy literals; add migration-guard equality tests. |
| crates/ironclaw_reborn_composition/src/lib.rs | Add the new host_ingress module behind slack-v2-host-beta. |
| crates/ironclaw_reborn_composition/src/host_ingress.rs | New helper to parse bundled manifest TOML and project host-ingress descriptors; lookup helper by route id. |
| crates/ironclaw_reborn_composition/src/available_extensions.rs | Expose Slack manifest TOML string for serve-time projection. |
| crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs | Add wire-path tests for parsing host_ingress, host_api’s fail-closed floor, and ingress credential coherence. |
| crates/ironclaw_product_adapter_registry/src/lib.rs | Add HostIngressRoute, manifest parsing for host_ingress, and validation for ingress credential coherence + duplicate route ids (within a section). |
| crates/ironclaw_product_adapter_registry/CLAUDE.md | Document the new manifest host_ingress contract and its validation responsibilities. |
| crates/ironclaw_first_party_extensions/assets/slack/manifest.toml | Declare Slack host-ingress routes and add slack_signing_secret as a required credential used to verify inbound routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [[product_adapter.inbound.host_ingress]] | ||
| credential_handles = ["slack_signing_secret"] | ||
|
|
| pub(crate) fn descriptor_for_route( | ||
| descriptors: &[IngressRouteDescriptor], | ||
| route_id: &str, | ||
| ) -> Result<IngressRouteDescriptor, HostIngressProjectionError> { | ||
| descriptors | ||
| .iter() | ||
| .find(|descriptor| descriptor.route_id().as_str() == route_id) | ||
| .cloned() | ||
| .ok_or_else(|| HostIngressProjectionError::RouteNotDeclared { | ||
| route_id: route_id.to_string(), | ||
| }) | ||
| } |
Reborn integration-tier coverageLine coverage (Reborn crates): 26.09% — 45017 / 172537 lines Per-crate breakdown (62 crates, lowest-covered first)
This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout. Exemptions (0 file(s) excluded from the accounting above)No exemptions configured. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/ironclaw_reborn_composition/src/host_ingress.rs (1)
42-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSecond manifest-ingestion path for serve descriptors persists.
This still reparses the bundled manifest independently of
available_extensions::bundled_extension_package, rather than reusing the already-projectedProductAdapterHostApiSections from bundled installation. The doc comment (lines 10-14) explains the parsing context now matches (default catalog/contracts), but the manifest is still parsed twice at different times/call sites, so a future context divergence between the two paths could silently reintroduce drift.A prior reviewer suggested reusing the same parsed bundled package path, or adding a registry API that returns already-projected sections once.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/host_ingress.rs` around lines 42 - 62, The bundled host ingress path still reparses the manifest instead of reusing the already-projected bundled package data. Update bundled_host_ingress_descriptors to consume the projected ProductAdapterHostApiSection values from the same bundled-installation flow used by available_extensions::bundled_extension_package, or add a registry helper that returns those projected sections directly. Keep the existing ingress flattening logic, but avoid calling ExtensionManifestRecord::from_toml_with_contracts and product_adapter_sections a second time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@crates/ironclaw_reborn_composition/src/host_ingress.rs`:
- Around line 42-62: The bundled host ingress path still reparses the manifest
instead of reusing the already-projected bundled package data. Update
bundled_host_ingress_descriptors to consume the projected
ProductAdapterHostApiSection values from the same bundled-installation flow used
by available_extensions::bundled_extension_package, or add a registry helper
that returns those projected sections directly. Keep the existing ingress
flattening logic, but avoid calling
ExtensionManifestRecord::from_toml_with_contracts and product_adapter_sections a
second time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d67af8af-d0ae-4109-a252-410069b5fd89
📒 Files selected for processing (8)
crates/ironclaw_first_party_extensions/assets/slack/manifest.tomlcrates/ironclaw_product_adapter_registry/CLAUDE.mdcrates/ironclaw_product_adapter_registry/src/lib.rscrates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rscrates/ironclaw_reborn_composition/src/available_extensions.rscrates/ironclaw_reborn_composition/src/host_ingress.rscrates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/slack_serve.rs
Summary
Makes the manifest-driven ingress contract from #5625 load-bearing: Slack's two inbound routes (
slack.events,slack.commands) are now declared as data in the bundled extension manifest and projected into descriptors at serve time, instead of being hand-written Rust policy literals. This is the real example of usage the mechanism needed — and it answers the "unused API" question on #5625 by givinghost_ingress()a production consumer that deletes per-channel Rust.Stacked on #5625 (needs
ProductAdapterHostApiSection::host_ingress()). Review/merge that first; this PR's base is the #5625 branch so the diff here is only the migration.What moved (and what didn't)
assets/slack/manifest.tomlnow declares[[product_adapter.inbound.host_ingress]]for both routes, each namingslack_bot_tokenas its verifying credential (fail-closed credential coherence, enforced by the registry).slack_serve.rs:slack_events_route_descriptors/slack_commands_route_descriptorsnow project their descriptor from the bundled manifest via a new generic helper (composition::host_ingress::bundled_host_ingress_descriptor); the two hardcodedslack_events_policy()/slack_commands_policy()literals (~40 lines each) are deleted.The descriptor is validated by
ironclaw_host_apion deserialize (dotted route id, absolute path, and the fail-closed floor that apublic_webhooklistener MUST requirewebhook_signature) and byironclaw_product_adapter_registryfor ingress credential coherence — so a manifest cannot declare a weaker route than the Rust literal did.Why this is safe
slack_{events,commands}_route_descriptor_matches_manifest_projectionassert the manifest-projected descriptor is byte-for-byte identical to the pre-migration Rust literal (1 MiB / 12k·60s for events, 16 KiB / 6k·60s for commands).cargo fmt+cargo clippy(slack-v2-host-beta,webui-v2-beta,libsql,--tests) clean. No public API signature changed; the manifest digest recomputes consistently (no test pins it).Net effect
Deletes the per-surface Rust policy literals for both Slack routes; adding or adjusting a channel's inbound route becomes editing a manifest, not writing serve-layer Rust. Next step (separate PR): a generic serve loop that mounts every enabled extension's declared ingress routes, so new channels need no bespoke mount code at all.
🤖 Generated with Claude Code