[codex] fix routing and stale mission-help cleanup#90
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Reviewed by Cursor Bugbot for commit 10ae7f5. Configure here.
| if let Err(e) = storage | ||
| .log_event( | ||
| mission_id, | ||
| &format!("Operator note queued via MCP: {}", input.message), |
There was a problem hiding this comment.
MCP note handler omits clear_help_request_state unlike other paths
Medium Severity
The MCP mission.note handler discards the loaded mission with Ok(Some(_)) => {} and never calls clear_help_request_state(), while every other operator acknowledgment path (CLI note, MCP pause/resume/stop, API resume/stop, and MCP reject) does call it and saves the mission. This means operator notes sent via MCP won't reset the dispatcher's remembered help-request state, so the dispatcher may re-send stale help requests to the conductor inbox even though the operator already responded.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 10ae7f5. Configure here.
| } | ||
|
|
||
| let raw_lower = raw.to_ascii_lowercase(); | ||
| let agents = self.channel.list_agents().await?; |
There was a problem hiding this comment.
Ambiguous agent ref now errors instead of returning None
High Severity
The dispatcher's resolve_agent_ref now delegates to channel.resolve_agent_ref, which returns Err on ambiguous matches. The old dispatcher code returned Ok(None) for ambiguity, which was gracefully handled at the call site (logged and skipped via continue). Now the Err propagates through ? at the call site, up through process_control_messages, and out of tick(), aborting the entire dispatcher tick for all claimed missions instead of just skipping one bad directive.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 10ae7f5. Configure here.
| let _: () = conn.del(&inbox_key).await?; | ||
| if !kept.is_empty() { | ||
| let _: () = conn.rpush(&inbox_key, kept).await?; | ||
| } |
There was a problem hiding this comment.
Non-atomic inbox read-delete-write loses concurrent messages
Medium Severity
remove_inbox_messages_matching reads the full inbox via lrange, then deletes the key and re-pushes kept messages. This is not atomic—any message pushed to the inbox between the lrange and the del will be silently lost. Since the dispatcher can send help requests to the conductor inbox concurrently with an operator action that triggers this cleanup, the race window is real even if small.
Reviewed by Cursor Bugbot for commit 10ae7f5. Configure here.
1 participant
What changed
[Mission Help Needed]conductor inbox items when an operator acknowledges a mission via note, pause, resume, or stop flowsWhy it changed
Issue #89 described three operator-facing rough edges during live mission supervision:
tt sendand related paths failed when operators or agents used the prominent nickname/display label instead of the canonical mailbox nameUser impact
tt statusandtt listRoot cause
Validation
cargo test test_agent_lookup_accepts_short_id_nickname_and_display_label -- --nocapturecargo test test_retire_help_requests_for_mission_removes_only_matching_queries -- --nocapturecargo test --no-runCloses #89.
Note
Medium Risk
Medium risk because it changes mission control flows (pause/resume/stop/note) to mutate mission state and prune supervisor inbox messages, which could affect operator workflows if matching is too broad or persistence fails. Agent lookup semantics also change to accept multiple identifiers and to error on ambiguous matches.
Overview
Improves operator-facing routing by introducing a shared
Channel::resolve_agent_refthat accepts UUIDs (full/short), canonical names, nicknames, and full display labels, and re-wiresTown::agentand dispatcher resolution to use it (with explicit ambiguity errors).Adds mission help-request cleanup: new mission helpers (
is_help_request_message_for_mission,retire_help_requests_for_mission) plusChannel::remove_inbox_messages_matchingare used across CLI/API/MCP mission note/pause/resume/stop paths to clear stale[Mission Help Needed]prompts and reset per-mission dispatcher help-request state viaMissionRun::clear_help_request_state.CLI output now warns when terminal agents are still listed and suggests running
tt prune, and new integration tests cover agent ref resolution and help-request retirement behavior.Reviewed by Cursor Bugbot for commit 10ae7f5. Bugbot is set up for automated code reviews on this repo. Configure here.