fix(slack): filter delivered gate routes by raw gate string (auth vs approval)

[henrypark133]

Summary

FIX 1 — replace fn(&GateRef)->bool with fn(&str)->bool for gate_kind_filter

The load_delivered_routes_for_envelope and select_delivered_gate_route functions previously took gate_kind_filter: Option<fn(&GateRef) -> bool>. This had two bugs:

• Per-route allocation: every candidate route's stored gate string was re-wrapped into a GateRef via GateRef::new(r.gate_ref.clone()) just to run a prefix check.
• Silent drop: any route whose stored string failed GateRef::new() validation was silently discarded before the predicate ran, meaning the filter never executed on such routes (they were invisible to it).

Changed to Option<fn(&str) -> bool> so the predicate receives the raw stored string directly. The GateRef::new wrap in the filter body is gone.

Updated predicates:

• is_approval_gate_ref (approval_interaction/gate_ref.rs) — now takes &str
• is_auth_gate_ref (auth_interaction/gate_ref.rs) — now takes &str

Updated callers that passed &GateRef:

• reborn_services.rs from_gate_shape → passes .as_str()
• slack_delivery.rs (two sites) → passes .as_str()
• projection/turn_events.rs → passes .as_str()

FIX 2 — symmetric bare-auth-deny integration test

New test: bare_auth_deny_with_stale_approval_route_selects_auth_route_not_approval

Scenario: a stale APPROVAL gate route is stored in the delivered-route store under the same conversation fingerprint as an incoming bare auth-deny. Asserts:

• The auth-kind filter (is_auth_gate_ref) drops the approval route → Miss
• The stale approval route's run_id is never forwarded to the auth service as a run_id_hint
• The auth service receives the correct auth gate_ref, not the stale approval one

This is the symmetric counterpart of scoped_approval_two_live_routes_same_conversation_rejects_ambiguous (which verifies the opposite direction). Under the old fn(&GateRef) shape, a stale approval route with an invalid stored string would have been silently dropped before the predicate ran — leaving the auth filter's correctness unverifiable for those routes.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed delivered-gate fallback routing to apply gate-kind filtering only for bare lookups, ensuring stale approval/auth routes don’t create ambiguity or forward an incorrect resolution target.

• Tests

  • Added contract and regression coverage for bare-approve and auth-deny delivered-route filtering, plus validation behavior for invalid stored gate references.
  • Updated delivered-gate E2E fixtures and assertions to use the new canonical approval gate reference values.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 706a7625-2042-452f-8315-7637f955b41f

📥 Commits

Reviewing files that changed from the base of the PR and between 4aa9214 and afd32a4.

📒 Files selected for processing (2)

• crates/ironclaw_product_workflow/src/workflow.rs
• crates/ironclaw_product_workflow/tests/product_workflow_contract.rs

---

📝 Walkthrough

Walkthrough

Gate reference predicates (is_approval_gate_ref, is_auth_gate_ref) change from accepting &GateRef to &str, removing type indirection. Delivered-gate route resolution now filters routes by gate kind during bare lookups to prevent approval and auth routes stored under the same conversation fingerprint from contaminating ambiguity detection. Call sites throughout the stack updated. Regression tests verify kind-based isolation prevents stale routes from infecting fallback behavior.

Changes

Gate-kind filtering for delivered route resolution

Layer / File(s)	Summary
Gate reference predicates refactored to &str crates/ironclaw_product_workflow/src/approval_interaction/gate_ref.rs, crates/ironclaw_product_workflow/src/auth_interaction/gate_ref.rs	is_approval_gate_ref and is_auth_gate_ref accept &str instead of &GateRef, performing direct string prefix/equality checks against gate constants. Unit tests pass string literals and validate both matching and non-matching gate shapes.
Gate classification site update crates/ironclaw_product_workflow/src/reborn_services.rs	GateResolutionRoute::from_gate_shape calls is_approval_gate_ref and is_auth_gate_ref with gate_ref.as_str() instead of &GateRef.
Delivered gate filtering by gate kind crates/ironclaw_product_workflow/src/workflow.rs	load_delivered_routes_for_envelope accepts optional gate_kind_filter: fn(&str) -> bool predicate; applies filter only for bare lookups to exclude routes of opposite kind, leaving exact-ref matches unaffected. select_delivered_gate_route threads predicate. Approval and auth fallback paths each supply their respective gate-kind filter. Imports updated to include is_auth_gate_ref.
Composition layer call site updates crates/ironclaw_reborn_composition/src/projection/turn_events.rs, crates/ironclaw_reborn_composition/src/slack_delivery.rs	approval_gate_prompt, slack_approval_gate_prompt_view, and triggered_notification_for_state pass gate_ref.as_str() to is_approval_gate_ref.
Contract and E2E regression tests crates/ironclaw_product_workflow/tests/product_workflow_contract.rs, crates/ironclaw_reborn_composition/src/slack_serve/e2e_tests.rs	Contract tests add BindingRequiredThenSucceedingService test double. bare_auth_deny_with_stale_approval_route_selects_auth_route_not_approval verifies auth-kind filter excludes stale approval routes from fallback. exact_named_generic_approval_gate_is_forwarded_not_dropped_by_kind_filter verifies generic (non-auth-prefixed) approval gates still forward through fallback. bare_approve_with_invalid_stored_approval_route_rejects_invalid_gate_ref and bare_auth_deny_with_invalid_stored_auth_route_rejects_invalid_gate_ref verify oversized gate-ref strings matching kind prefix surface InvalidRequest (400) without calling interaction services. E2E tests: approval gate constants updated to gate:approval-...-000X; new test bare_approve_with_one_approval_and_one_stale_auth_gate_resolves_approval verifies approval and auth route isolation; DM fixture wiring and JSON updated for explicit approval gate refs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🚪 Two gates, one conversation—
Now filtering keeps them apart,
Approval stays with approval,
Auth with auth, gates in their lanes,
Stale routes filtered clean away ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title follows Conventional Commits style with type(scope) and clearly summarizes the main change: filtering delivered gate routes by raw gate strings.
Description check	✅ Passed	Description provides detailed summary of both fixes, change rationale, and new test coverage. Validation section incomplete but summary, linked issue, and technical justification are present.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[gemini-code-assist]

Code Review

This pull request refactors the gate reference checking functions is_approval_gate_ref and is_auth_gate_ref to accept &str instead of &GateRef. It introduces a gate_kind_filter parameter to route loading and selection functions in the workflow module. This filter ensures that when resolving delivered routes, approval routes are separated from auth routes, preventing lingering gates of a different kind in the same conversation bucket from causing spurious ambiguity errors. Comprehensive unit and integration tests have been added to verify this behavior. There are no review comments, and I have no additional feedback to provide.

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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e85083f25e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[henrypark133]

Code Review (multi-agent)

Intent: Fix Slack delivered-route filtering so auth and approval gates are matched by raw gate string, with symmetric regression coverage.

Stats: 5 findings (from 5 raw, 5 after dedup) across 2 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0

Tests

1. Medium Untested invalid stored approval gate route (crates/ironclaw_product_workflow/src/workflow.rs:659-666, confidence 75) — anchor: crates/ironclaw_product_workflow/src/workflow.rs:659
   The new raw-string gate-kind filter can now select a stored route whose string starts with gate:approval- but still fails GateRef validation, causing the delivered approval fallback to return InvalidGateRef.
2. Medium Untested invalid stored auth gate route (crates/ironclaw_product_workflow/src/workflow.rs:736-743, confidence 75) — anchor: crates/ironclaw_product_workflow/src/workflow.rs:736
   The auth delivered-route fallback now filters raw stored strings before reconstructing GateRef, so malformed auth-prefixed stored routes can now exercise the InvalidGateRef path.

Conventions

1. Medium Raw invalid stored gate refs are still untested (crates/ironclaw_product_workflow/src/workflow.rs:473-476, confidence 75) — anchor: .claude/rules/testing.md:24
   The added tests cover cross-kind contamination with valid stored refs, but not the stated invalid-stored-string regression.

Local Patterns

1. Low Test comment claims coverage the fixture does not exercise (crates/ironclaw_product_workflow/tests/product_workflow_contract.rs:2062-2067, confidence 75) — anchor: crates/ironclaw_product_workflow/tests/product_workflow_contract.rs:2062
   The fixture uses a valid approval gate while the comment says it covers invalid stored strings being silently dropped before the predicate ran.

Maintainability

1. Low Gate-kind filter does not need a None mode (crates/ironclaw_product_workflow/src/workflow.rs:477-477, confidence 75) — anchor: crates/ironclaw_product_workflow/src/workflow.rs:477
   The selector is private and both approval/auth callers always pass a predicate, so the optional unfiltered branch appears unused.

[coderabbitai]

♻️ Duplicate comments (3)

crates/ironclaw_product_workflow/src/workflow.rs (3)

481-481: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Optional gate-kind filter has no None-mode caller.

Both resolve_via_delivered_approval_route (line 653) and resolve_via_delivered_auth_route (line 730) always pass Some(...). The None branch at lines 546-553 is unreachable. Making the parameter mandatory (fn(&str) -> bool) eliminates the dead branch and clarifies that delivered-route fallback is always typed by interaction kind.

🤖 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_product_workflow/src/workflow.rs` at line 481, The
gate_kind_filter parameter is defined as Optional but all callers always provide
a value, making the None-handling branch at lines 546-553 unreachable dead code.
Change the gate_kind_filter parameter from Option<fn(&str) -> bool> to fn(&str)
-> bool (removing the Option wrapper) to make it mandatory, then remove the
unreachable None branch in the code that checks for None. The two callers at
resolve_via_delivered_approval_route and resolve_via_delivered_auth_route
already pass Some(...) so they will not need changes.

---

742-746: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Untested InvalidGateRef path for malformed stored auth route.

The auth delivered-route fallback filters raw stored strings before reconstructing GateRef, so a stored string like an overlong gate:auth-* route can pass is_auth_gate_ref and then hit the InvalidGateRef error path. Existing auth coverage uses valid stale approval routes and never exercises this newly-reachable malformed stored-route path. .claude/rules/testing.md requires regression tests for bug fixes.

🤖 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_product_workflow/src/workflow.rs` around lines 742 - 746, The
InvalidGateRef error path in the GateRef::new() call at line 742-746 is untested
because existing auth coverage only uses valid stale approval routes. Add a
regression test that exercises this path by testing with a malformed stored auth
route (such as an overlong gate:auth-* route) that passes the is_auth_gate_ref
validation filter but then fails when attempting to construct the GateRef
object. This test should verify that the AuthInteractionRejected error with
InvalidGateRef kind is properly returned for such malformed stored routes.

---

665-669: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Untested InvalidGateRef path for malformed stored approval route.

The raw-string filter can now select a stored route whose gate_ref string passes is_approval_gate_ref but fails GateRef::new() validation (e.g., overlong gate:approval-* exceeding validation limits). The delivered approval fallback would return InvalidGateRef, but no test seeds a malformed stored approval route exercising this error branch. .claude/rules/testing.md requires regression coverage for bug fixes.

🤖 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_product_workflow/src/workflow.rs` around lines 665 - 669, The
InvalidGateRef error path in the GateRef::new() call is untested when handling a
malformed stored approval route. Add a regression test case that seeds a stored
approval route with a gate_ref string that passes is_approval_gate_ref
validation but exceeds GateRef::new() validation limits (e.g., an overlong
gate:approval-* string). The test should verify that when the approval
interaction path is exercised with this malformed route, it returns a
ProductWorkflowError::ApprovalInteractionRejected error with
ApprovalInteractionRejectionKind::InvalidGateRef, ensuring the error handling in
the GateRef::new() map_err block is properly covered.

🤖 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_product_workflow/src/workflow.rs`:
- Line 481: The gate_kind_filter parameter is defined as Optional but all
callers always provide a value, making the None-handling branch at lines 546-553
unreachable dead code. Change the gate_kind_filter parameter from
Option<fn(&str) -> bool> to fn(&str) -> bool (removing the Option wrapper) to
make it mandatory, then remove the unreachable None branch in the code that
checks for None. The two callers at resolve_via_delivered_approval_route and
resolve_via_delivered_auth_route already pass Some(...) so they will not need
changes.
- Around line 742-746: The InvalidGateRef error path in the GateRef::new() call
at line 742-746 is untested because existing auth coverage only uses valid stale
approval routes. Add a regression test that exercises this path by testing with
a malformed stored auth route (such as an overlong gate:auth-* route) that
passes the is_auth_gate_ref validation filter but then fails when attempting to
construct the GateRef object. This test should verify that the
AuthInteractionRejected error with InvalidGateRef kind is properly returned for
such malformed stored routes.
- Around line 665-669: The InvalidGateRef error path in the GateRef::new() call
is untested when handling a malformed stored approval route. Add a regression
test case that seeds a stored approval route with a gate_ref string that passes
is_approval_gate_ref validation but exceeds GateRef::new() validation limits
(e.g., an overlong gate:approval-* string). The test should verify that when the
approval interaction path is exercised with this malformed route, it returns a
ProductWorkflowError::ApprovalInteractionRejected error with
ApprovalInteractionRejectionKind::InvalidGateRef, ensuring the error handling in
the GateRef::new() map_err block is properly covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 85bf8463-46e9-427a-9204-eb59158130a6

📥 Commits

Reviewing files that changed from the base of the PR and between e85083f and 4aa9214.

📒 Files selected for processing (2)

• crates/ironclaw_product_workflow/src/workflow.rs
• crates/ironclaw_product_workflow/tests/product_workflow_contract.rs