Skip to content

Commit 6b4bb4a

Browse files
committed
Keep finished lanes from leaving stale reminders armed
The next repo-local sweep target was ROADMAP #66: reminder/cron state could stay enabled after the associated lane had already finished, which left stale nudges firing into completed work. The fix teaches successful lane persistence to disable matching enabled cron entries and record which reminder ids were shut down on the finished event. Constraint: Preserve existing cron/task registries and add the shutdown behavior only on the successful lane-finished path Rejected: Add a separate reminder-cleanup command that operators must remember to run | leaves the completion leak unfixed at the source Confidence: high Scope-risk: narrow Reversibility: clean Directive: If cron-matching heuristics change later, update `disable_matching_crons`, its regression, and the ROADMAP closeout together Tested: cargo fmt --all --check; cargo clippy --workspace --all-targets -- -D warnings; cargo test --workspace; architect review APPROVE Not-tested: Cross-process cron/reminder persistence beyond the in-memory registry used in this repo
1 parent e75d67d commit 6b4bb4a

File tree

2 files changed

+84
-3
lines changed

2 files changed

+84
-3
lines changed

ROADMAP.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ Model name prefix now wins unconditionally over env-var presence. Regression tes
504504

505505
65. **Backlog-scanning team lanes emit opaque stops, not structured selection outcomes****done (verified 2026-04-12):** completed lane persistence in `rust/crates/tools/src/lib.rs` now recognizes backlog-scan selection summaries and records structured `selectionOutcome` metadata on `lane.finished`, including `chosenItems`, `skippedItems`, `action`, and optional `rationale`, while preserving existing non-selection and review-lane behavior. Regression coverage locks the structured backlog-scan payload alongside the earlier quality-floor and review-verdict paths. **Original filing below.**
506506

507-
66. **Completion-aware reminder shutdown missing**dogfooded 2026-04-12. Ultraclaw batch completed and was reported as done, but 10-minute cron reminder (`roadmap-nudge-10min`) kept firing into channel as if work still pending. Reminder/cron state not coupled to terminal task state. **Fix shape:** (a) cron jobs should check task completion state before firing; (b) or, provide explicit `cron.remove` on task completion; (c) or, reminders should include "work complete" detection and auto-expire. Blocker: none. Source: gaebal-gajae dogfood analysis 2026-04-12.
507+
66. **Completion-aware reminder shutdown missing****done (verified 2026-04-12):** completed lane persistence in `rust/crates/tools/src/lib.rs` now disables matching enabled cron reminders when the associated lane finishes successfully, and records the affected cron ids in `lane.finished.data.disabledCronIds`. Regression coverage locks the path where a ROADMAP-linked reminder is disabled on successful completion while leaving incomplete work untouched. **Original filing below.**
508508

509509
67. **Scoped review lanes do not emit structured verdicts****done (verified 2026-04-12):** completed lane persistence in `rust/crates/tools/src/lib.rs` now recognizes review-style `APPROVE`/`REJECT`/`BLOCKED` results and records structured `reviewVerdict`, `reviewTarget`, and `reviewRationale` metadata on the `lane.finished` event while preserving existing non-review lane behavior. Regression coverage locks both the normal completion path and a scoped review-lane completion payload. **Original filing below.**
510510

rust/crates/tools/src/lib.rs

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3764,7 +3764,8 @@ fn persist_agent_terminal_state(
37643764
.push(LaneEvent::failed(iso8601_now(), &blocker));
37653765
} else {
37663766
next_manifest.current_blocker = None;
3767-
let finished_summary = build_lane_finished_summary(&next_manifest, result);
3767+
let mut finished_summary = build_lane_finished_summary(&next_manifest, result);
3768+
finished_summary.data.disabled_cron_ids = disable_matching_crons(&next_manifest, result);
37683769
next_manifest.lane_events.push(
37693770
LaneEvent::finished(iso8601_now(), finished_summary.detail).with_data(
37703771
serde_json::to_value(&finished_summary.data)
@@ -3846,6 +3847,8 @@ struct LaneFinishedSummaryData {
38463847
selection_outcome: Option<SelectionOutcome>,
38473848
#[serde(rename = "artifactProvenance", skip_serializing_if = "Option::is_none")]
38483849
artifact_provenance: Option<ArtifactProvenance>,
3850+
#[serde(rename = "disabledCronIds", skip_serializing_if = "Vec::is_empty")]
3851+
disabled_cron_ids: Vec<String>,
38493852
}
38503853

38513854
#[derive(Debug, Clone)]
@@ -3928,6 +3931,7 @@ fn build_lane_finished_summary(
39283931
review_rationale: review_outcome.and_then(|outcome| outcome.rationale),
39293932
selection_outcome: extract_selection_outcome(raw_summary.unwrap_or_default()),
39303933
artifact_provenance,
3934+
disabled_cron_ids: Vec::new(),
39313935
},
39323936
}
39333937
}
@@ -4200,6 +4204,46 @@ fn normalize_diff_stat(value: &str) -> String {
42004204
trimmed.to_string()
42014205
}
42024206

4207+
fn disable_matching_crons(manifest: &AgentOutput, result: Option<&str>) -> Vec<String> {
4208+
let tokens = cron_match_tokens(manifest, result);
4209+
if tokens.is_empty() {
4210+
return Vec::new();
4211+
}
4212+
4213+
let mut disabled = Vec::new();
4214+
for entry in global_cron_registry().list(true) {
4215+
let haystack = format!(
4216+
"{} {}",
4217+
entry.prompt,
4218+
entry.description.as_deref().unwrap_or_default()
4219+
)
4220+
.to_ascii_lowercase();
4221+
if tokens.iter().any(|token| haystack.contains(token))
4222+
&& global_cron_registry().disable(&entry.cron_id).is_ok()
4223+
{
4224+
disabled.push(entry.cron_id);
4225+
}
4226+
}
4227+
disabled.sort();
4228+
disabled
4229+
}
4230+
4231+
fn cron_match_tokens(manifest: &AgentOutput, result: Option<&str>) -> Vec<String> {
4232+
let mut tokens = extract_roadmap_items(manifest.description.as_str())
4233+
.into_iter()
4234+
.chain(extract_roadmap_items(result.unwrap_or_default()))
4235+
.map(|item| item.to_ascii_lowercase())
4236+
.collect::<Vec<_>>();
4237+
4238+
if tokens.is_empty() && !manifest.name.trim().is_empty() {
4239+
tokens.push(manifest.name.trim().to_ascii_lowercase());
4240+
}
4241+
4242+
tokens.sort();
4243+
tokens.dedup();
4244+
tokens
4245+
}
4246+
42034247
fn derive_agent_state(
42044248
status: &str,
42054249
result: Option<&str>,
@@ -5985,7 +6029,7 @@ mod tests {
59856029
use super::{
59866030
agent_permission_policy, allowed_tools_for_subagent, classify_lane_failure,
59876031
derive_agent_state, execute_agent_with_spawn, execute_tool, final_assistant_text,
5988-
maybe_commit_provenance, mvp_tool_specs, permission_mode_from_plugin,
6032+
global_cron_registry, maybe_commit_provenance, mvp_tool_specs, permission_mode_from_plugin,
59896033
persist_agent_terminal_state, push_output_block, run_task_packet, AgentInput, AgentJob,
59906034
GlobalToolRegistry, LaneEventName, LaneFailureClass, ProviderRuntimeClient,
59916035
SubagentToolExecutor,
@@ -7945,6 +7989,43 @@ mod tests {
79457989
"deadbee"
79467990
);
79477991

7992+
let cron = global_cron_registry().create(
7993+
"*/10 * * * *",
7994+
"roadmap-nudge-10min for ROADMAP #66",
7995+
Some("ROADMAP #66 reminder"),
7996+
);
7997+
let reminder = execute_agent_with_spawn(
7998+
AgentInput {
7999+
description: "Close ROADMAP #66 reminder shutdown".to_string(),
8000+
prompt: "Finish the cron shutdown fix".to_string(),
8001+
subagent_type: Some("Explore".to_string()),
8002+
name: Some("cron-closeout".to_string()),
8003+
model: None,
8004+
},
8005+
|job| {
8006+
persist_agent_terminal_state(
8007+
&job.manifest,
8008+
"completed",
8009+
Some("Completed ROADMAP #66 after verification."),
8010+
None,
8011+
)
8012+
},
8013+
)
8014+
.expect("reminder agent should succeed");
8015+
8016+
let reminder_manifest = std::fs::read_to_string(&reminder.manifest_file)
8017+
.expect("reminder manifest should exist");
8018+
let reminder_manifest_json: serde_json::Value =
8019+
serde_json::from_str(&reminder_manifest).expect("reminder manifest json");
8020+
assert_eq!(
8021+
reminder_manifest_json["laneEvents"][1]["data"]["disabledCronIds"][0],
8022+
cron.cron_id
8023+
);
8024+
let disabled_entry = global_cron_registry()
8025+
.get(&cron.cron_id)
8026+
.expect("cron should still exist");
8027+
assert!(!disabled_entry.enabled);
8028+
79488029
let spawn_error = execute_agent_with_spawn(
79498030
AgentInput {
79508031
description: "Spawn error task".to_string(),

0 commit comments

Comments
 (0)