Skip to content

Commit 99fe556

Browse files
committed
fix(tasks): tighten worker lookup and branch-cancel parsing
1 parent 99ea296 commit 99fe556

2 files changed

Lines changed: 96 additions & 19 deletions

File tree

src/agent/channel.rs

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ const CHANGE_COMPARISON_VERBS: &[&str] = &[
9898
"proceed with ",
9999
];
100100
const BRANCH_CANCELLED_PREFIX: &str = "Branch cancelled:";
101+
const BRANCH_CANCELLED_SENTENCE: &str = "Branch cancelled.";
101102

102103
async fn recv_channel_event(
103104
event_rx: &mut broadcast::Receiver<ProcessEvent>,
@@ -196,7 +197,7 @@ fn truncate_working_memory_summary(summary: &str) -> String {
196197
fn branch_working_memory_event_summary(
197198
conclusion: &str,
198199
) -> (crate::memory::WorkingMemoryEventType, String) {
199-
if let Some(reason) = conclusion.strip_prefix(BRANCH_CANCELLED_PREFIX) {
200+
if let Some(reason) = parse_branch_cancellation_reason(conclusion) {
200201
let reason = truncate_working_memory_summary(reason.trim());
201202
let summary = if reason.is_empty() {
202203
"Branch cancelled".to_string()
@@ -217,6 +218,17 @@ fn branch_working_memory_event_summary(
217218
)
218219
}
219220

221+
fn parse_branch_cancellation_reason(conclusion: &str) -> Option<&str> {
222+
let trimmed = conclusion.trim();
223+
if let Some(rest) = trimmed.strip_prefix(BRANCH_CANCELLED_PREFIX) {
224+
return Some(rest);
225+
}
226+
if let Some(rest) = trimmed.strip_prefix(BRANCH_CANCELLED_SENTENCE) {
227+
return Some(rest);
228+
}
229+
None
230+
}
231+
220232
fn sentence_contains_decision_marker(sentence: &str) -> bool {
221233
let sentence_lower = sentence.to_ascii_lowercase();
222234
DECISION_MARKERS
@@ -530,9 +542,9 @@ impl ChannelState {
530542

531543
let reason = crate::summarize_first_non_empty_line(reason, crate::EVENT_SUMMARY_MAX_CHARS);
532544
let conclusion = if reason.is_empty() {
533-
"Branch cancelled.".to_string()
545+
BRANCH_CANCELLED_SENTENCE.to_string()
534546
} else {
535-
format!("Branch cancelled: {reason}")
547+
format!("{BRANCH_CANCELLED_PREFIX} {reason}")
536548
};
537549
self.process_run_logger
538550
.log_branch_completed(branch_id, &conclusion);
@@ -3299,7 +3311,7 @@ impl Channel {
32993311
// Regular branch: accumulate result for the next retrigger.
33003312
// The result text will be embedded directly in the retrigger
33013313
// message so the LLM knows exactly which process produced it.
3302-
let branch_success = !conclusion.starts_with(BRANCH_CANCELLED_PREFIX);
3314+
let branch_success = parse_branch_cancellation_reason(conclusion).is_none();
33033315
self.pending_results.push(PendingResult {
33043316
process_type: "branch",
33053317
process_id: branch_id.to_string(),
@@ -4326,6 +4338,14 @@ mod tests {
43264338
assert_eq!(summary, "Branch cancelled: superseded by user request");
43274339
}
43284340

4341+
#[test]
4342+
fn branch_working_memory_event_records_sentence_cancellation_as_error() {
4343+
let (event_type, summary) = branch_working_memory_event_summary("Branch cancelled.");
4344+
4345+
assert_eq!(event_type, WorkingMemoryEventType::Error);
4346+
assert_eq!(summary, "Branch cancelled");
4347+
}
4348+
43294349
#[test]
43304350
fn quiet_mode_invocation_uses_discord_mention_and_reply_metadata() {
43314351
let message = inbound_message(

src/tasks/store.rs

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -428,32 +428,36 @@ impl TaskStore {
428428
.await
429429
.context("failed to open worker task update transaction")?;
430430

431-
let row = sqlx::query(&format!(
432-
"{SELECT_COLUMNS} FROM tasks WHERE worker_id = ? ORDER BY updated_at DESC LIMIT 1"
431+
let exact_row = sqlx::query(&format!(
432+
"{SELECT_COLUMNS} FROM tasks WHERE worker_id = ? AND task_number = ?"
433433
))
434434
.bind(worker_id)
435+
.bind(task_number)
435436
.fetch_optional(&mut *tx)
436437
.await
437-
.context("failed to fetch task by worker id for update")?;
438+
.context("failed to fetch worker task by id and number for update")?;
439+
440+
let Some(row) = exact_row else {
441+
let assigned_task_number = sqlx::query_scalar::<_, i64>(
442+
"SELECT task_number FROM tasks WHERE worker_id = ? ORDER BY task_number DESC LIMIT 1",
443+
)
444+
.bind(worker_id)
445+
.fetch_optional(&mut *tx)
446+
.await
447+
.context("failed to fetch any task by worker id for update")?;
438448

439-
let Some(row) = row else {
440449
tx.commit()
441450
.await
442-
.context("failed to commit empty worker task update transaction")?;
451+
.context("failed to commit unmatched worker task update transaction")?;
452+
if let Some(assigned_task_number) = assigned_task_number {
453+
return Ok(WorkerTaskUpdateResult::WrongTask {
454+
assigned_task_number,
455+
});
456+
}
443457
return Ok(WorkerTaskUpdateResult::NotAssigned);
444458
};
445459

446460
let current = task_from_row(row)?;
447-
if current.task_number != task_number {
448-
let assigned_task_number = current.task_number;
449-
tx.commit()
450-
.await
451-
.context("failed to commit rejected worker task update transaction")?;
452-
return Ok(WorkerTaskUpdateResult::WrongTask {
453-
assigned_task_number,
454-
});
455-
}
456-
457461
let previous_status = current.status;
458462
let task = Self::update_current_in_tx(&mut tx, task_number, current, input).await?;
459463

@@ -928,6 +932,59 @@ mod tests {
928932
assert_eq!(result.task.status, TaskStatus::Done);
929933
}
930934

935+
#[tokio::test]
936+
async fn update_worker_task_prefers_exact_match_with_duplicate_worker_bindings() {
937+
let store = setup_store().await;
938+
let first = store
939+
.create(self_assigned_input("first task", TaskStatus::InProgress))
940+
.await
941+
.expect("first task should be created");
942+
let second = store
943+
.create(self_assigned_input("second task", TaskStatus::InProgress))
944+
.await
945+
.expect("second task should be created");
946+
947+
let shared_worker_id = "worker-shared";
948+
store
949+
.update(
950+
first.task_number,
951+
UpdateTaskInput {
952+
worker_id: Some(shared_worker_id.to_string()),
953+
..Default::default()
954+
},
955+
)
956+
.await
957+
.expect("first worker binding should update");
958+
store
959+
.update(
960+
second.task_number,
961+
UpdateTaskInput {
962+
worker_id: Some(shared_worker_id.to_string()),
963+
..Default::default()
964+
},
965+
)
966+
.await
967+
.expect("second worker binding should update");
968+
969+
let result = store
970+
.update_worker_task(
971+
shared_worker_id,
972+
first.task_number,
973+
UpdateTaskInput {
974+
metadata: Some(serde_json::json!({"target": "first"})),
975+
..Default::default()
976+
},
977+
)
978+
.await
979+
.expect("worker-scoped update should succeed");
980+
981+
let WorkerTaskUpdateResult::Updated(result) = result else {
982+
panic!("expected exact task update despite duplicate worker bindings");
983+
};
984+
assert_eq!(result.task.task_number, first.task_number);
985+
assert_eq!(result.task.metadata["target"], "first");
986+
}
987+
931988
#[tokio::test]
932989
async fn can_requeue_in_progress_and_clear_worker_binding() {
933990
let store = setup_store().await;

0 commit comments

Comments
 (0)