PR 18.5a: type-seal trusted trigger ingress

[henrypark133]

Summary

Implements PR 18.5a for the trigger loop delivery resolution plan: type-seal the host-trusted trigger ingress path and remove the old public authority-token facade.

• seals TrustedTriggerSubmitRequest so only the trigger worker can mint trusted trigger submissions from durable trigger state
• moves trigger-to-conversation trusted inbound mapping into ironclaw_conversations, keeping TrustedInboundTurnRequest private
• exposes a narrow trusted_trigger_fire_submitter(...) -> Arc<dyn TrustedTriggerFireSubmitter> factory instead of a public concrete trusted submitter
• keeps Reborn composition as wiring only for materializer, authorizer, submitter, and poller services
• adds architecture guardrails preventing product, adapter, capability, WebUI, and host-runtime paths from constructing or submitting host-trusted trigger ingress
• updates Reborn contracts for the trusted trigger ownership boundary

Validation

• cargo fmt
• cargo test -p ironclaw_triggers worker --lib
• cargo test -p ironclaw_triggers worker --lib -- --nocapture
• cargo test -p ironclaw_conversations inbound::tests --lib
• cargo test -p ironclaw_reborn_composition trigger_poller_trusted_submit::tests -- --nocapture
• cargo test -p ironclaw_reborn_composition materializer_rejects_foreign_tenant_fire_before_binding_or_prompt_write -- --nocapture
• cargo test -p ironclaw_architecture --test reborn_dependency_boundaries -- --nocapture
• git diff --check
• $code-review with gpt-5.4-mini reviewers; straightforward test comments fixed, design/hygiene comment addressed by factory + guardrails

[gemini-code-assist]

Code Review

This pull request refactors the trusted trigger ingress boundary by retiring the ironclaw_trusted_ingress crate and moving the trusted trigger submission logic directly into ironclaw_conversations. It introduces the ConversationTrustedTriggerSubmitter implementing TrustedTriggerFireSubmitter, seals TrustedTriggerSubmitRequest fields, and updates architecture boundary tests to prevent untrusted ingress paths from submitting host-trusted inbound requests. Feedback on the changes suggests optimizing performance by avoiding unnecessary heap allocations when constructing TrustedInboundTurnRequest and filtering non-blocking warnings, as well as improving maintainability by using an exhaustive match statement instead of a wildcard arm when mapping inbound turn errors.

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.

[henrypark133]

Code Review (8 reviewers, sonnet) — head 766d314

Intent: PR 18.5a: type-seal host-trusted trigger ingress path. Seal TrustedTriggerSubmitRequest so only trigger worker can mint trusted submissions from durable state. Move trigger→conversation trusted inbound mapping into ironclaw_conversations. Expose narrow trusted_trigger_fire_submitter(...) -> Arc<dyn TrustedTriggerSubmitter>. Remove old public authority-token facade.

Stats: 9 findings (from ~15 raw, 9 after dedup) across 2 files (mainly inbound.rs). Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Reviewers failed: none. Body-only: 0.

Event: would-be REQUEST_CHANGES (1 High ≥75). PR is draft + author's own PR → posting as COMMENT. Treat the High finding as a blocker before promoting from draft.

Findings

Bugs (High — must fix)

1. High submit_trusted_trigger_outcome returns Replayed for freshly-submitted turn when idempotency==Duplicate but no prior turn_submission existed (crates/ironclaw_conversations/src/inbound.rs:391-411, confidence 82) — anchor: :402
   • submit_or_replay only returns early with existing turn_submission when Duplicate AND inbound_message_turn_submission == Some. When Duplicate but no prior record, falls through and submits fresh turn — response carries idempotency=Duplicate + brand-new turn_submission. submit_trusted_trigger_outcome sees Duplicate → returns Replayed{original_run_id: NEW_RUN_ID}. Caller calls mark_fire_replayed instead of mark_fire_accepted → replayed semantics on fresh submission → can stall subsequent poller decisions gating on active_run_ref.
   • Fix: return Replayed only when response came from stored prior submission, not purely on idempotency flag. Thread an explicit "returned-existing-record" flag from submit_or_replay.
   • Also flagged by: tests/Medium — None turn_submission branch untested

Conventions / Architecture (Medium)

2. Medium validate_trigger_prompt + classify_inbound_error duplicated byte-for-byte across ironclaw_conversations + ironclaw_reborn_composition (crates/ironclaw_conversations/src/inbound.rs:452-484, confidence 92) — anchor: crates/ironclaw_reborn_composition/src/trigger_poller_trusted_submit.rs:170

   • 4 reviewers flagged (conventions/maintainability/local-patterns x2). Same Sanitizer, same Severity::High threshold, same log/error messages. Safety/policy change must land in both files or silently diverge. Per .claude/rules/architecture.md §4 ('duplicate dispatch pipelines'). PR's stated goal was to consolidate trusted trigger logic in ironclaw_conversations — but most safety-critical helper still lives in two places.
   • Fix: extract validate_trigger_prompt to ironclaw_safety (or ironclaw_triggers); same for classify_inbound_error.

3. Medium Public trusted_trigger_fire_submitter re-export at crate root removes compile-time authority enforcement (crates/ironclaw_conversations/src/lib.rs:30-34, confidence 78) — anchor: :32

   • Old design: &HostTrustedTriggerIngress zero-sized authority token (feature-gated constructor) → compile-time boundary. New design: pub fn re-exported from crate root, relying solely on architecture tests (string-matching untrusted_src_roots allowlist). New workspace crate importing ironclaw_conversations can call factory if missing from allowlist.
   • Fix: cargo feature trusted-submitter only composition-root crates enable; OR enumerate Cargo.toml deps against allowlist (rather than separate src-root list); OR keep factory pub(crate).
   • Also flagged by: security/Medium — trust-boundary-weakening

4. Medium classify_trusted_trigger_inbound_error never produces RetryableFailed/PermanentFailed Ok variants expected by caller (crates/ironclaw_conversations/src/inbound.rs:414-450, confidence 75) — anchor: :414

   • Always returns Err(TriggerError::Backend/InvalidMaterialization). Caller due_fire.rs:176-193 has match arms for RetryableFailed/PermanentFailed — UNREACHABLE. End behavior likely correct (Err arm at due_fire.rs:194 also classifies), but trait contract with 4 Ok variants partially unfulfilled.
   • Fix: either return Ok(RetryableFailed/PermanentFailed) to align trait contract, or document TrustedTriggerSubmitter trait that Err is preferred signal and Ok-failure variants are deprecated.
   • Also flagged by: tests/Medium — classifier has no unit tests

5. Medium ironclaw_conversations now owns trigger-protocol normalization that crate CLAUDE.md says belongs in adapters (crates/ironclaw_conversations/src/inbound.rs:221-328, confidence 72) — anchor: crates/ironclaw_conversations/CLAUDE.md:1

   • ConversationTrustedTriggerSubmitter + trusted_inbound_request_from_trigger embed hard-coded AdapterKind::new("trigger"), AdapterInstallationId::new("reborn-trigger-poller"), TriggerFire field extraction. Per crate CLAUDE.md: "Own adapter-safe conversation binding... only. Do not parse concrete Slack/Telegram/Web/CLI payloads. Product adapters normalize protocol payloads before calling these services." TriggerFire is a protocol payload.
   • Fix: keep ironclaw_conversations protocol-neutral. Accept pre-built InboundTurnRequest/TrustedInboundTurnRequest from caller. Move trigger-to-conversation mapping back to ironclaw_triggers or ironclaw_reborn_composition.
   • Also flagged by: maintainability/Low — field mapping duplicated

Security (Medium → Low)

6. Medium validate_trigger_prompt scans fire().prompt but content_ref target (actual conversation content) is unchecked at submission (crates/ironclaw_conversations/src/inbound.rs:276-291, confidence 65) — anchor: :281

   • Type system doesn't enforce 'content_ref was minted from this exact scanned prompt'. Test shim or alternate materializer with mismatched prompt/content_ref bypasses detection at submission.
   • Fix: document materializer contract clearly OR carry sealed token from materializer→submitter proving consistency.

7. Low tracing::warn-equivalent internal identifiers (actor_id/thread_id) may leak via TriggerError reason strings to operational logs (crates/ironclaw_conversations/src/inbound.rs:414-450, confidence 55) — anchor: :446

   • Catch-all arm format!("trusted trigger inbound request invalid: {error}"). InboundTurnError::AccessDenied{actor_id, thread_id} Display includes identifiers. Reason propagated to logs.
   • Fix: opaque message + tracing::debug! for internal detail.

Performance / Tests

8. Low Trigger prompt safety scan runs twice per fire — materializer + submitter both call validate_trigger_prompt (crates/ironclaw_conversations/src/inbound.rs:281, confidence 85) — anchor: :281

   • Pure function of input → second scan cannot produce different result. Hot path waste.
   • Fix: remove submitter scan (materializer contract guarantees pre-scan); OR document intentional defense-in-depth.

9. Low validate_trigger_prompt + trusted_inbound_request_from_trigger have no direct tests in inbound.rs (crates/ironclaw_conversations/src/inbound.rs:452-484, confidence 75) — anchor: :452

   • Only integration tests in trigger_poller_trusted_submit drive indirectly. New fallible new() error paths (invalid creator_user_id, etc.) uncovered.

Other categories

• pattern-refactor ✅ empty — Factory pattern correctly applied; 2-arm BindingResolutionDispatch is correct inline shape

Mergeability

⚠️ Blocked on #1 (High bugs). Replayed-vs-Accepted misclassification on idempotency-Duplicate-without-prior-submission can leak into production poller behavior. Must close before promoting from draft.

Other priorities before merge:

• #2 (verbatim duplicate of safety helper across crate boundary — PR's whole point was consolidation)
• #3 (pub factory re-export weakens type-seal that PR title promised)
• #5 (layer-misplacement — adapter normalization in ironclaw_conversations violates its own CLAUDE.md)

#4, #6-#9 = follow-up.

[henrypark133]

Code Review Round 2 (8 reviewers, sonnet) — head e026031

Intent: R2 follow-up — address R1 review feedback on PR 18.5a type-seal trusted trigger ingress.

R1 status — substantial progress ✅:

• ✅ R1 High (82) bugs Replayed bug — RESOLVED. replayed_turn_submission: bool added to InboundTurnResponse; both submit_or_replay branches set it correctly; full test coverage at submit_trusted_trigger_outcome_preserves_received_at_for_accepted_and_replayed_fires.
• ⚠️ R1 Med (92) conv duplicate validate+classify — PARTIAL. validate_trigger_prompt shared. Two classify_*_inbound_error functions remain (see #2 below + new bug #1).
• ✅ R1 Med (78) conv public re-export — RESOLVED. ConversationTrustedTriggerSubmitter pub(crate); only factory trusted_trigger_fire_submitter exported.
• ✅ R1 Med (75) bugs dead Ok variants — RESOLVED via restructure.
• ✅ R1 Med (72) conv layer-misplacement — RESOLVED per commit 11ec73a.
• ✅ R1 Med (65) sec content_ref scan gap — RESOLVED via documentation.
• ✅ R1 Low (85) perf double-scan — DOCUMENTED as intentional defense-in-depth.
• ✅ R1 Low (75) tests untested helpers — RESOLVED.
• ⚠️ R1 Low (55) sec actor_id/thread_id leak — PARTIAL. opaque_trusted_trigger_inbound_rejection added for permanent path. Retryable branches still leak (see #10).

Stats: 11 findings (from ~16 raw, 11 after dedup). Reviewers: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Failed: none. Body-only: 2. performance/conventions/pattern-refactor clean.

Event: would-be REQUEST_CHANGES (1 High ≥75). PR is draft + author's own PR → posting as COMMENT. Treat the High finding as blocker before promoting from draft.

Findings

Bugs (High — direct consequence of partial R1 #2 fix)

1. High DurableState error classified as permanent — discards trigger fires during transient DB outages (crates/ironclaw_conversations/src/inbound.rs:465-476, confidence 80) — anchor: :472
   • catch-all routes InboundTurnError::DurableState to opaque_trusted_trigger_inbound_rejection → TriggerError::InvalidMaterialization (permanent). Sibling classify_inbound_error at trigger_poller_trusted_submit.rs:269-272 correctly maps DurableState to Backend (retryable). DB outage during submission phase → silent permanent fire loss instead of retry. Direct consequence of unresolved R1 #2 (two divergent classifiers).
   • Fix: add explicit arm before catch-all returning Backend with durable state unavailable reason.

Maintainability (Medium — root cause of #1)

2. Medium R1 #2 PARTIAL — two diverging classify_*_inbound_error functions remain (crates/ironclaw_conversations/src/inbound.rs:428-477, confidence 75) — anchor: :428
   • inbound.rs:428 (exhaustive) + trigger_poller_trusted_submit.rs:240 (catch-all _ arm). Same InboundTurnError → TriggerError mapping in two places. #1 DurableState bug is direct consequence. Future TurnError variants silently diverge.
   • Fix: consolidate to impl From<InboundTurnError> for TriggerError in ironclaw_triggers or ironclaw_conversations. Delete composition copy.
   • Also flagged by: local-patterns/Nit naming divergence

Tests (Medium — would have caught #1)

3. Medium classify_trusted_trigger_inbound_error has no direct unit tests in conversations crate (crates/ironclaw_conversations/src/inbound.rs:428-477, confidence 75) — anchor: :428

   • 10 InboundTurnError variants; only exercised indirectly via composition crate's parallel classify_inbound_error. Direct unit test for DurableState would have caught #1.
   • Fix: add classify_trusted_trigger_inbound_error_durable_state_is_retryable_backend + arm-by-arm tests.

4. Medium trusted_inbound_request_from_trigger error paths untested at caller level (crates/ironclaw_conversations/src/inbound.rs:305-338, confidence 75) — anchor: :305

   • Helper calls 6 *::new() Result-returning constructors mapped via ? to InboundTurnError. Caller-level test required by testing.md (wrapper between helper + side effect).

Security (Medium)

5. Medium validate_trigger_prompt keeps only LAST High-severity warning + missing audit counts (crates/ironclaw_conversations/src/inbound.rs:493-523, confidence 72) — anchor: :497
   • Loop overwrites blocked_warning each High hit. High warnings don't increment warning_count or update max_severity → audit log shows 0 warnings + no max_severity if ALL findings are High. Prompt still blocked, defense-in-depth audit trail lost on multi-pattern injection.
   • Fix: count+max_severity-update ALL warnings; break on first High (early-exit safer).

Maintainability (Low)

6. Low Reborn-specific tracing-target check hardcoded into shared boundary scanner (crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs:2648-2651, confidence 72) — anchor: :2649

   • collect_forbidden_uses + collect_forbidden_reborn_auth_file_uses contain if rule.pattern == "ironclaw::" && is_reborn_tracing_target_line(line) — couples Reborn-specific exemption into general-purpose scanner.
   • Fix: add exempt: Option<fn(&str) -> bool> field to ForbiddenUse struct.

7. Low replayed_turn_submission: bool leaks trigger-delivery concern into general InboundTurnResponse (crates/ironclaw_conversations/src/types.rs:193-200, confidence 65) — anchor: :199

   • pub+serialized field used only by trigger submission path. Part of cross-crate contract for trigger-specific concern.
   • Fix: return distinction from submit_or_replay via local SubmitOrReplayOutcome struct; don't expose on shared InboundTurnResponse.

Local patterns

8. Low validate_trigger_prompt returns TriggerError but lives in ironclaw_conversations (crates/ironclaw_conversations/src/inbound.rs:489-524, confidence 65) — anchor: CLAUDE.md:1
   • Function returns TriggerError (from ironclaw_triggers). conversations CLAUDE.md restricts crate to "adapter-safe conversation binding". Reader would not look here for trigger prompt validation.
   • Fix: move to ironclaw_triggers (e.g. src/validation.rs adjacent to TriggerError).

Tests (Low)

9. Low dir-scanner detection self-test missing (crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs:2626-2664, confidence 65) — anchor: :2626
   • File-level scanner has detection self-test ✅. Dir-level scanner only has allowance test (skip tracing targets). No test that dir-scanner REPORTS violations.

Security (Low — R1 #9 partial)

10. Low R1 #9 PARTIAL — retryable Backend reasons still embed raw InboundTurnError display (crates/ironclaw_conversations/src/inbound.rs:428-456, confidence 58) — anchor: :430

    • Opaque rejection ✅ for permanent. Retryable arms still format!("...{error}") interpolating Display incl. active_run_id from ThreadBusy. Cross-tenant identifier exposure via logs.
    • Fix: apply same opaque wrapping to retryable branches.

11. Low Composition catch-all classify_inbound_error also embeds InboundTurnError display (crates/ironclaw_reborn_composition/src/trigger_poller_trusted_submit.rs:240-276, confidence 55) — anchor: :272 (body only)

    • Same leak in catch-all {error}. Includes raw actor_id/thread_id from AccessDenied/BindingConflict. Folds into #10 fix.

Other categories

• performance ✅ clean — bounded by MAX_TRIGGER_PROMPT_BYTES=32KiB + 1 fire/tick
• conventions ✅ clean (rolled into local-patterns + maintainability above)
• pattern-refactor ✅ empty

Mergeability

⚠️ Blocked on #1 (High bugs DurableState misclassification). Direct production data-loss risk on transient DB outage. Must close before promoting from draft.

Other priorities before merge:

• #2 (consolidate to single classifier — would prevent class of bugs like #1)
• #3-#4 (add tests that would have caught #1)
• #5 (validate audit-trail gap on multi-High injection)
• #10 (R1 #9 retryable leak — finishes deferred R1 work)

#6-#9, #11 = follow-up.

[henrypark133]

Code Review Round 3 (8 reviewers, sonnet) — head c1056bd

Intent: R3 follow-up — address R2 review feedback. Extract validate_trigger_prompt to ironclaw_safety. Move trusted_submit helpers to ironclaw_triggers. Retire old ironclaw_trusted_ingress crate. Redact classifier logs.

R2 status — substantial progress ✅:

• ✅ R2 High (80) DurableState — RESOLVED. Now retryable_trusted_trigger_backend_error in both classifiers.
• ⚠️ R2 Med (75) duplicate classifiers — PARTIAL. validate_trigger_prompt extracted ✅, BUT classifiers STILL duplicated (see #1).
• ✅ R2 Med (72) validate keeps LAST warning — RESOLVED. Now FIRST via blocked_warning.is_none() guard.
• ✅ R2 Low (72) Reborn tracing-target hardcoded — RESOLVED. exempt: Option<fn(&str) -> bool> field added to ForbiddenUse.
• ✅ R2 Low (65) validate_trigger_prompt location — RESOLVED. Extracted to ironclaw_safety/src/prompt_validation.rs.
• ✅ R2 Low (65) dir-scanner detection test — RESOLVED (collect_forbidden_uses_detects_violation added).
• ✅ R2 Low (58) retryable Backend reason leak — RESOLVED. _error: &InboundTurnError unused, static strings only.
• ✅ R2 Low (55) composition catch-all leak — RESOLVED. Same pattern.
• ⚠️ R2 Low (65) replayed_turn_submission field leak — STILL OPEN (see #7).
• ⚠️ R2 Med (75) tests classify untested — PARTIAL coverage (see #2/#3).
• ⚠️ R2 Med (75) tests trusted_inbound_request_from_trigger untested — STILL OPEN (see #4).

✅ Old ironclaw_trusted_ingress crate retired — clean removal of AGENTS.md / Cargo.toml / lib.rs.

Stats: 10 findings (from ~13 raw, 10 after dedup) across 5 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Reviewers failed: none. Body-only: 5. security/conventions/pattern-refactor clean.

Event: COMMENT — no Critical/High ≥75. Largest = Med 85.

Findings

Maintainability (Medium — R2 #2 PARTIAL)

1. Medium R2 #2 PARTIAL — InboundTurnError→TriggerError classifier still written twice (crates/ironclaw_conversations/src/inbound.rs:441-484, confidence 85) — anchor: :441
   • validate_trigger_prompt extracted ✅, but classify_trusted_trigger_inbound_error (inbound.rs:441) + classify_materializer_inbound_error (trigger_poller_trusted_submit.rs:251) STILL structurally identical: same exhaustive match arms, same Backend/InvalidMaterialization mapping. Any new InboundTurnError variant requires changes in both. Architecture test enforces privacy but not single source of truth.
   • Fix: extract pub(crate) fn classify_inbound_turn_error_for_trigger into ironclaw_triggers::trusted_submit. Both callers invoke shared fn.
   • Also flagged by: local-patterns/Low (helper functions take unused _error param breaking sibling consistency with composition variants)

Tests (Medium — class of bugs prevented by coverage)

2. Medium classify_trusted_trigger_inbound_error missing AdmissionRejected branches + remaining TurnError + non-submission arms (crates/ironclaw_conversations/src/inbound.rs:441-484, confidence 85) — anchor: :441

   • Current tests cover only ThreadBusy/Conflict/Unauthorized/DurableState/InvalidExternalRef. UNTESTED: AdmissionRejected(TenantLimit/Unavailable/ProfileRejected/Policy), Unavailable, CapacityExceeded, ScopeNotFound, InvalidRequest, InvalidTransition, LeaseMismatch, BindingRequired, AccessDenied, BindingConflict, ThreadNotFound, StatePoisoned, InvalidCanonicalRef. Misclassification silently stalls or discards fires.

3. Medium classify_materializer_inbound_error missing non-submission + remaining TurnError arms (crates/ironclaw_reborn_composition/src/trigger_poller_trusted_submit.rs:251-293, confidence 80) — anchor: :251 (body only)

   • Materializer-side classifier coverage mirrors #2 gap. UNTESTED: BindingRequired, BindingConflict, ThreadNotFound, StatePoisoned, InvalidCanonicalRef, ScopeNotFound, InvalidRequest, InvalidTransition, LeaseMismatch. Same misclassification risk.

4. Medium R2 #4 STILL OPEN — trusted_inbound_request_from_trigger error paths untested (crates/ironclaw_conversations/src/inbound.rs:308-341, confidence 75) — anchor: :308

   • Helper calls 6 *::new() Result-returning constructors. None of constructor-failure paths tested. Malformed TriggerTrustedInboundBinding → silently classified InvalidMaterialization without path exercised.

Bugs (Medium — variant mismatch)

5. Medium submit_trusted_trigger_outcome conflates None+non-Accepted variants into misleading Backend error (crates/ironclaw_conversations/src/inbound.rs:411-417, confidence 72) — anchor: :412
   • let Some(SubmitTurnResponse::Accepted{...}) = &response.turn_submission else collapses two distinct cases into one error: (1) None, (2) Some(non-Accepted variant). Future non-Accepted variant → factually-wrong "no turn submission" reason + retryable classification. Correct behavior for non-Accepted may be permanent.
   • Fix: match None vs Some(_) catch-all separately.

Tests (Medium — defense-in-depth gate untested at submitter boundary)

6. Medium submit_trusted_trigger_fire safety-rejection path untested at submitter boundary (crates/ironclaw_conversations/src/inbound.rs:262-305, confidence 70) — anchor: :286
   • ConversationTrustedTriggerSubmitter::submit_trusted_trigger_fire defense-in-depth scan returns InvalidMaterialization on rejection. No test calls submit_trusted_trigger_fire directly with malicious prompt — only materializer-level scan tested. Per testing.md Test-Through-the-Caller, submitter's own gate needs coverage.

Performance

7. Medium Default TriggerActiveRunLookup::active_run_states impl is N sequential awaits, not batch (crates/ironclaw_triggers/src/worker/ports.rs:108-117, confidence 62) — anchor: crates/ironclaw_triggers/src/worker/active_cleanup.rs:70 (body only)
   • Default loops + awaits one at a time. active_cleanup batches requests specifically to amortize backend reads. Production composition wiring default (not override) → active-cleanup degrades from O(1 batch) to O(N sequential) per tick.
   • Fix: seal trait OR make active_run_states the required method with active_run_state convenience default.

Maintainability (Low)

8. Low R2 #7 STILL OPEN — replayed_turn_submission bool leaks trigger replay semantics into shared InboundTurnResponse (crates/ironclaw_conversations/src/types.rs:193-200, confidence 72) — anchor: :199 (body only)

   • InboundTurnResponse used by both untrusted + trusted paths. Untrusted callers never read field. CLAUDE.md scopes crate to 'adapter-safe conversation binding'.
   • Fix: return enum from handle_inbound_turn_with_trusted_scope encoding trusted outcome directly.

9. Low TriggerTrustedInboundBinding stores typed values as raw Strings, forcing re-validation downstream (crates/ironclaw_triggers/src/trusted_submit.rs:10-60, confidence 65) — anchor: :10 (body only)

   • 7 String fields. Downstream consumers call AdapterKind::new etc. re-validating values already typed in for_fire(). Compiler can't see re-validation can never fail → 4 .map_err calls + nested Ok(...) per call site.
   • Fix: store typed values OR expose into_inbound_turn_request() method on binding.

Tests (Low)

10. Low validate_trusted_trigger_prompt missing empty-string + multi-medium coverage (crates/ironclaw_safety/src/prompt_validation.rs:23-57, confidence 65) — anchor: :31 (body only)
    • Empty-prompt + multi-medium audit-log path uncovered. Audit branch conditioning not exercised.

Other categories

• security ✅ clean — R2 #10/#11 leaks resolved via _error-unused + static strings
• conventions ✅ clean — trusted_ingress crate retirement properly removes all artifacts
• pattern-refactor ✅ empty — Factory correctly applied; classifier duplication caught by maintainability

Mergeability

✅ Substantial R3 progress — 8 of 11 R2 findings RESOLVED. PR still draft.

Recommend before promoting from draft:

• Close #1 (consolidate classifiers — R2 #2 PARTIAL, prevents class of bugs from divergence)
• Close #2 + #3 (test the existing classifiers — would have caught misclassification regressions)
• Close #4 (R2 #4 carryover — caller-level error path test)
• Close #5 (one-line match restructure — guards future SubmitTurnResponse variants)

Optional follow-ups: #6 (submitter-level safety test), #7-#10 (architectural cleanups).

[henrypark133]

Code Review Round 4 (8 reviewers, sonnet) — head 8824613

R3 status — 4/10 RESOLVED ✅:

• ✅ R3 #1 Med (85) duplicate classifier → RESOLVED. New crates/ironclaw_conversations/src/trusted_trigger.rs centralizes; composition imports from there.
• ✅ R3 #2 Med (85) AdmissionRejected + remaining InboundTurnError arms → RESOLVED (b2f8ab8).
• ✅ R3 #3 Med (80) materializer non-submission arms → RESOLVED (same commit).
• ✅ R3 #5 Med (72) submit_outcome conflates None/non-Accepted → RESOLVED (8824613).
• ⚠️ R3 #4 Med (75) trusted_inbound_request_from_trigger error paths → STILL OPEN (see #2).
• ⚠️ R3 #6 Med (70) submitter-level safety-rejection → STILL OPEN (see #1).
• ⚪ R3 #7 Med (62) default active_run_states N+1 → NOTE-ONLY (R3 deferred).
• ⚠️ R3 #8 Low (72) replayed_turn_submission field leak → STILL OPEN. NOTE-ONLY per directive.
• ⚠️ R3 #9 Low (65) raw Strings → STILL OPEN (see #7).
• ⚠️ R3 #10 Low (65) prompt empty/multi-medium → STILL OPEN. NOTE-ONLY.

Stats: 7 findings (from ~9 raw, 7 after dedup). Reviewers: 8. Reviewers failed: none. Body-only: 3. security/performance/pattern-refactor clean.

Event: COMMENT — no Critical/High ≥75.

Findings

Tests (Medium)

1. Medium R3 #6 STILL OPEN — submit_trusted_trigger_fire safety-rejection untested at submitter boundary (crates/ironclaw_conversations/src/inbound.rs:287-308, conf 85) — anchor: :295
   • validate_trusted_trigger_prompt + trigger_prompt_safety_rejection map_err NOT tested through ConversationTrustedTriggerSubmitter. Per Test-Through-the-Caller.
2. Medium R3 #4 STILL OPEN — trusted_inbound_request_from_trigger field-validation error paths untested (crates/ironclaw_conversations/src/inbound.rs:309-357, conf 80) — anchor: :309
   • 5 *::new() constructors → InvalidExternalRef. None of rejection arms exercised.

Conventions (Medium)

3. Medium trusted_trigger module is pub — all sibling modules in same crate private (crates/ironclaw_conversations/src/lib.rs:22, conf 72) — anchor: :22 (body only)
   • pub mod trusted_trigger makes classify_inbound_error + TrustedTriggerInboundFailureKind accessible to every downstream crate. Architecture test conversation_trusted_trigger_classifier_stays_out_of_root_exports only guards root re-exports — NOT pub module. untrusted_ingress_paths_cannot_submit_host_trusted_inbound only checks 4 patterns — classify_inbound_error not listed. All other modules in crate are private + selective pub use.
   • Fix: mod trusted_trigger; pub use trusted_trigger::{TrustedTriggerInboundFailureKind, classify_inbound_error}; OR add architecture guard forbidding classify_inbound_error in untrusted ingress paths.
   • Also flagged by: local-patterns/Low (sibling-pattern)

Bugs (Low)

4. Low opaque_trusted_trigger_inbound_rejection logs same hardcoded message for both call sites (crates/ironclaw_conversations/src/inbound.rs:467-475, conf 85) — anchor: :471
   • SubmitRejected (reason='trusted trigger submit rejected') + InboundRequestRejected (reason='trusted trigger inbound request rejected') both emit identical debug log. SubmitRejected case silently misattributed in traces.
   • Fix: tracing::debug!(reason, "trusted trigger inbound rejection");

Maintainability (Low)

5. Low TriggerConversationFields single-use pass-through struct (crates/ironclaw_reborn_composition/src/trigger_poller_trusted_submit.rs:130-183, conf 75) — anchor: :130 (body only)
   • Struct constructed only by trigger_conversation_fields, fields immediately destructured. No methods, no reuse.
6. Low BindingResolutionPolicy + BindingResolutionDispatch structurally identical enums (crates/ironclaw_conversations/src/inbound.rs:344-390, conf 72) — anchor: :353
   • Same variants. Only semantic difference: into_resolution_parts zeros requested_* on Trusted arm. Single enum + inline match sufficient.

Local patterns (Low)

7. Low R3 #9 STILL — TriggerTrustedInboundBinding 7 raw String fields (crates/ironclaw_triggers/src/trusted_submit.rs:10-18, conf 65) — anchor: :10 (body only)
   • types.md rule: domain identifiers should use newtypes. Re-validated downstream → callers can pass raw strings without compile enforcement.

Other categories

• security ✅ clean
• performance ✅ clean (double scan documented defense-in-depth)
• pattern-refactor ✅ empty

Mergeability

⚠️ PR still draft + mergeable_state=dirty (conflicts). Once rebased:

Recommend before promoting from draft:

• Close #1 (R3 #6 — submitter-level safety rejection test)
• Close #2 (R3 #4 — field-validation error paths)
• Close #3 (pub mod → mod + pub use; matches sibling pattern + closes untrusted-import surface)
• Close #4 (1-line tracing fix — diagnostic quality)

#5-#7 = follow-up cleanups.

[henrypark133]

R3 #7 active-run batch lookup: keeping this deferred. The general trait note is valid: the default TriggerActiveRunLookup::active_run_states implementation loops over active_run_state sequentially. But the current production composition implementation already overrides active_run_states and reads one snapshot for the whole batch, and active cleanup already calls the batch API. So this is not a current hot-path regression in PR 18.5a; broadening this PR to reshape the trait default is not necessary for the trusted-ingress boundary work.

[henrypark133]

Code Review Round 5 (8 reviewers, sonnet) — head 20931c1

Intent: R5 follow-up — hide trusted trigger classifier module + R4 follow-ups.

R4 status:

• ⚠️ R4 #1 Med (85) tests submit_trusted_trigger_fire safety-rejection at submitter → STILL OPEN (see #2).
• ⚠️ R4 #2 Med (80) tests trusted_inbound_request_from_trigger field-validation → STILL OPEN (see #3).
• ⚠️ R4 #3 Med (72) conv trusted_trigger pub module → PARTIAL — module is private ✅ but symbols re-exported (see #1).
• ✅ R4 #4 Low (85) bugs opaque log message → RESOLVED. Now uses structured reason field.
• ⚪ R4 #5 Low (75) maint TriggerConversationFields → NOTE-ONLY (deferred).
• ✅ R4 #6 Low (72) maint BindingResolutionDispatch removed → RESOLVED. BindingResolutionPolicy is single enum.
• ⚪ R4 #7 Low (65) LP raw String fields → NOTE-ONLY (deferred).

Stats: 3 findings. Reviewers: 8. Reviewers failed: none. Body-only: 0. security/bugs/performance/local-patterns/maintainability/pattern-refactor clean.

Event: COMMENT — no Critical/High ≥75.

Findings

Conventions (Medium — high-confidence regression)

1. Medium R4 #3 PARTIAL — lib.rs re-exports classify_inbound_error despite "Hide trusted trigger classifier module" commit (crates/ironclaw_conversations/src/lib.rs:36, confidence 95) — anchor: :36
   • 5 reviewers converged on this finding. mod trusted_trigger made private ✅. But line 36 still pub use trusted_trigger::{TrustedTriggerInboundFailureKind, classify_inbound_error}. Architecture test conversation_trusted_trigger_classifier_stays_out_of_root_exports checks for OLD name classify_trusted_trigger_inbound_error + pub mod trusted_trigger — both absent now, test passes SILENTLY while new name classify_inbound_error is still exported. ironclaw_reborn_composition imports both via crate root (trigger_poller_trusted_submit.rs:8). Self-contradicting commit.
   • Fix: remove pub use trusted_trigger::{...} from lib.rs. Update architecture guard to also check new names. Composition absorbs local classification OR accesses via crate-private path.

Tests (Medium — R4 carryovers)

2. Medium R4 #1 STILL OPEN — submit_trusted_trigger_fire safety-rejection at submitter boundary (crates/ironclaw_conversations/src/inbound.rs:289-308, confidence 85) — anchor: :297
   • validate_trusted_trigger_prompt + trigger_prompt_safety_rejection map_err uncovered through direct ConversationTrustedTriggerSubmitter call. Worker-path tests insufficient per Test-Through-the-Caller — wrapper + mapping layer between helper + side effect.
3. Medium R4 #2 STILL OPEN — trusted_inbound_request_from_trigger field-validation untested at caller (crates/ironclaw_conversations/src/inbound.rs:311-357, confidence 80) — anchor: :311
   • 6 *::new() constructors → InvalidExternalRef. None of rejection arms exercised. Malformed binding → silent classification.

Other categories all clean

Mergeability

⚠️ PR still draft + mergeable_state=unknown. Once rebased:

Must close before promoting from draft:

• #1 (boundary leak — high conf 95, the commit title was "Hide" but symbols still exported. Architecture guard doesn't catch the new names.)
• #2 + #3 (R4 carryovers — Test-Through-the-Caller for safety + binding validation)

Other R4 findings either RESOLVED or NOTE-ONLY (raw Strings, TriggerConversationFields).

[henrypark133]

Medium — R4 #1 STILL OPEN — submit_trusted_trigger_fire safety-rejection untested at submitter boundary.

Reviewer disagreement: some say resolved via worker-path test, others say submitter-level safety gate (validate_trusted_trigger_prompt + trigger_prompt_safety_rejection map_err) still uncovered when driven directly through ConversationTrustedTriggerSubmitter. Per testing.md Test-Through-the-Caller — wrapper + mapping layer between helper + side effect needs caller-level test at the submitter itself, not just the worker.

Fix: Add submit_trusted_trigger_fire_rejects_high_severity_injection_prompt — construct ConversationTrustedTriggerSubmitter via trusted_trigger_fire_submitter, submit TrustedTriggerSubmitRequest with injection prompt, assert InvalidMaterialization + coordinator.submissions().is_empty().

[henrypark133]

Medium — R4 #2 STILL OPEN — trusted_inbound_request_from_trigger field-validation untested at caller.

5 *::new() constructors (AdapterKind, AdapterInstallationId, ExternalActorRef, ExternalConversationRef, ExternalEventId, InboundMessageContentRef) → InvalidExternalRef. None of rejection arms exercised through full ConversationTrustedTriggerSubmitter::submit_trusted_trigger_fire path. Malformed TriggerTrustedInboundBinding (empty adapter_kind) → propagates through classify → InvalidMaterialization without dedicated test.

Fix: Add submit_trusted_trigger_fire_maps_invalid_binding_field_to_invalid_materialization covering empty/malformed adapter_kind, external_actor_id, content_ref propagated through trusted_inbound_request_from_trigger.

[henrypark133]

R5 classifier export finding addressed in efe42cb. Removed the root export of trusted_trigger classifier symbols, made TrustedTriggerInboundFailureKind and classify_inbound_error pub(crate), updated composition to stop importing them from ironclaw_conversations, and tightened the architecture guardrail to check the current symbol names instead of only the old private helper name. This preserves the boundary: conversations keeps submitter classification private; composition owns its local materializer classification.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9bc720737d

ℹ️ 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".

[chatgpt-codex-connector]

Advance past claim-only active rows

When the first active record in a cleanup page has active_fire_slot but no active_run_ref, this now records first_unadvanced_cursor, which prevents next_cursor from advancing for every later record in the page. In the inspected trigger poller cleanup flow, a single claim-only row at the front of the active scan causes every subsequent tick to re-read and skip that same row, so terminal active runs after it are never looked up or cleared and their triggers remain stuck as already-active indefinitely.

Useful? React with 👍 / 👎.