[Bug]: Task::Active::rewind() never rewinds, and livelocks the fleet adapter for any phase_id except the earliest completed one

[HappySamuel]

Before proceeding, is there an existing issue or discussion for this?

• I have done a search for similar issues and discussions.

OS and version

Ubuntu 24.04

Open-RMF installation type

Binaries

Other Open-RMF installation methods

No response

Open-RMF version or commit hash

2.7.2-1noble.20260615.164301

ROS distribution

Jazzy

ROS installation type

Binaries

Other ROS installation methods

No response

Package or library, if applicable

No response

Description of the bug

Task::Active::rewind() in rmf_task_sequence/src/rmf_task_sequence/Task.cpp:617-650
contains two independent defects. Together they mean rewind_task has never worked,
and one of them takes the whole fleet adapter down.

void Task::Active::rewind(uint64_t phase_id)
{
  std::lock_guard lock(_next_phase_mutex);
  assert(_completed_phases.size() == _completed_stages.size());
  std::size_t completed_index = 0;
  auto stage_it = _completed_stages.begin();
  while (stage_it != _completed_stages.end())
  {
    if ((*stage_it)->id == phase_id)
      break;

    ++completed_index;          // <-- (1) stage_it is never incremented
  }

  if (stage_it == _completed_stages.end())
    return;

  _pending_stages.insert(
    _pending_stages.begin(),
    stage_it,
    _completed_stages.begin()); // <-- (2) empty range; should be .end()

  _pending_stages.push_back(_active_stage);
  _generate_pending_phases();
  _active_phase->cancel();
}

Defect 1 — the search loop never advances its iterator. _completed_stages is a
std::list; only completed_index is incremented, so stage_it stays pinned to
begin(). The loop can therefore only exit through the break, and only when the
first completed stage matches. For any other phase_id the condition
stage_it != _completed_stages.end() is permanently true and the function spins
forever while holding _next_phase_mutex.

Because this runs on the fleet adapter's executor thread, the entire adapter
livelocks: one core pegged at 100%, task states stop publishing, /fleet_states
goes silent, and newly dispatched tasks fail because the fleet can no longer bid.
Critically the process stays alive, so systemd/supervisors do not restart it —
arguably worse operationally than a crash.

Defect 2 — the insert range is empty. [stage_it, _completed_stages.begin())
is empty whenever stage_it == begin() (which, given defect 1, is always), and
would be a reversed/invalid range if the iterator ever did advance. The bound
should be _completed_stages.end(). Consequently no completed stage is ever put
back into pending, so even the one phase_id that returns does not rewind
anything.

Two supporting signs that this function has never been executed:

• completed_index is computed and never read.
• _completed_stages / _completed_phases are never trimmed, so the
  assert on line 620 would be left inconsistent even if the iteration were fixed.

Also note push_back(_active_stage) appends the active stage to the end of
pending, rather than immediately after the restored stages where it belongs.

Steps to reproduce the bug

Stock rmf_demos office demo, no customisation required.

1. Launch the office demo:

   ros2 launch rmf_demos_gz office.launch.xml headless:=1
   

2. Dispatch a multi-phase compose task:

   {
     "category": "compose",
     "description": {
       "category": "patrol_demo",
       "phases": [
         { "activity": { "category": "go_to_place", "description": "pantry" } },
         { "activity": { "category": "go_to_place", "description": "lounge" } },
         { "activity": { "category": "go_to_place", "description": "hardware_2" } },
         { "activity": { "category": "go_to_place", "description": "supplies" } },
         { "activity": { "category": "go_to_place", "description": "coe" } }
       ]
     }
   }

3. Wait until at least two phases have completed (e.g. completed=[1,2,3],
   phase 4 active).

4. Case A — livelock. Rewind to any completed phase that is not the earliest:

   curl -X POST 'http://localhost:8000/tasks/rewind_task' \
     -H 'Content-Type: application/json' \
     -d '{"type":"rewind_task_request","task_id":"<task_id>","phase_id":2}'
   

   No response is ever returned. top -H -p <fleet_adapter_pid> shows one thread
   at 100% CPU; /proc/<pid>/task/<tid>/ shows state=R, wchan=0, no syscall
   (a userspace spin, not a lock wait). ros2 topic echo /fleet_states goes
   silent, and any newly dispatched task ends up failed.

5. Case B — silent no-op. Restart the adapter, repeat to the same point, then
   rewind to the earliest completed phase ("phase_id": 1). It returns
   {"success":true}, but the task does not rewind: the active phase is merely
   cancelled and re-queued at the very end of the task.

Expected behavior

Per rewind_task_request.json:

"Specify the phase that should be rewound to. The task will restart at the
beginning of this phase."

and rmf_task/include/rmf_task/Task.hpp:341:

"Rewind the Task to a specific phase. This can be issued by operators if a phase
did not actually go as intended and needs to be repeated."

For completed=[1,2,3] with phase 4 active, rewind_task(phase_id=2) should
requeue phases 2 and 3 followed by the cancelled phase 4, giving execution order
2 → 3 → 4 → 5, with completed trimmed back to [1]. The request should return
promptly and the fleet adapter should remain fully responsive.

Actual behavior

• phase_id = earliest completed phase: returns {"success":true}, but no rewind
  occurs. The active phase is cancelled and appended to the tail of pending, so
  execution order becomes 4 → 5 → 3 instead of 1 → 2 → 3 → 4 → 5.
• phase_id = any other completed phase: no response at all; the fleet adapter
  livelocks at 100% CPU and stops serving every robot in the fleet until it is
  manually killed. The process does not exit, so it is not auto-restarted.

Additional information or screenshots

Affected branches — both defects are present, unmodified, on every branch:

branch	rmf_task_sequence
humble	2.1.8
iron	2.2.5
jazzy	2.5.1
kilted	2.7.0
lyrical	2.10.0
rolling	2.1.3
main	2.10.0

Line numbers are identical on jazzy and main (function at 617, faulty insert
bound at 641), i.e. the code is untouched since it was introduced.

Reproduced on ROS 2 Jazzy, Ubuntu 24.04, binaries:
ros-jazzy-rmf-task 2.5.1-1noble, ros-jazzy-rmf-fleet-adapter 2.7.2-1noble.

Candidate fix (verified in simulation — happy to open a PR):

   std::size_t completed_index = 0;
   auto stage_it = _completed_stages.begin();
   while (stage_it != _completed_stages.end())
   {
     if ((*stage_it)->id == phase_id)
       break;
 
+    ++stage_it;
     ++completed_index;
   }
 
   if (stage_it == _completed_stages.end())
   {
     // TODO(MXG): Indicate to the user that they asked to rewind to a phase
     // that hasn't been reached yet.
     return;
   }
 
+  // The currently active stage goes back into pending, to run again after the
+  // rewound stages have been repeated.
+  _pending_stages.push_front(_active_stage);
+
+  // Queue the completed stages from phase_id onwards ahead of it. This must
+  // happen before the erase below, since stage_it points into _completed_stages.
   _pending_stages.insert(
     _pending_stages.begin(),
     stage_it,
-    _completed_stages.begin());
+    _completed_stages.end());
+
+  // The rewound phases are no longer completed. _completed_stages and
+  // _completed_phases are appended in lockstep by _finish_phase, so
+  // completed_index applies to both.
+  _completed_stages.erase(stage_it, _completed_stages.end());
+  _completed_phases.erase(
+    _completed_phases.begin() + completed_index, _completed_phases.end());
 
-  // The currently active stage should also be put back into pending
-  _pending_stages.push_back(_active_stage);
   _generate_pending_phases();
 
   _active_phase->cancel();

Verification with the patch applied, office demo, 5-phase compose task:

Trigger state	Request	Result	Executed
completed=[1,2,3], phase 4 active	rewind→2	active=2, pending=[3,4,5], completed=[1]	2→3→4→5
completed=[1,2,4], phase 5 active	rewind→4	active=4, pending=[5], completed=[1,2]	4→5
phase not yet reached / unknown id	rewind	clean no-op	—

Every request that previously livelocked now returns in ~2s, no spinning thread
appears, and tasks rewound twice still run to completion.

Related but distinct: rewinding to a phase that has not been reached, and
rewinding to the currently-active phase, are both silent no-ops that still return
{"success":true} (the existing TODO(MXG) acknowledges the first). Reporting
those properly would need a signature change to the void rewind() virtual, so
they are out of scope here.

Cross-reference: open-rmf/rmf_ros2#542 / open-rmf/rmf_ros2#543 fix a different
bug on the same operator-controls path (the phases[] JSON key type in
TaskManager::publish_task_state()). That fix is required to skip/rewind at all
without crashing, but it does not touch the logic reported here.