fix(channel): use worker_handles as guard for WorkerComplete events

[ciaranashton]

Summary

• Workers completed successfully but their results were never relayed back to the channel
• The WorkerComplete event handler in channel.rs guarded on active_workers.remove(), but active_workers (HashMap<WorkerId, Worker>) was never populated — the Worker struct is consumed by .run() when spawned, so it can't be stored
• Every WorkerComplete event was silently dropped at the early return
• Fixed by switching the guard to worker_handles, which is the actual source of truth for running workers (populated by spawn_worker_task, also used by cancel_worker_with_reason)

Test plan

• Tested locally — spawned workers now report results back to the channel agent
• Verify retrigger fires and Slack receives the worker result
• Verify cancel_worker still works correctly

🤖 Generated with Claude Code

[tembo]

Minor robustness tweak: the early return on missing worker_handles skips cleanup of worker_inputs (and any future active_workers usage) if we ever see duplicate/late WorkerComplete events. Consider always cleaning those up, and reword the comment to avoid the absolute “never populated” claim.

Suggested change

	// Use worker_handles as the source of truth for active workers.
	// (active_workers is never populated because Worker is consumed by .run())
	if self.state.worker_handles.write().await.remove(worker_id).is_none() {
	return Ok(());
	}
	drop(workers);
	run_logger.log_worker_completed(*worker_id, result, *success);
	self.state.worker_handles.write().await.remove(worker_id);
	self.state.active_workers.write().await.remove(worker_id);
	self.state.worker_inputs.write().await.remove(worker_id);
	// `worker_handles` is the source of truth for running workers.
	let removed_handle = self.state.worker_handles.write().await.remove(worker_id);
	self.state.active_workers.write().await.remove(worker_id);
	self.state.worker_inputs.write().await.remove(worker_id);
	if removed_handle.is_none() {
	return Ok(());
	}
	run_logger.log_worker_completed(*worker_id, result, *success);

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 223885c5-6e60-43b4-a153-75d72a1be7fe

📥 Commits

Reviewing files that changed from the base of the PR and between 2445032 and b99c273.

📒 Files selected for processing (1)

• src/agent/channel.rs

---

Walkthrough

Use worker_handles as the canonical source for worker presence when handling ProcessEvent::WorkerComplete; update removal and input cleanup ordering. Add unit tests validating channel-scoped WorkerComplete filtering and channel_id propagation from spawned worker tasks.

Changes

Cohort / File(s)	Summary
Worker lifecycle handling src/agent/channel.rs	ProcessEvent::WorkerComplete now checks presence in worker_handles and returns early if absent; removes from worker_handles, then updates active_workers and clears worker_inputs for the worker.
Tests: channel filtering & propagation src/agent/channel.rs, src/agent/channel_dispatch.rs	Added three tests for WorkerComplete channel filtering (worker_complete_event_matches_own_channel, worker_complete_event_ignored_for_other_channel, worker_complete_event_ignored_when_no_channel) and one test (spawn_worker_task_carries_channel_id) asserting spawned worker task carries channel_id in its WorkerComplete event.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: make background result retriggers deterministic #231 — Modifies src/agent/channel.rs around worker completion and channel handling; likely related to routing/metadata changes for WorkerComplete events.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing WorkerComplete event handling by using worker_handles as the guard instead of active_workers.
Description check	✅ Passed	The description is directly related to the changeset, explaining the bug, root cause, and solution with concrete details about the code changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jamiepine]

I broke this on my last PR but my internet went out so I couldn't fix it before sleep, thank you!