lywsvip/ironclaw
1
0
Fork 0
mirror of https://github.com/nearai/ironclaw.git synced 2026-09-03 08:06:01 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(run_delivery): address review comments and restore CI gates

Browse Source 
Review fixes (CodeRabbit on 01e887f/f8af109):
- Grace loop fails loud: log the bound TurnError on state-poll failure and
  the RunDeliveryError on terminal-notice build failure before falling back
  to the timeout copy, with silent-ok markers on both intentional fallbacks.
- Hoist TriggeredReplyTargetAuthority, CodecChannelTargetResolver, and
  TriggeredNotificationContext to one construction before the watcher loop;
  the race-grace arm, timeout arm, and loop body now share it.
- Collapse the duplicated failure-summary expression into one closure and
  name TurnStatus::Failed explicitly so future statuses are compiler-visible.
- Drop the stale "Only three states" count from the surface-contract doc.
- Test fixture: encode the late-terminal flip as one Option<(usize,
  ScriptedRunState)> field instead of two correlated Options with an expect.
- Terminal-crossing test: document why flip_after=30 deterministically
  outruns the wait poll budget and assert the grace loop issues no
  cancellation (cancel_calls == 0).

CI:
- composition-budget: re-seed loc_ceiling 40432 -> 40593 (measured on the
  merged tree; the #7131 settlement observer adds +161 governed LOC of
  wiring) and move the arch-test record with it.
- trigger_poller: use the colon-form tracing target required by #7146.
This commit is contained in:
Firat Sertgoz
2026-08-08 13:42:28 +03:00
parent f8af109542
commit c2460ed9ef
5 changed files with 99 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
crates/app/ironclaw_architecture_tests/tests/reborn_restructure_baselines.rs
View File

@@ -123,7 +123,11 @@ const WS0_COMPOSITION_SHARE_BP: usize = 658;
/// notification-channel capability split and delivery wiring preserve their
/// mediated owners. Measured on the merged tree; the manifest ceiling and
/// observed value move with this record so the increase is explicit.
const COMPOSITION_ABSOLUTE_SRC_LOC: usize = 40_432;
/// ✎ Re-recorded 40_432 → 40_593 on 2026-08-08 for #7131: the required
/// `on_run_failure_settled` production observer plus its traced observer
/// test. Measured on the merged tree; the manifest ceiling and observed
/// value move with this record.
const COMPOSITION_ABSOLUTE_SRC_LOC: usize = 40_593;
/// Composition dispatch, from the same `--print` run: "composition dispatch:
/// 827 Arc<dyn> (governed prod, excl slack/extension_host)".

2
crates/app/ironclaw_composition/src/automation/trigger_poller.rs
View File

@@ -244,7 +244,7 @@ impl TriggerFireSettlementObserver for PostSubmitHookObserver {
// observer only emits structured automation-health telemetry and
// must never mint a replacement run or bypass the delivery path.
tracing::warn!(
target = "ironclaw::reborn::trigger_poller",
target: "ironclaw::reborn::trigger_poller",
tenant_id = %event.tenant_id,
trigger_id = %event.trigger_id,
fire_slot = %event.fire_slot,

142
crates/product/ironclaw_assistant/src/run_delivery/triggered.rs
View File

@@ -609,6 +609,27 @@ async fn notify_background_run(
// extensions, so retraction is per-message.
let mut messages_to_delete_after_final: Vec<(String, DeliveredChannelMessage)> = Vec::new();
// The reply authority, codec resolver, and notification context are
// loop-invariant for one fire: scope, actor, run id, and the codec
// snapshot never change across polls. Built once so the watcher loop,
// the race-grace arm, and the timeout arm share one construction.
let authority = TriggeredReplyTargetAuthority {
scope: scope.clone(),
actor: actor.clone(),
};
let target_resolver = CodecChannelTargetResolver::with_context_label(
target_codecs.to_vec(),
"background run notification",
);
let notification_context = TriggeredNotificationContext {
scope: &scope,
thread_scope: &thread_scope,
actor: &actor,
run_id,
authority: &authority,
target_resolver: &target_resolver,
};
loop {
let state = match wait_for_actionable_state(
services.turn_coordinator.as_ref(),
@@ -658,17 +679,30 @@ async fn notify_background_run(
let grace_deadline =
tokio::time::Instant::now() + settings.max_wait.min(TERMINAL_RACE_GRACE);
loop {
let fresh = services
let fresh = match services
.turn_coordinator
.get_run_state(GetRunStateRequest {
scope: scope.clone(),
run_id,
})
.await
.ok();
{
Ok(state) => Some(state),
Err(err) => {
// silent-ok: the state poll during the race-grace
// window failed; fall back to the timeout notice.
tracing::debug!(
target: TRACE_TARGET,
%run_id,
error = %err,
"terminal race grace poll failed; using timeout notice"
);
None
}
};
match fresh {
Some(state) if state.status.is_terminal() => {
if let Ok(Some(plan)) = notification_plan_for_state(
let plan = notification_plan_for_state(
services,
&scope,
&actor,
@@ -676,25 +710,19 @@ async fn notify_background_run(
run_id,
&trigger_label,
)
.await
{
let authority = TriggeredReplyTargetAuthority {
scope: scope.clone(),
actor: actor.clone(),
};
let target_resolver =
CodecChannelTargetResolver::with_context_label(
target_codecs.to_vec(),
"background run notification",
);
let notification_context = TriggeredNotificationContext {
scope: &scope,
thread_scope: &thread_scope,
actor: &actor,
run_id,
authority: &authority,
target_resolver: &target_resolver,
};
.await;
if let Err(err) = &plan {
// silent-ok: the terminal notice could not be
// built during the grace window; fall back to
// the timeout copy.
tracing::warn!(
target: TRACE_TARGET,
%run_id,
error = %err,
"terminal race notification build failed; using timeout notice"
);
}
if let Ok(Some(plan)) = plan {
let fan =
fan_out_plan(services, &notification_context, &plan, &targets)
.await;
@@ -731,22 +759,6 @@ async fn notify_background_run(
gate_ref_for_routing: None,
keeps_run_parked: false,
};
let authority = TriggeredReplyTargetAuthority {
scope: scope.clone(),
actor: actor.clone(),
};
let target_resolver = CodecChannelTargetResolver::with_context_label(
target_codecs.to_vec(),
"background run notification",
);
let notification_context = TriggeredNotificationContext {
scope: &scope,
thread_scope: &thread_scope,
actor: &actor,
run_id,
authority: &authority,
target_resolver: &target_resolver,
};
let fan =
fan_out_plan(services, &notification_context, &timeout_plan, &targets).await;
let outcome = delivery_outcome_for_fan(&fan);
@@ -806,22 +818,6 @@ async fn notify_background_run(
};
let next_blocked_marker = blocked_actionable_marker(&state);
let authority = TriggeredReplyTargetAuthority {
scope: scope.clone(),
actor: actor.clone(),
};
let target_resolver = CodecChannelTargetResolver::with_context_label(
target_codecs.to_vec(),
"background run notification",
);
let notification_context = TriggeredNotificationContext {
scope: &scope,
thread_scope: &thread_scope,
actor: &actor,
run_id,
authority: &authority,
target_resolver: &target_resolver,
};
let fan = fan_out_plan(services, &notification_context, &plan, &targets).await;
messages_to_delete_after_final.extend(fan.messages_to_retract_after_final);
@@ -991,8 +987,8 @@ async fn resolve_notification_targets(
/// ## Background-run channel surface contract
///
/// A background run is **notification-only, plus gate-resolution input** — it
/// is NOT a conversational surface and it never pushes results. Only three
/// states produce output:
/// is NOT a conversational surface and it never pushes results. The
/// deliverable states are:
///
/// - `BlockedApproval` → gate prompt (approve/deny) on every channel
/// - `BlockedAuth` → OAuth prompt to personal DMs + a redacted notice
@@ -1159,33 +1155,23 @@ async fn notification_plan_for_state(
// notice — cancelled runs never carry a failure category in the
// real system, and a failure summary would mislabel a host or
// operator cancel as a failed run.
let failure_summary = || {
reborn_failure_summary_for_category(
state.failure.as_ref().map(|failure| failure.category()),
)
.to_string()
};
let (text, discriminator) = match state.status {
TurnStatus::Cancelled => (
prompts::TRIGGERED_RUN_CANCELED_MESSAGE.to_string(),
"cancelled",
),
TurnStatus::RecoveryRequired => (
state
.failure
.as_ref()
.map(|failure| {
reborn_failure_summary_for_category(Some(failure.category()))
})
.unwrap_or_else(|| reborn_failure_summary_for_category(None))
.to_string(),
"recovery-required",
),
_ => (
state
.failure
.as_ref()
.map(|failure| {
reborn_failure_summary_for_category(Some(failure.category()))
})
.unwrap_or_else(|| reborn_failure_summary_for_category(None))
.to_string(),
"failed",
),
TurnStatus::RecoveryRequired => (failure_summary(), "recovery-required"),
TurnStatus::Failed => (failure_summary(), "failed"),
// Unreachable: the enclosing arm narrows to the three
// terminal statuses; named arms keep new statuses
// compiler-visible instead of silently inheriting "failed".
_ => return Ok(None),
};
Ok(Some(TriggeredNotificationPlan {
notifications: vec![TriggeredNotification {

35
crates/product/ironclaw_assistant/tests/run_delivery_contract.rs
View File

@@ -111,11 +111,11 @@ struct ScriptedTurnCoordinator {
clamp_at_last: bool,
calls: Mutex<usize>,
cancel_calls: Mutex<Vec<TurnRunId>>,
/// Optional late transition: from call `flip_after` on, `flip_to` is
/// returned instead of the scripted sequence — used to race a terminal
/// state in after the wait backstop has already fired.
flip_after: Option<usize>,
flip_to: Option<ScriptedRunState>,
/// Optional late transition: from call `flip.0` on, `flip.1` is returned
/// instead of the scripted sequence — used to race a terminal state in
/// after the wait backstop has already fired. One tuple keeps the flip
/// point and target from being configured independently.
flip: Option<(usize, ScriptedRunState)>,
}
impl ScriptedTurnCoordinator {
@@ -126,8 +126,7 @@ impl ScriptedTurnCoordinator {
clamp_at_last: true,
calls: Mutex::new(0),
cancel_calls: Mutex::new(Vec::new()),
flip_after: None,
flip_to: None,
flip: None,
}
}
@@ -143,8 +142,7 @@ impl ScriptedTurnCoordinator {
clamp_at_last: true,
calls: Mutex::new(0),
cancel_calls: Mutex::new(Vec::new()),
flip_after: Some(flip_after),
flip_to: Some(terminal),
flip: Some((flip_after, terminal)),
}
}
@@ -187,11 +185,8 @@ impl TurnCoordinator for ScriptedTurnCoordinator {
let mut calls = self.calls.lock().expect("calls");
let call = *calls;
*calls += 1;
let scripted = match self.flip_after {
Some(flip) if call >= flip => self
.flip_to
.clone()
.expect("late-terminal flip target present"),
let scripted = match self.flip {
Some((flip_after, ref terminal)) if call >= flip_after => terminal.clone(),
_ => {
let idx = if self.clamp_at_last {
call.min(self.states.len() - 1)
@@ -2893,6 +2888,13 @@ async fn triggered_run_crossing_terminal_during_timeout_grace_delivers_cancellat
// crosses into a terminal state during the bounded race-grace window
// must receive the correct terminal notice — the timeout arm used to
// exit the watcher and lose the terminal copy.
//
// Deterministic grace-path entry: the wait loop polls at 1ms intervals
// doubling to a 5s cap against a 60ms max_wait, so the wait backstop
// fires after at most ~8 `get_run_state` calls — far below the flip at
// call 30. The scripted Cancelled state is therefore never observable
// inside the wait loop; only the grace window (60ms, 1ms polls) reaches
// call 30 and observes the terminal state.
let harness = build_triggered_harness_with_turns(
Arc::new(ScriptedTurnCoordinator::with_late_terminal(
scripted_state(TurnStatus::Running, None),
@@ -2923,6 +2925,11 @@ async fn triggered_run_crossing_terminal_during_timeout_grace_delivers_cancellat
"no timeout copy for a run that reached terminal: {}",
texts[0]
);
assert_eq!(
harness.turns.cancel_call_count(),
0,
"grace loop observed the terminal state without issuing a cancellation"
);
}
/// Spec §7: with no notification channels configured, notifications live in

11
scripts/ci/composition-budget.toml
View File

@@ -151,7 +151,12 @@ observed_date = "2026-08-07"
# Phase 0 this file blesses — rather than raising the ceiling to absorb it. Net
# effect is a smaller composition than before the fixes. Measured on this tree
# with `bash scripts/ci/check-composition-budget.sh`.
loc_ceiling = 40432
# Re-seeded 40432 -> 40593 on 2026-08-08 for #7131: the required
# `on_run_failure_settled` production observer (triggered-run failure
# settlement health telemetry) plus its traced observer test add +161 governed
# LOC of composition wiring; measured on the merged tree with the gate (40593,
# matching the CI failure output). Set to current, not padded.
loc_ceiling = 40593
# Working slack for in-flight PRs. Deliberately small: the inflow this gate
# exists to catch was +619 lines, and a tolerance that would have absorbed it
# is a gate that constrains nothing. A change adding more than this to
@@ -162,8 +167,8 @@ loc_tolerance = 150
loc_nudge_slack = 200
# Informational — observed when this file was last updated. Not consulted for
# the pass/fail decision.
loc_observed = 40432
loc_observed_date = "2026-08-07"
loc_observed = 40593
loc_observed_date = "2026-08-08"
# --- Dispatch (Arc<dyn>) ratchet ------------------------------------------
# A companion metric for the "reduce traits & dispatch" goal (issue #6168 / the