Conversation
… mint (WS6) PROPOSAL §6.4.2 asked for the trusted-trigger prompt safety scan to move "behind the triggers/kernel seam it guards". It was not a module: it was three lines inside `ConversationTrustedTriggerSubmitter::submit_trusted_trigger_fire` — one of the two implementations of `ironclaw_triggers::TrustedTriggerFireSubmitter` — holding its own `Arc<dyn InjectionScanner>` from `Sanitizer::new()`. That placement is a fail-open: a guard that lives inside one implementation of a port is lost the moment a second implementation exists, and nothing in the tree forced a new submitter to re-run it. The seam is `TrustedTriggerFireSubmitter`, whose only input is the sealed `TrustedTriggerSubmitRequest`, which `ironclaw_triggers` is the sole minter of. So the scan moved to the mint: `TrustedTriggerSubmitRequest::new` is now fallible and calls the new `ironclaw_triggers::prompt_safety` first, making "this prompt passed the trusted-prompt scan" an invariant of the type rather than a step some submitter performs. `new_for_test` delegates to `new`, so the test-support seal bypasses visibility only, never the scan. Behaviour at the fire level is unchanged — same rejection point, same `TriggerError::InvalidMaterialization`, same permanent disposition — and composition's pre-materialization scan is untouched, so defence in depth survives with the second scan relocated and now covering every submitter. `ironclaw_conversations` drops `ironclaw_safety` entirely (the scan was its only use). Enforcement: triggers' boundary rule stops forbidding `ironclaw_safety` (a same-layer, I/O-free `substrates` leaf — a peer edge, not a reach upward), and a NEW `BoundaryRule` for `ironclaw_conversations` forbids it, plus `ironclaw_threads` (§6.4.2's "Never: transcript content"), a crate that was unruled until now. Regression coverage at the caller tier, not on the helper: `tick_rejects_injection_prompt_before_any_trusted_submitter_is_reached` drives the real `TriggerPollerWorker::tick_once` with a materializer that does NOT scan and a submitter configured to accept, and asserts the submitter is never reached. A companion pins that a medium-severity-only prompt still submits, so the mint cannot drift into a blanket filter. Tests: conversations 97 -> 97 (name-identical), triggers 169 -> 173 (+2 worker, +2 prompt_safety unit), architecture 206 -> 206. LAYER_MATRIX_EXCEPTIONS unchanged at 10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-7136 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7136
August 4, 2026 11:20
Destroyed
github-actions
Bot
added
scope: docs
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
💤 Files with no reviewable changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughTrusted-trigger prompt scanning moved from ChangesTrusted trigger safety ownership
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TriggerWorker
participant TrustedTriggerSubmitRequest
participant prompt_safety
participant TrustedTriggerSubmitter
TriggerWorker->>TrustedTriggerSubmitRequest: Create request
TrustedTriggerSubmitRequest->>prompt_safety: Validate prompt
alt Rejected
prompt_safety-->>TrustedTriggerSubmitRequest: InvalidMaterialization
TrustedTriggerSubmitRequest-->>TriggerWorker: Error
TriggerWorker->>TriggerWorker: Persist permanent failure
else Accepted
prompt_safety-->>TrustedTriggerSubmitRequest: Validated prompt
TrustedTriggerSubmitRequest-->>TriggerWorker: Sealed request
TriggerWorker->>TrustedTriggerSubmitter: Submit request
end
Possibly related PRs
Suggested reviewers: 🚥 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 |
Contributor
🔎 Review · PR #7136
Submitted review →Reviewed the complete trusted base-to-head comparison. No concrete correctness, security, architecture, maintainability, or test-coverage defects were found. The prompt scan is enforced at the sealed request mint, constructor failures retain the prior permanent-failure disposition, all mint call sites handle the new fallible API, and dependency/architecture documentation changes are consistent. Automatic · PR opened · attempt 1 of 3 · completed in 1m 12s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Review complete · PR #7136
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison. No concrete correctness, security, architecture, maintainability, or test-coverage defects were found. The prompt scan is enforced at the sealed request mint, constructor failures retain the prior permanent-failure disposition, all mint call sites handle the new fallible API, and dependency/architecture documentation changes are consistent.
Validation and technical details
- Inspected all 16 changed files across refs/ironloop/base (1e2a294) and refs/ironloop/head (847729d).
- Traced every TrustedTriggerSubmitRequest production/test constructor call and the TrustedTriggerFireSubmitter path.
- Verified InvalidMaterialization from mint validation flows through classify_submit_failure and permanent fire persistence as before.
- Verified the production composition materializer retains its independent pre-materialization safety scan.
- Checked dependency-boundary changes, manifests, Cargo.lock, local guidance, and architecture documentation for consistency.
- git diff --check completed successfully.
- Targeted cargo tests could not be executed because cargo is unavailable in the review environment (/bin/bash: cargo: command not found).
- Base:
main - Head:
ws6/conversations-trigger-safetyat847729d - Run:
9af4f366-13ca-4b84-b2ce-cc47bc635815
Collaborator
Author
|
Superseded: this branch's content merged to main inside the #7152 rename PR (verified by content, not ancestry — every added line present under the rename map, prompt_safety.rs byte-identical, the mint call-site live, the dep moved, and the caller-tier regression present; verification recorded in the #7152 refresh report). Closing as merged-via-#7152, not abandoned. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
PROPOSAL §6.4.2 asks for the trusted-trigger prompt safety scan to move "behind the triggers/kernel seam it guards (module move)". This does that, and reports two things the row got wrong.
One of the four items on CHECKLIST WS6.
It was not a module, and its location was a fail-open
The scan was three lines inside
ConversationTrustedTriggerSubmitter::submit_trusted_trigger_fire(crates/ironclaw_conversations/src/inbound.rs), holding its ownArc<dyn InjectionScanner>built fromSanitizer::new().src/trusted_trigger.rs— the module whose name suggests it — isTurnErrorclassification and did not move.That placement is the defect, not just the address: a guard inside one implementation of a port is lost the moment a second implementation exists.
TrustedTriggerFireSubmitteris a public trait with adynwiring point (TriggerPollerWorkerDeps::trusted_submitter); nothing in the tree forced a new impl to re-run the scan, and the crate-tier suite never covered it (the only coverage was composition's worker-driven test).Where it went, and why there
The seam is
TrustedTriggerFireSubmitter, whose only input is the sealedTrustedTriggerSubmitRequest— which §6.4.3 already makesironclaw_triggersthe sole minter of. So the scan moved to the mint:"This prompt passed the trusted-prompt scan" is now an invariant of the type, not a step a submitter performs.
new_for_testdelegates tonew, so the test-support seal bypasses visibility only, never the scan.Severity policy did not move:
ironclaw_safety::validate_trusted_trigger_promptstill owns high/critical-reject, medium-and-below-audit-only. The newcrates/ironclaw_triggers/src/prompt_safety.rsowns only the scanner instance (aLazyLock<Sanitizer>—Sanitizer::newcompiles an Aho-Corasick automaton plus a regex set) and the mapping toTriggerError::InvalidMaterialization.Rejected alternatives
due_fire.rsjust before the mintnewispub(crate)precisely so this crate can add one.TrustedTriggerPromptScannerport wired by compositionBehaviour: identical per fire, wider per port
Same rejection point, same
TriggerError::InvalidMaterialization, same poller disposition —InvalidMaterializationclassifiesPermanentin bothclassify_failureandclassify_submit_failure, which differ only onNotFound. Composition's pre-materialization scan (trigger_poller_trusted_submit.rs:172) is untouched, so the two-scan defence in depth survives with the second scan relocated. What changes is coverage: the second scan now applies to everyTrustedTriggerFireSubmitter, not to whichever one composition happens to wire.⚠ §6.4.2's dep list is wrong by one
§6.4.2 says the move is a module move and lists
safetyamong conversations' target deps. Both cannot hold: the scan was that crate's only use ofironclaw_safety(inbound.rs:4, four imported symbols, one call site). The move drops the dependency. Correct target list:extension_contracts,filesystem,host_api,triggers+ turn vocabulary viahost_api. Amended in place with the sentence it replaces quoted.Enforcement
ironclaw_triggers' boundary rule stops forbiddingironclaw_safety, with the reason inline: safety is a same-layer, I/O-freesubstratesleaf (noironclaw_*deps at all), so this is a peer edge, not a reach upward, and a mint that does not validate what it seals is not a mint.BoundaryRuleforironclaw_conversations— the crate had none — forbidsironclaw_safety, so the fail-open cannot be re-introduced by re-adding the dependency. It also pinsironclaw_threads(§6.4.2's "Never: … transcript content") and the usual retired/kernel-and-above set. Enforced againstcargo metadata, not source text.LAYER_MATRIX_EXCEPTIONSunchanged at 10 (counted in Python betweenconst LAYER_MATRIX_EXCEPTIONSand its];, so neither the struct definition nor the four out-of-array fixtures are miscounted).Sabotage-proved at the caller tier
tick_rejects_injection_prompt_before_any_trusted_submitter_is_reacheddrives the realTriggerPollerWorker::tick_oncewith a materializer that does not scan (RecordingMaterializer— the shape of any materializer that skips or loses the check) and a submitter configured to accept, then asserts the submitter is never reached.Deleting the one scan line from
new:tick_rejects_injection_prompt_before_any_trusted_submitter_is_reached(triggers)FAILED— "an injection prompt must fail the fire permanently, got Some(Submitted { run_id: … })"unsafe_trigger_prompt_is_rejected_before_turn_submission(composition, pre-existing)FAILED— the production-wired guarantee tracks the new locationBoth restored green (
476 passed; 0 failedacross the three touched crates; composition's testok). A companion test pins that a medium-severity-only prompt still submits, so the mint cannot drift into a blanket filter — ordinary automation prompts tripact as.Test accounting (unfiltered
--list, quiescent tree)ironclaw_conversationsironclaw_triggers+prompt_safety::tests::{high_severity_prompt_maps_to_invalid_materialization, medium_severity_warning_is_audit_only},+worker::tests::{tick_rejects_injection_prompt_before_any_trusted_submitter_is_reached, tick_submits_a_prompt_whose_only_injection_warning_is_audit_only}ironclaw_architectureNo test edited for content. Function-roster diff against
origin/mainfor every touched file shows exactly one removal (trigger_prompt_safety_rejection, which moved) and the two added tests — nothing silently deleted.Also verified:
cargo check --tests -p ironclaw_reborn_integration_tests(exit 0) for the twonew_for_testcall sites intests/integration/support/triggered_submit.rs, andcargo clippy --all-targets --all-features -D warningson the three touched crates.🤖 Generated with Claude Code