test(reborn): record QA-trace phrases to terminal-or-gate instead of hanging on auth gates

[serrrfirat]

Summary

• The QA-trace recorder drove phrases via RebornRuntime::send_user_message, which blocks in wait_for_terminal until the run reaches a terminal status. A phrase that raises an interactive auth/approval gate (e.g. "connect to Gmail") parks the run in the non-terminal TurnStatus::BlockedAuth and never reaches terminal — so the recorder hung for the full 180s poll budget and died with RunTimeout, producing no fixture.
• This is purely a recorder limitation, not a product bug: in production the same gate is resolved by the user via the WebUI OAuth flow + resolve_gate. The recorder has no resolver and no human, so a parked run looks identical to "still working."
• Adds a test-support-only RebornRuntime::send_user_message_until_gate that returns when the run first reaches a terminal status or parks on a Blocked* gate, plus the RebornTurnDriveOutcome it returns. The QA recorder uses it so a gated phrase records the agent's decisions up to the gate and reports the pause, instead of hanging.

Why this shape

• Observation only, no second resolution path. This method only observes where a run paused. Gate resolution stays on the WebUI RebornServicesApi facade (resolve_gate) per the [Reborn] Add approval/auth interaction services #3094 seam, honoring the crate guardrail ("keep this crate's auth seam reusable by that layer without implementing a second gate-resolution path").
• Zero production bytes. send_user_message_until_gate, wait_for_terminal_or_gate, and RebornTurnDriveOutcome are all #[cfg(any(test, feature = "test-support"))].
• The only production-path edit is extracting submit_user_turn and switching the per-conversation send lock from lock() to lock_owned() so the guard can outlive the shared submit call — behavior-preserving for the existing send_user_message path.

What this does NOT do

• No fixtures committed and no contract/replay tests enabled — recorded fixtures and the contract tooling land separately after triage.
• No gate resolution / token seeding.

Change Type

• Bug fix
• New feature
• Refactor
• Documentation
• CI/Infrastructure
• Security
• Dependencies
• Test infrastructure

Validation

• cargo fmt --all -- --check — clean
• cargo clippy -p ironclaw_reborn_composition --features test-support --tests — zero warnings
• cargo test --test reborn_qa_recorded_behavior — 12 passed, 0 failed, 21 ignored
• All five sibling QA suites (which include the edited qa_trace.rs) compile: reborn_qa_{connect_flows,channel_delivery,doc_grounding,routines,web_fetch}
• Manual live confirmation: all 10 record_* phrases now record; the four auth-gated phrases (connect_gmail, routine_crm_inbox, routine_meeting_prep, routine_release_watch) report paused at gate: status BlockedAuth, gate_ref … and flush instead of hanging 180s.

Security Impact

None. New surface is test-support gated; the production change is a lock-type swap with identical mutual-exclusion semantics.

Database Impact

None.

Reborn Trust-Boundary Checklist

N/A — test-support recorder seam; no policy/trust/evidence types, no gate-resolution path added (resolution remains on the WebUI facade per #3094).

Blast Radius

ironclaw_reborn_composition runtime send path (refactored, behavior-preserving) + the QA-trace recorder support module. No production binary surface changes.

Rollback Plan

Revert the single commit.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Testing Support
  • Updated QA trace recording to drive phrases through runtime gate-awareness, recording either successful final replies or an expected “paused on gate” outcome (including pause status, gate reference, and partial text).
  • Added test-only ability to submit a user turn and stop when the run is terminal or parked on a resolvable gate, returning a structured outcome for assertions.
• Bug Fixes
  • Improved the user-turn submission/wait flow to preserve turn synchronization across async waits and to avoid masking successful primary results with cleanup errors.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors Reborn runtime's user-turn flow to support gated interactions (OAuth/approval). A new SubmittedTurn holds the send-lock guard across awaits using OwnedMutexGuard. A test-only send_user_message_until_gate returns RebornTurnDriveOutcome::Terminal or ::BlockedOnGate. QA trace recording now handles both outcomes.

Changes

Gated Turn Support for Reborn Runtime Testing

Layer / File(s)	Summary
Lock Management & Outcome Types crates/ironclaw_reborn_composition/src/runtime.rs	Import OwnedMutexGuard. Introduce SubmittedTurn struct carrying the owned guard with scope/run identifiers, and RebornTurnDriveOutcome enum (Terminal reply or BlockedOnGate with gate ref and partial text).
Turn Submission with Owned Guard crates/ironclaw_reborn_composition/src/runtime.rs	submit_user_turn acquires the send lock via lock_owned() and returns SubmittedTurn containing scope, run id, accepted-message ref, and the owned guard for later awaits.
Internal Send Message Refactor crates/ironclaw_reborn_composition/src/runtime.rs	send_user_message_internal now delegates to submit_user_turn, awaits terminal state separately while holding the owned guard, reads latest assistant text, and performs accepted-message cleanup after the primary reply future resolves with conditional error surfacing.
Gate-Aware Wait & Test-Only API crates/ironclaw_reborn_composition/src/runtime.rs	Add wait_for_terminal_or_gate helper to return early on terminal state or user-resolvable BlockedApproval, BlockedAuth, or BlockedResource (excluding BlockedDependentRun). Add public (test-only) send_user_message_until_gate that returns RebornTurnDriveOutcome::Terminal or ::BlockedOnGate (includes gate_ref and partial_text).
Crate-Root Type Export crates/ironclaw_reborn_composition/src/lib.rs	Conditionally re-export RebornTurnDriveOutcome under cfg(any(test, feature = "test-support")).
Test Gateway & Integration crates/ironclaw_reborn_composition/src/runtime.rs	Add AuthGateToolCallingGateway test helper gating model calls behind extension capabilities. Add integration test send_user_message_until_gate_returns_blocked_on_auth_gate verifying BlockedOnGate outcome with auth-prefixed gate_ref and TurnStatus::BlockedAuth.
QA Trace Recording Integration tests/support/reborn/qa_trace.rs	record_qa_phrase now uses send_user_message_until_gate and branches on RebornTurnDriveOutcome: assert successful final reply for Terminal; log gate details for BlockedOnGate. Minor import rewrap.

Sequence Diagram

sequenceDiagram
  participant Test as Test Code
  participant API as send_user_message_until_gate
  participant Submit as submit_user_turn
  participant Wait as wait_for_terminal_or_gate
  participant State as RunState Monitor
  participant Outcome as RebornTurnDriveOutcome
  
  Test->>API: conversation, phrase text
  API->>Submit: acquire send lock
  Submit-->>API: SubmittedTurn (owned guard)
  API->>Wait: owned guard, run_id
  
  loop Poll Until Terminal or Gate
    Wait->>State: check run state
    alt Terminal
      State-->>Wait: Success/Error
      Wait-->>API: proceed
    else BlockedApproval/Auth/Resource
      State-->>Wait: blocked status
      Wait-->>API: return early
    end
  end
  
  API->>Outcome: build Terminal or BlockedOnGate
  Outcome-->>Test: RebornTurnDriveOutcome

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~23 minutes

Possibly Related PRs

• nearai/ironclaw#4773: Implements the gated turn API (send_user_message_until_gate/RebornTurnDriveOutcome) that this PR extends to QA trace recording.

Poem

A lock held firm through async sleep,
Gate pauses caught, no race so deep.
Terminal paths or halts await,
Traces now record the gated state. 🔒✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title follows Conventional Commits style with type (test), scope (reborn), and summary clearly describing the main change.
Description check	✅ Passed	Description is comprehensive: includes summary, change type, validation steps, and required checklists (security, trust boundary, database, blast radius, rollback). All critical sections are filled.
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 introduces support for driving a single turn until it pauses on a gate (such as auth or approval) rather than waiting only for a terminal status, which prevents hangs during QA trace recording. It refactors RebornRuntime to extract turn submission logic and adds send_user_message_until_gate along with corresponding test updates. The review feedback focuses on improving error handling and safety: specifically, ensuring that primary errors (such as timeouts, cancellations, or reply failures) are not swallowed when secondary cleanup operations fail, and replacing a matches! macro with an exhaustive match statement on TurnStatus to guarantee compile-time safety.

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.

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@crates/ironclaw_reborn_composition/src/runtime.rs`:
- Around line 1621-1629: The fast-path is wrongly grouping
TurnStatus::BlockedDependentRun with user-resolvable gate states; remove
TurnStatus::BlockedDependentRun from the matches() that assigns blocked_on_gate
so it no longer triggers the early return, and add an explicit branch for
TurnStatus::BlockedDependentRun (or otherwise let it fall through) so
dependent-run waits are handled separately (i.e., update the match in the code
that sets blocked_on_gate to exclude BlockedDependentRun and, if needed, add a
dedicated match arm for TurnStatus::BlockedDependentRun to avoid returning
early).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ad560b01-3491-44ae-931a-1f8ed1668cfd

📥 Commits

Reviewing files that changed from the base of the PR and between c9edbfe and f20b3f8.

📒 Files selected for processing (3)

• crates/ironclaw_reborn_composition/src/lib.rs
• crates/ironclaw_reborn_composition/src/runtime.rs
• tests/support/reborn/qa_trace.rs

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@crates/ironclaw_reborn_composition/src/runtime.rs`:
- Around line 1751-1757: Add a regression test that exercises the live
pause/resume path for BlockedOnGate to pin the invariant described: drive a
single runtime through send_user_message_until_gate(...) to hit the gate, call
clear_accepted_message, then resolve_gate(...) and let the same runtime resume
to completion, asserting the resumed run finishes correctly and that
HostSkillContextSource::load_skill_context_candidates falls back to
active_plan_candidates when take_message_for_run returns None; place or extend
the test in the existing QA trace test module (the caller currently in
tests/support/reborn/qa_trace.rs) so it does not shutdown immediately but
performs the full send->resolve->resume flow. Ensure the test fails if resume
depends on the cleared accepted-message entry (i.e., assert final outputs/state
match expectations).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c5ca9c8a-6304-4d10-beae-3adbdff53aea

📥 Commits

Reviewing files that changed from the base of the PR and between c1a64a5 and 59e824b.

📒 Files selected for processing (1)

• crates/ironclaw_reborn_composition/src/runtime.rs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the target segment and surrounding functions
rg -n "send_user_message_until_gate|clear_accepted_message|accepted_message_ref|BlockedOnGate|resolve_gate" crates/ironclaw_reborn_composition/src/runtime.rs tests/support/reborn/qa_trace.rs crates -S

Repository: nearai/ironclaw

Length of output: 50371

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "send_user_message_until_gate|clear_accepted_message|accepted_message_ref|BlockedOnGate|resolve_gate" crates/ironclaw_reborn_composition/src/runtime.rs tests/support/reborn/qa_trace.rs crates -S

Repository: nearai/ironclaw

Length of output: 50371

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show exact context around the changed comment
sed -n '1680,1795p' crates/ironclaw_reborn_composition/src/runtime.rs | nl -ba | sed -n '1,160p'

# Show the QA trace call site that drives BlockedOnGate
sed -n '140,260p' tests/support/reborn/qa_trace.rs | nl -ba | sed -n '1,160p'

# Show where resolve_gate is called from within this runtime module near the other gate handling
rg -n "resolve_gate\\(" crates/ironclaw_reborn_composition/src/runtime.rs -S

Repository: nearai/ironclaw

Length of output: 102

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show exact context around the changed comment
sed -n '1680,1795p' crates/ironclaw_reborn_composition/src/runtime.rs | nl -ba | sed -n '1,160p'

# Show the QA trace call site that drives BlockedOnGate
sed -n '140,260p' tests/support/reborn/qa_trace.rs | nl -ba | sed -n '1,160p'

# Show where resolve_gate is called from within this runtime module near the other gate handling
rg -n "resolve_gate\\(" crates/ironclaw_reborn_composition/src/runtime.rs -S

Repository: nearai/ironclaw

Length of output: 102

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== runtime.rs: blocked-path cleanup comment context =="
rg -n "Clearing the accepted message is safe even on the `BlockedOnGate`" crates/ironclaw_reborn_composition/src/runtime.rs
sed -n '1735,1785p' crates/ironclaw_reborn_composition/src/runtime.rs

echo
echo "== qa_trace.rs: BlockedOnGate match context =="
rg -n "RebornTurnDriveOutcome::BlockedOnGate" tests/support/reborn/qa_trace.rs
sed -n '140,240p' tests/support/reborn/qa_trace.rs

echo
echo "== runtime.rs: resolve_gate calls =="
rg -n "resolve_gate\\(" crates/ironclaw_reborn_composition/src/runtime.rs -S

Repository: nearai/ironclaw

Length of output: 7001

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Where BlockedOnGate is handled in runtime.rs
echo "== BlockedOnGate occurrences (runtime.rs) =="
rg -n "BlockedOnGate" crates/ironclaw_reborn_composition/src/runtime.rs

echo
echo "== Gate resume / resolve_gate-adjacent code regions =="
sed -n '6200,6900p' crates/ironclaw_reborn_composition/src/runtime.rs | nl -ba | sed -n '1,900p'

# 2) Accepted-message plumbing: find take_message_for_run / accepted message key usage
echo
echo "== take_message_for_run usages (repo) =="
rg -n "take_message_for_run" crates

echo
echo "== Skill activation accepted-message handling (activation.rs) =="
rg -n "clear_accepted_message|take_message_for_run|SkillActivationMessageKey::new|AcceptedMessageRef" crates/ironclaw_first_party_extension_ports/src/activation.rs
sed -n '320,460p' crates/ironclaw_first_party_extension_ports/src/activation.rs

# 3) Where resolve_gate is defined in runtime (if any)
echo
echo "== resolve_gate definitions (runtime.rs + composition) =="
rg -n "async fn resolve_gate|fn resolve_gate" crates/ironclaw_reborn_composition/src

Repository: nearai/ironclaw

Length of output: 392

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== runtime.rs: inspect around resolve_gate call sites (6440-6535, 6495-6565, 6680-6725) =="

sed -n '6400,6565p' crates/ironclaw_reborn_composition/src/runtime.rs | cat

echo "----"

sed -n '6660,6745p' crates/ironclaw_reborn_composition/src/runtime.rs | cat

echo
echo "== activation.rs: clear_accepted_message + take_message_for_run + selector usage =="

rg -n "fn clear_accepted_message|take_message_for_run|SkillActivationMessageKey::new|accepted_message_ref" crates/ironclaw_first_party_extension_ports/src/activation.rs

sed -n '320,430p' crates/ironclaw_first_party_extension_ports/src/activation.rs | cat
sed -n '740,820p' crates/ironclaw_first_party_extension_ports/src/activation.rs | cat

echo
echo "== runtime.rs: where resolve_gate is mentioned in comments/docs =="

rg -n "resolve_gate|Gate resume|gate-resume|BlockedOnGate" crates/ironclaw_reborn_composition/src/runtime.rs | head -n 80

Repository: nearai/ironclaw

Length of output: 21620

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find tests exercising blocked gates + resolve_gate / resumed completion
rg -n "BlockedOnGate" crates/ironclaw_reborn_composition/src/runtime -S
rg -n "resolve_gate\\(" crates/ironclaw_reborn_composition/src/runtime -S

rg -n "BlockedOnGate" crates/ironclaw_reborn_composition/tests -S
rg -n "resolve_gate\\(" crates/ironclaw_reborn_composition/tests -S

# Also check broader crates for a pattern: drive until gate, then resolve_gate
rg -n "send_user_message_until_gate\\(|send_user_message_until_gate\\s*\\(" crates -S
rg -n "send_user_message_until_gate[\\s\\S]{0,800}resolve_gate\\(" crates -S || true

Repository: nearai/ironclaw

Length of output: 41

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Print line hits only (avoid huge output)
echo "== runtime tests: BlockedOnGate =="
rg -n "BlockedOnGate" crates/ironclaw_reborn_composition/src/runtime -S || true

echo
echo "== composition tests: BlockedOnGate =="
rg -n "BlockedOnGate" crates/ironclaw_reborn_composition/tests -S || true

echo
echo "== any tests using send_user_message_until_gate =="
rg -n "send_user_message_until_gate\\(" crates/ironclaw_reborn_composition -S || true

echo
echo "== any tests calling resolve_gate and also send_user_message_until_gate nearby =="
rg -n --context 4 "send_user_message_until_gate\\(|resolve_gate\\(" crates/ironclaw_reborn_composition -S || true

Repository: nearai/ironclaw

Length of output: 6586

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the full send_user_message_until_gate function area
sed -n '1675,1795p' crates/ironclaw_reborn_composition/src/runtime.rs | cat

# Look for capture_plan / selected_candidates / activation capture flags near gate-blocked flow
rg -n "capture_plan|active_plan_candidates|selected_candidates\\(|merge_active_plan|CapturedSkillActivationPlan" crates/ironclaw_reborn_composition/src/runtime.rs

# Inspect nearby drive/submit path around where BlockedOnGate outcome is constructed
rg -n "Ok\\(RebornTurnDriveOutcome::BlockedOnGate" -n crates/ironclaw_reborn_composition/src/runtime.rs
sed -n '1600,1775p' crates/ironclaw_reborn_composition/src/runtime.rs | cat

# Search tests for a pattern: drive until gate (BlockedOnGate) then resolve_gate on same runtime/thread
rg -n --context 6 "send_user_message_until_gate\\(" crates/ironclaw_reborn_composition/tests -S || true
rg -n --context 6 "resolve_gate\\(" crates/ironclaw_reborn_composition/tests -S || true

# Also check qa_trace usage of outcome + whether any follow-up resolve_gate is called on same runtime
rg -n "qa_trace" -S crates/ironclaw_reborn_composition/tests tests/support/reborn/qa_trace.rs
sed -n '1,260p' tests/support/reborn/qa_trace.rs | cat

Repository: nearai/ironclaw

Length of output: 5356

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find how capture_plan is set/used in the runtime + where SkillActivationMessage is constructed
echo "== capture_plan references (composition) =="
rg -n "capture_plan" crates/ironclaw_reborn_composition/src/runtime.rs crates/ironclaw_reborn_composition/src -S

echo
echo "== capture_plan references (activation / SkillActivationMessage) =="
rg -n "struct SkillActivationMessage|capture_plan" crates/ironclaw_first_party_extension_ports/src/activation.rs -S
rg -n "capture_plan:" crates/ironclaw_first_party_extension_ports/src/activation.rs -S
rg -n "SkillActivationMessage\\b" crates/ironclaw_first_party_extension_ports/src/activation.rs -S

echo
echo "== where record_message (with capture flags) is called =="
rg -n "record_.*message|record_user_message|SkillActivationMessage" crates/ironclaw_reborn_composition/src/runtime.rs crates/ironclaw_reborn_composition/src/webui.rs crates/ironclaw_first_party_extension_ports/src -S

# Show the likely constructor site for the skill activation accepted message
rg -n "SkillActivationMessage \\{|SkillActivationMessage\\b" crates/ironclaw_first_party_extension_ports/src/activation.rs -S
# Print the area around that struct if found
# (use a broad range; avoids needing exact line numbers)
sed -n '1,220p' crates/ironclaw_first_party_extension_ports/src/activation.rs | cat

echo
echo "== runtime.rs: submit/record user turn path likely around submit_user_turn =="
rg -n "submit_user_turn\\(|record_user_message|accepted_message_ref" crates/ironclaw_reborn_composition/src/runtime.rs -S | head -n 80
rg -n "submit_user_turn\\(" crates/ironclaw_reborn_composition/src/runtime.rs -S

Repository: nearai/ironclaw

Length of output: 103

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== activation.rs: capture_plan + SkillActivationMessage struct =="

rg -n "struct SkillActivationMessage|capture_plan" crates/ironclaw_first_party_extension_ports/src/activation.rs -S

echo
# Print the struct region
rg -n "struct SkillActivationMessage" crates/ironclaw_first_party_extension_ports/src/activation.rs -n
# Fallback: print broad range around the first match area
sed -n '640,760p' crates/ironclaw_first_party_extension_ports/src/activation.rs | cat

echo
echo "== activation.rs: active_plan and active_plan_candidates =="

rg -n "fn active_plan\\b|active_plan_candidates" crates/ironclaw_first_party_extension_ports/src/activation.rs -S

sed -n '760,900p' crates/ironclaw_first_party_extension_ports/src/activation.rs | cat

echo
echo "== activation.rs: record_message call sites / how capture_plan is set =="

rg -n "record_message\\(|record_user_message|record_skill_activation_message|capture_plan" crates/ironclaw_first_party_extension_ports/src/activation.rs -S

# Show record_message definitions
rg -n "fn record_message\\b" crates/ironclaw_first_party_extension_ports/src/activation.rs -S
sed -n '200,320p' crates/ironclaw_first_party_extension_ports/src/activation.rs | cat

echo
echo "== runtime.rs: record_user_message(...) argument list near accepted_message_ref capture flags =="

# Focus around the earlier hit lines where record_user_message is called
sed -n '1320,1465p' crates/ironclaw_reborn_composition/src/runtime.rs | cat

Repository: nearai/ironclaw

Length of output: 21245

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Where prompt/policy rebuild for a paused run happens and what skill context source is used
rg -n "gate.*resume|BlockedOnGate|wait_for_terminal_or_gate|resume|resolve_gate" crates/ironclaw_reborn_composition/src/runtime.rs crates/ironclaw_first_party_extension_ports/src/activation.rs -S

# Find HostSkillContextSource usage in composition (how resumes rebuild context)
rg -n "HostSkillContextSource|load_skill_context_candidates|skill_activation_source|SelectableSkillContextSource" crates/ironclaw_reborn_composition/src crates/ironclaw_first_party_extension_ports/src -S

# Check where active_plan/merge_active_plan is invoked during prompt construction
rg -n "merge_active_plan\\(|resolve_activation_plan_with_candidates\\(|activate_skills_for_run\\(" crates/ironclaw_first_party_extension_ports/src/activation.rs -S

# See if gate-resume uses accepted_message_ref on the run_context
rg -n "accepted_message_ref" crates/ironclaw_reborn_composition/src/runtime.rs crates/ironclaw_reborn_composition/src -S | head -n 80

Repository: nearai/ironclaw

Length of output: 32930

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find where active_plans_by_run is inserted/updated in activation.rs
rg -n "active_plans_by_run" crates/ironclaw_first_party_extension_ports/src/activation.rs -S
rg -n "merge_active_plan\\(|active_plans_by_run" crates/ironclaw_first_party_extension_ports/src/activation.rs -S

# show nearby code around merge_active_plan insert into active_plans_by_run (approx)
sed -n '560,720p' crates/ironclaw_first_party_extension_ports/src/activation.rs | cat

# 2) Pull relevant invariants/claude text about regression tests / test-through-caller (from this crate's CLAUDE)
ls -la crates/ironclaw_reborn_composition | head
sed -n '1,220p' crates/ironclaw_reborn_composition/CLAUDE.md | cat

Repository: nearai/ironclaw

Length of output: 21651

---

Pin BlockedOnGate accepted-message cleanup with a pause/resume regression test

The comment’s contract matches the actual selection logic: after clear_accepted_message removes the SkillActivationMessage, HostSkillContextSource::load_skill_context_candidates falls back to active_plan_candidates when take_message_for_run returns None (so gate-resume shouldn’t depend on the cleared accepted-message entry).
Still, the only caller exercising BlockedOnGate here (tests/support/reborn/qa_trace.rs) records the pause and immediately shutdown()s; it never drives the real same-runtime send_user_message_until_gate(...) -> resolve_gate(...) -> resumed completion flow, leaving the “safe even while live/resumable” claim unpinned by a caller-level regression (repo invariant: regression + test-through-the-caller).

🤖 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/runtime.rs` around lines 1751 - 1757,
Add a regression test that exercises the live pause/resume path for
BlockedOnGate to pin the invariant described: drive a single runtime through
send_user_message_until_gate(...) to hit the gate, call clear_accepted_message,
then resolve_gate(...) and let the same runtime resume to completion, asserting
the resumed run finishes correctly and that
HostSkillContextSource::load_skill_context_candidates falls back to
active_plan_candidates when take_message_for_run returns None; place or extend
the test in the existing QA trace test module (the caller currently in
tests/support/reborn/qa_trace.rs) so it does not shutdown immediately but
performs the full send->resolve->resume flow. Ensure the test fails if resume
depends on the cleared accepted-message entry (i.e., assert final outputs/state
match expectations).

Source: Coding guidelines

[serrrfirat]

Multi-agent review summary for PR #4842.

Reviewers completed: security, bugs, performance/concurrency, tests, conventions.
Reviewers failed: none.
Findings after filtering/deduplication: 3 total (2 Medium, 1 Low).

Intent understood as: add test-support-only QA trace recording support that stops at terminal status or interactive auth/approval/resource gates without adding a second gate-resolution path.

Findings:

• Medium / tests: missing hermetic coverage for the new BlockedAuth stop-at-gate behavior.
• Medium / conventions: BlockedOnGate exposes gate_ref as optional even though blocked gate reasons carry required gate refs.
• Low / conventions: public docs still describe dependent-run as a facade-resolvable gate while the implementation correctly excludes it.

[serrrfirat]

Medium: missing hermetic gate-path test

The new wait_for_terminal_or_gate behavior is the core regression guard for this PR, but I only found send_user_message_until_gate exercised through the ignored/live QA recorder path. A change that stops treating BlockedAuth as an immediate gate outcome would still pass the automated suite and return to the 180s timeout behavior this PR is fixing.

Please add a hermetic runtime test, e.g. tests::runtime::send_user_message_until_gate_returns_blocked_on_auth_gate, that drives a scripted auth-gated turn and asserts RebornTurnDriveOutcome::BlockedOnGate { status: TurnStatus::BlockedAuth, .. } before the poll timeout.

[serrrfirat]

Agreed this is the right regression guard. I implemented it (a model gateway that scripts a gmail extension_activate with no configured credential, asserting BlockedOnGate{BlockedAuth}), but the scripted gate scaffolding currently yields a driver_protocol_violation instead of parking on BlockedAuth — a test-harness issue, not a defect in send_user_message_until_gate. Rather than ship it flaky, I'm landing it as a verified follow-up (the fix is the two-model-call install→activate flow mirroring the existing MultiToolCallGateway template). The two other findings are fixed in 9f95d6d.

[coderabbitai]

♻️ Duplicate comments (1)

crates/ironclaw_reborn_composition/src/runtime.rs (1)

1766-1772: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Add the caller-level blocked-gate regression here.

This change fixes a hang by returning early on Blocked*, but it still does not add a #[tokio::test] that drives the real pause/resume path on one runtime (send_user_message_until_gate(...) -> resolve_gate(...) -> resumed completion). That leaves both the original hang and the “clearing the accepted message is safe on BlockedOnGate” invariant unpinned. As per coding guidelines, "Every bug fix must include a regression test using #[test] or #[tokio::test]" and "Test through the caller: when a helper gates a side effect, require a test driving the real call site."

🤖 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/runtime.rs` around lines 1766 - 1772,
Add a caller-level `#[tokio::test]` regression test that validates the
pause/resume path for the `BlockedOnGate` scenario in the runtime. The test
should drive the complete flow by calling send_user_message_until_gate(...) to
trigger a pause, then calling resolve_gate(...) to resume, and verifying that
the runtime completes successfully. This test pins both the hang fix and the
invariant about clearing the accepted message being safe on the BlockedOnGate
path, ensuring the behavior is validated through the actual call site rather
than just the internal helper logic.

Source: Coding guidelines

🤖 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/runtime.rs`:
- Around line 1766-1772: Add a caller-level `#[tokio::test]` regression test
that validates the pause/resume path for the `BlockedOnGate` scenario in the
runtime. The test should drive the complete flow by calling
send_user_message_until_gate(...) to trigger a pause, then calling
resolve_gate(...) to resume, and verifying that the runtime completes
successfully. This test pins both the hang fix and the invariant about clearing
the accepted message being safe on the BlockedOnGate path, ensuring the behavior
is validated through the actual call site rather than just the internal helper
logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6489cfa9-fbd6-4196-a7ec-0cf55dbc390a

📥 Commits

Reviewing files that changed from the base of the PR and between 59e824b and 9f95d6d.

📒 Files selected for processing (2)

• crates/ironclaw_reborn_composition/src/runtime.rs
• tests/support/reborn/qa_trace.rs

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn_composition/src/runtime.rs (1)

1416-1443: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the primary submit/cancel outcome when accepted-message cleanup fails.

This helper still lets secondary cleanup override the real outcome. On the submit_turn error path, clear_accepted_message(...)? can replace the original coordinator failure. On the cancellation path, the same ? returns before cancel_run, so an already-accepted run can keep executing after the caller cancelled.

Suggested fix

         {
             Ok(response) => response,
             Err(error) => {
                 if let Some(skill_activation_source) = &self.skill_activation_source {
-                    skill_activation_source
-                        .clear_accepted_message(&scope, &accepted_message_ref)
-                        .map_err(|clear_error| {
-                            RebornRuntimeError::TurnSubmission(clear_error.to_string())
-                        })?;
+                    if skill_activation_source
+                        .clear_accepted_message(&scope, &accepted_message_ref)
+                        .is_err()
+                    {
+                        tracing::debug!(
+                            accepted_message_ref = accepted_message_ref.as_str(),
+                            "failed to clear accepted message after turn submission failure"
+                        );
+                    }
                 }
                 return Err(error.into());
             }
         };
@@
         if cancellation.is_cancelled() {
             if let Some(skill_activation_source) = &self.skill_activation_source {
-                skill_activation_source
-                    .clear_accepted_message(&scope, &accepted_message_ref)
-                    .map_err(|error| RebornRuntimeError::TurnSubmission(error.to_string()))?;
+                if skill_activation_source
+                    .clear_accepted_message(&scope, &accepted_message_ref)
+                    .is_err()
+                {
+                    tracing::debug!(
+                        accepted_message_ref = accepted_message_ref.as_str(),
+                        "failed to clear accepted message after caller cancellation"
+                    );
+                }
             }
             self.cancel_run(
                 &scope,
                 run_id,
                 SanitizedCancelReason::UserRequested,

As per coding guidelines, "Fail loud: flag silent-failure patterns — ... Errors propagate with ? into thiserror types with context," and per the PR objective this path should not let cleanup failures mask the primary turn outcome.

🤖 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/runtime.rs` around lines 1416 - 1443,
The cleanup logic is allowing secondary errors from clear_accepted_message to
override the primary outcomes. In the error handling block where submit_turn
fails, the clear_accepted_message call with ? can mask the original submission
error. Similarly, in the cancellation path, the clear_accepted_message call with
? can prevent cancel_run from executing if cleanup fails. Instead of propagating
cleanup errors with ?, log them and allow the primary outcomes to be returned:
in the first case return the original submission error, and in the second case
proceed to call cancel_run and return the cancellation error. This ensures that
primary turn submission or cancellation outcomes are not masked by secondary
cleanup failures.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/runtime.rs`:
- Around line 1416-1443: The cleanup logic is allowing secondary errors from
clear_accepted_message to override the primary outcomes. In the error handling
block where submit_turn fails, the clear_accepted_message call with ? can mask
the original submission error. Similarly, in the cancellation path, the
clear_accepted_message call with ? can prevent cancel_run from executing if
cleanup fails. Instead of propagating cleanup errors with ?, log them and allow
the primary outcomes to be returned: in the first case return the original
submission error, and in the second case proceed to call cancel_run and return
the cancellation error. This ensures that primary turn submission or
cancellation outcomes are not masked by secondary cleanup failures.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bbbfbaa7-7bfb-499e-85a6-17964f14d230

📥 Commits

Reviewing files that changed from the base of the PR and between 9f95d6d and eaa0547.

📒 Files selected for processing (1)

• crates/ironclaw_reborn_composition/src/runtime.rs