feat(manip): restore reset, and expose cancel as a skill - #3872
Conversation
reset was removed in #3447 but FAULT stayed, and planning only runs from IDLE or COMPLETED, so a faulted module accepted nothing further. Both manipulator agent prompts still instruct the model to call reset after a failure, one of them describing it as available as a skill and an RPC, so an agent that faulted was told to call a tool that did not exist. A Drake-gated test in test_manipulation_module.py calls it too. cancel already stops the trajectory, bumps the planning epoch, drops the pending plan and dismisses its preview, so reset delegates to it and adds only the unconditional return to IDLE with the error cleared. Resetting cancels an active motion rather than refusing one: telling a caller to cancel first is a dead end for anyone holding only the skill surface, where cancel was never exposed. It is now.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## mb/grasp-04-wrist-camera-tf #3872 +/- ##
===============================================================
+ Coverage 77.65% 77.72% +0.06%
===============================================================
Files 1324 1324
Lines 125452 125488 +36
Branches 10911 10912 +1
===============================================================
+ Hits 97422 97537 +115
+ Misses 24889 24865 -24
+ Partials 3141 3086 -55
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Greptile SummaryThis change restores manipulation reset, exposes cancellation and reset as agent skills, and expands recovery coverage. Runtime checks reproduced two unsafe recovery behaviors: reset can permit a new trajectory after the previous stop was not confirmed, and the cancellation skill can report that same unconfirmed stop as successful. These paths should be corrected before merging. Confidence Score: 3/5Not safe to merge until uncertain cancellation outcomes remain faulted and are exposed as failures to callers. Focused runtime harnesses exercised both uncertain-cancellation paths and observed redispatch after reset as well as a successful skill result while the module remained faulted. Files Needing Attention:
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| cancelled = self.cancel().status is not ExecutionStatus.NO_EXECUTION | ||
| with self._lock: | ||
| self._state = ManipulationState.IDLE | ||
| self._error_message = "" |
There was a problem hiding this comment.
reset() invokes cancel() but unconditionally clears the error and restores IDLE, including when cancellation returned UNCERTAIN. That outcome means the coordinator could not confirm that the active trajectory stopped. Restoring IDLE reopens the planning gate, allowing a new trajectory to be dispatched while the earlier one may still be moving. Preserve FAULT and reject planning until a deterministic stopped or terminal result is available.
Artifacts
Focused uncertain-cancellation reset harness
- This executable harness injects a coordinator UNCERTAIN cancellation and asserts the resulting reset, planning admission, and new-dispatch behavior, with the takeaway that it exercises the exact claimed safety path.
Observed uncertain-cancellation reset execution
- This captured command output records UNCERTAIN cancellation followed by IDLE reset, PLANNING admission, and ACCEPTED redispatch with exit code 0, with the takeaway that the defect is confirmed at runtime.
| @skill | ||
| def cancel(self) -> SkillResult[ManipulationSkillError]: | ||
| """Stop the active motion or planning attempt, leaving the arm where it is.""" | ||
| return SkillResult.ok(self.manipulation.cancel().message or "Cancelled") |
There was a problem hiding this comment.
The module transitions to FAULT when the coordinator returns UNCERTAIN, but this skill always returns SkillResult.ok. An agent branching on the skill result can therefore continue as though the arm stopped even though the stop was not confirmed and the module requires recovery. Return a failed or explicitly uncertain skill result for UNCERTAIN and other faulted cancellation outcomes.
Artifacts
Focused uncertain cancellation skill runtime harness
- Authored harness that creates an executing manipulation module, makes the coordinator return UNCERTAIN, and exercises both the module and skill paths; it provides the reproducible defect trigger.
Underlying module cancellation output for an uncertain coordinator response
- Executed module-level cancellation capture showing `ExecutionStatus.UNCERTAIN` and module state `FAULT`; it establishes the authoritative unsafe cancellation state before the skill wrapper.
Cancellation skill output for the same uncertain coordinator response
- Executed skill-level cancellation capture showing `skill_success=True` with no error code while the underlying module remains in `FAULT`; it confirms the unsafe success translation.
1 participant
resetwas removed in #3447 butFAULTstayed, and planning only runs fromIDLEorCOMPLETED, so a faulted module accepted nothing further. Both manipulator agent prompts still instruct the model to callresetafter a failure, one of them describing it as available as a skill and an RPC, so an agent that faulted was told to call a tool that did not exist.test_state_transitions_during_executioncalls it too.cancelalready stops the trajectory, bumps the planning epoch, drops the pending plan and dismisses its preview, soresetdelegates to it and adds only the unconditional return toIDLEwith the error cleared — that is the whole of the difference between the two verbs, and writing it that way keeps it that way. Resetting cancels an active motion rather than refusing one: telling a caller to cancel first is a dead end for anyone holding only the skill surface, wherecancelwas never exposed. It is now.Fifth of nine in the xArm grasping re-landing stack.