test(reborn): extract trigger-prompt materializer into shared test support - #5609
Conversation
… (wave-4 lane C) Committed follow-up on PR #5584's review thread: submit_triggered_turn_scripted hand-mirrored ConversationContentRefMaterializer::materialize_prompt (trigger_resolve_request + record_trigger_prompt + the content-ref shape, field-by-field) instead of reusing it, and — as flagged — deliberately SKIPPED authorize_trigger_fire and validate_trusted_trigger_prompt. Flagged as a drift trap (trusted-trigger materialization is an ownership boundary, AGENTS.md:61); the review agreed the fix is a #[cfg(feature = "test-support")] materializer helper returning (TriggerMaterializedPrompt, TurnScope) living beside the real materializer, held out of #5584 as a fast-follow with this exact shape. New production-crate (test-support-gated, compiles out of default builds) surface in ironclaw_reborn_composition: - trigger_poller_trusted_submit.rs: materialize_trigger_prompt_for_test, #[cfg(any(test, feature = "test-support"))] — runs the REAL production pipeline via ConversationContentRefMaterializer::materialize_prompt (authorize + validate + resolve + record + content-ref), then an idempotent second resolve_or_create_binding_with_trusted_scope call (safe — same request, same already-created binding) to also return the TurnScope the trait method computes internally but never exposes. Plus two crate-tier unit tests: positive (returned scope/content-ref match an independent ground-truth resolve) and negative (an unsafe prompt is rejected by the REAL safety validator). - test_support/trigger_materializer.rs: pub, feature="test-support"-gated thin wrapper re-exported from test_support/mod.rs — the established wrap_project_create_capability_for_test-style pattern. tests/support/reborn/triggered_submit.rs: submit_triggered_turn_scripted now calls this ONE production-owned helper instead of hand-mirroring; deletes ~90 net lines of duplicated resolve/thread-record/content-ref logic. Verified default-features build of ironclaw_reborn_composition stays warning-free (function/import correctly compile out). Flip-checked at the INTEGRATION level (not just the new crate-unit tests): forced an injection-pattern prompt through submit_triggered_turn_scripted — every triggered-gate scenario correctly failed with "rejected by safety scan", proving the old hand-mirrored path's skip of validate_trusted_trigger_prompt is now closed. Reverted before commit. All touched integration test bins (reborn_group_triggers, reborn_integration_triggered_submit, plus every other wave-4 lane-C bin) rerun green after the extraction.
|
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 selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a test-support seam for trusted-trigger prompt materialization, exposes it from ChangesTrigger prompt test materializer
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TestHarness
participant submit_triggered_turn_scripted
participant materialize_trigger_prompt_for_test
participant trusted_trigger_fire_submitter
TestHarness->>submit_triggered_turn_scripted: invoke with TriggerFire
submit_triggered_turn_scripted->>materialize_trigger_prompt_for_test: materialize prompt and scope
materialize_trigger_prompt_for_test-->>submit_triggered_turn_scripted: (materialized_prompt, turn_scope)
submit_triggered_turn_scripted->>trusted_trigger_fire_submitter: submit(materialized_prompt)
trusted_trigger_fire_submitter-->>submit_triggered_turn_scripted: Accepted{submitted_scope}
submit_triggered_turn_scripted-->>TestHarness: TriggeredSubmission
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a production-owned test helper materialize_trigger_prompt_for_test to drive trusted-trigger prompt materialization in integration tests. This replaces hand-mirrored steps in the integration-test harness, avoiding potential drift in binding resolution, thread recording, and content reference generation. The integration-test harness has been refactored to use this helper, and unit tests have been added to verify its behavior. No review comments were provided, so there is no feedback to address.
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.
Reborn integration-tier coverageLine coverage (Reborn crates): 17.15% — 11039 / 64362 lines Per-crate breakdown (11 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. |
|
🚅 Deployed to the ironclaw-pr-5609 environment in ironclaw-ci-preview
|
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Extract Reborn trigger-prompt materialization into shared test support using the real production materializer to improve harness fidelity.
Stats: 3 findings (from 5 raw, 3 after dedup) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0
tests
- Medium Unsafe prompt rejection is not covered through the harness (
tests/support/reborn/triggered_submit.rs:219-226, confidence 75) - anchor:tests/support/reborn/triggered_submit.rs:219
The crate-tier helper test covers unsafe prompts directly, but no integration test drivesRebornIntegrationHarness::submit_triggered_turn_scriptedwith an unsafe prompt to prove this public harness seam propagates the production validator rejection.
maintainability
- Medium Recover the scope from the real materializer instead of resolving twice (
crates/ironclaw_reborn_composition/src/trigger_poller_trusted_submit.rs:472-486, confidence 75) - anchor:crates/ironclaw_reborn_composition/src/trigger_poller_trusted_submit.rs:472
The helper callsmaterialize_prompt, then rebuilds the same trusted binding request and resolves again solely to recoverTurnScope, coupling test support to idempotency and duplicated private resolve inputs. Also flagged by: performance/Low, approach/Medium.
local-patterns
- Low Module docs carry PR-thread archaeology (
crates/ironclaw_reborn_composition/src/test_support/trigger_materializer.rs:4-13, confidence 75) - anchor:crates/ironclaw_reborn_composition/src/test_support/project_create.rs:3; crates/ironclaw_reborn_composition/src/test_support/skill_activation.rs:64
The new module rustdoc embeds a PR review quote andAGENTS.md:61line-number reference instead of stable seam-oriented prose, which can go stale as docs move.
IronLoop Review StatusHead: Current reviewers:
Recent activity:
Commands:
|
test(reborn): extract trigger-prompt materializer into shared test support
Summary
Follow-up committed on the #5584 review thread: the Reborn integration harness's triggered-submit path (
tests/support/reborn/triggered_submit.rs) hand-mirrored the production trigger-prompt materialization logic. This extracts a single shared, cfg-gated helper that calls the REAL production materializer instead.What changed
crates/ironclaw_reborn_composition/src/trigger_poller_trusted_submit.rs: newmaterialize_trigger_prompt_for_testunder#[cfg(any(test, feature = "test-support"))]— returns(TriggerMaterializedPrompt, TurnScope)by driving the realConversationContentRefMaterializer::materialize_prompt(authorize + validate + resolve + record + content-ref) plus one idempotent second resolve to recover theTurnScope. Compiles out of default builds (verified).crates/ironclaw_reborn_composition/src/test_support/trigger_materializer.rs: re-export home.tests/support/reborn/triggered_submit.rs:submit_triggered_turn_scriptednow calls the shared helper instead of hand-mirroring — net −67 lines.Why it matters (harness-fidelity fix)
The old hand-mirrored path skipped
validate_trusted_trigger_prompt— triggered-submit integration tests could accept prompts production would reject. Integration-level flip-check: forcing an injection-pattern prompt through the harness now correctly fails every triggered-gate scenario with "rejected by safety scan".Verification
cargo build -p ironclaw_reborn_composition(default features) — new code compiled outcargo build --tests --all-features;cargo clippy --all --tests --examples --all-features -- -D warnings— cleanreborn_group_triggersunder--features integration— greenZero production-behavior change; the wave-4 coverage PR stacks on this.
🤖 Generated with Claude Code