fix: don't postpone a parent twice on panic

CHANGELOG.md

@@ -3,6 +3,12 @@
 The format of this file is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and I do try to adhere to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+
+* `interactive manage-tasks`: a parent task postponed together with its child could be postponed once more in its own turn, and with caldav 3.6 this crashed with a "412 Precondition Failed" error.  It is now skipped, with a note.  A task changed on the server in the meantime is reloaded before it is postponed.
+
 ## v1.3.0 - 2026-10-23
 
 ### Added

plann/commands.py

@@ -511,6 +511,8 @@ def _dismiss_panic(ctx, hours_per_day, lookahead='60d'):
         click.echo("No need to panic :-)")
         return
 
+    ## a task may be postponed through its child before its own turn
+    postponed = set()
     for priority in range(9,0,-1):
         first_low_pri_tasks = []
         other_low_pri_tasks = []
@@ -560,13 +562,13 @@ def _dismiss_panic(ctx, hours_per_day, lookahead='60d'):
             ## TODO: since tasks with different priority may be affected, it's needed to restart the algorithm
             _mass_interactive_edit([x['obj'] for x in first_low_pri_tasks], default=f"postpone {default_procrastination_time}")
         else:
-            _procrastinate([x['obj'] for x in first_low_pri_tasks], procrastination_time,  check_dependent='interactive', err_callback=click.echo, confirm_callback=click.confirm)
+            _procrastinate([x['obj'] for x in first_low_pri_tasks], procrastination_time,  check_dependent='interactive', err_callback=click.echo, confirm_callback=click.confirm, postponed=postponed)
 
         if other_low_pri_tasks:
-            click.echo(f"There are {len(other_low_pri_tasks)} later pri>={priority} tasks selected which should maybe probably be considered to be postponed a bit as well")
+            click.echo(f"There are {len(other_low_pri_tasks)} later pri>={priority} tasks selected which should maybe be considered to be postponed a bit as well")
             procrastination_time = click.prompt("Push the due-date for those with ...", default='0h')
             if procrastination_time not in ('0', '0h', '0m', '0d', 0):
-                _procrastinate([x['obj'] for x in other_low_pri_tasks], procrastination_time, check_dependent='interactive', err_callback=click.echo, confirm_callback=click.confirm)
+                _procrastinate([x['obj'] for x in other_low_pri_tasks], procrastination_time, check_dependent='interactive', err_callback=click.echo, confirm_callback=click.confirm, postponed=postponed)
 
 def _split_huge_tasks(ctx, threshold='4h', max_lookahead='60d', limit_lookahead=640):
     _select(ctx=ctx, todo=True, end=f"+{max_lookahead}", limit=limit_lookahead, sort_key=['{DTSTART:?{DUE:?(0000)?}?%F %H:%M:%S}', '{PRIORITY:?0?}'])

plann/lib.py

@@ -17,6 +17,7 @@
 import click  ## TODO - this should be removed, eventually
 import icalendar
 from caldav.config import extract_conn_params_from_section
+from caldav.lib.error import ETagMismatchError, NotFoundError, ScheduleTagMismatchError, UidConflictError
 
 from plann.template import Template
 from plann.timespec import (
@@ -301,14 +302,23 @@ def _summary(obj):
 childlike = {'CHILD', 'NEXT', 'FINISHTOSTART'}
 parentlike = {'PARENT', 'FIRST', 'DEPENDS-ON', 'STARTTOFINISH'}
 
-def _procrastinate(objs, delay, check_dependent="error", with_children=False, with_family=False, with_parent=False, err_callback=print, confirm_callback=lambda x: False, recursivity=0):
+def _procrastinate(objs, delay, check_dependent="error", with_children=False, with_family=False, with_parent=False, err_callback=print, confirm_callback=lambda x: False, postponed=None, reload_on_conflict=True, recursivity=0):
     if delay in ('0', '0s', '0m', '0h', '0d', datetime.timedelta(0)):
         ## Do nothing!
         return
     assert recursivity<16 ## TODO: better error message.  Probably we have some kind of relationship loop here.
+    ## UIDs already postponed.  A task may come up again as another
+    ## object, i.e. a parent fetched by set_due and postponed with its
+    ## child.  Pass the same set to postpone several lists.
+    if postponed is None:
+        postponed = set()
     for x in objs:
         if not hasattr(x, 'set_due'):
             continue
+        uid = x.icalendar_component.get('UID')
+        if uid and uid in postponed:
+            err_callback(f"{_summary(x)} was already postponed")
+            continue
         if x.icalendar_component.get('STATUS', 'NEEDS-ACTION') == 'COMPLETED':
             continue
         if x.icalendar_component.get('RELATED-TO'):
@@ -325,15 +335,15 @@ def _procrastinate(objs, delay, check_dependent="error", with_children=False, wi
             for rel_type in parents_:
                 parents.update(parents_[rel_type])
             if parents:
-                _procrastinate(parents, delay, check_dependent, with_children, with_family, with_parent, err_callback, confirm_callback, recursivity=recursivity+1)
+                _procrastinate(parents, delay, check_dependent, with_children, with_family, with_parent, err_callback, confirm_callback, postponed=postponed, recursivity=recursivity+1)
                 continue
             else:
-                _procrastinate([x], delay, check_dependent, with_children=True, with_family=False, with_parent=False, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
+                _procrastinate([x], delay, check_dependent, with_children=True, with_family=False, with_parent=False, err_callback=err_callback, confirm_callback=confirm_callback, postponed=postponed, recursivity=recursivity+1)
                 continue
         if with_parent:
             parents = x.get_relatives(reltypes=parentlike)
             for rel_type in parents: ## Should only be PARENT as for now.
-                _procrastinate(parents[rel_type], delay, check_dependent, with_children=True, with_family=False, with_parent=False, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
+                _procrastinate(parents[rel_type], delay, check_dependent, with_children=True, with_family=False, with_parent=False, err_callback=err_callback, confirm_callback=confirm_callback, postponed=postponed, recursivity=recursivity+1)
 
         chk_parent = 'return' if check_dependent else False
         if isinstance(delay, datetime.date):
@@ -354,7 +364,7 @@ def _procrastinate(objs, delay, check_dependent="error", with_children=False, wi
                 summary = _summary(i)
                 p = parent.icalendar_component
                 if p.get('STATUS') == 'COMPLETED':
-                    _procrastinate([x], new_due, check_dependent=False, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
+                    _procrastinate([x], new_due, check_dependent=False, err_callback=err_callback, confirm_callback=confirm_callback, postponed=postponed, recursivity=recursivity+1)
                 else:
                     p_postponable = check_dependent == "interactive" and p.get('priority', 9)>2
                     p_auto_postponable = p_postponable and i.get('priority',0) <= p.get('priority', 0)
@@ -363,19 +373,38 @@ def _procrastinate(objs, delay, check_dependent="error", with_children=False, wi
                     else:
                         err_callback(f"{summary} could not be postponed due to parent {_summary(p)} with due {_ensure_ts(p['DUE'])} and priority {p.get('priority', 0)}")
                     if p_postponable and (p_auto_postponable or confirm_callback("procrastinate parent?")):
-                        _procrastinate([parent], new_due+max(parent.get_duration()+x.get_duration()+datetime.timedelta(minutes=1), datetime.timedelta(minutes=1)), check_dependent=check_dependent, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
-                        _procrastinate([x], new_due, check_dependent=check_dependent, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
+                        ## the child needs it moved, even if postponed already
+                        postponed.discard(p.get('UID'))
+                        _procrastinate([parent], new_due+max(parent.get_duration()+x.get_duration()+datetime.timedelta(minutes=1), datetime.timedelta(minutes=1)), check_dependent=check_dependent, err_callback=err_callback, confirm_callback=confirm_callback, postponed=postponed, recursivity=recursivity+1)
+                        _procrastinate([x], new_due, check_dependent=check_dependent, err_callback=err_callback, confirm_callback=confirm_callback, postponed=postponed, recursivity=recursivity+1)
             elif check_dependent == "return":
                 return parent
         else:
-            x.save()
+            try:
+                x.save()
+            except UidConflictError:
+                raise
+            except (ETagMismatchError, ScheduleTagMismatchError):
+                if not reload_on_conflict:
+                    err_callback(f"{_summary(x)} could not be saved, it keeps changing on the server")
+                    continue
+                err_callback(f"{_summary(x)} was changed on the server, reloading it")
+                try:
+                    x.load()
+                except NotFoundError:
+                    err_callback(f"{_summary(x)} was deleted on the server")
+                    continue
+                _procrastinate([x], delay, check_dependent, with_children, with_family, with_parent, err_callback, confirm_callback, postponed=postponed, reload_on_conflict=False, recursivity=recursivity+1)
+                continue
+            if uid:
+                postponed.add(uid)
         if with_children:
             ## TODO: refactor.  Make relations into a class.
             children_ = x.get_relatives(reltypes=childlike)
             children = set()
             for rel_type in children_:
                 children.update(children_[rel_type])
-            _procrastinate(children, delay, check_dependent, with_children=True, with_family=False, with_parent=False, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
+            _procrastinate(children, delay, check_dependent, with_children=True, with_family=False, with_parent=False, err_callback=err_callback, confirm_callback=confirm_callback, postponed=postponed, recursivity=recursivity+1)
 
 def _adjust_ical_relations(obj, relations_wanted={}):
     """

tests/test_lib.py

@@ -307,6 +307,139 @@ def test_procrastinate_without_relations():
             timearg = set_due_mocked.call_args[0][0]
             assert(timearg.astimezone(utc) == future+timedelta(days=10))
 
+def _todo(uid, priority=5, related_to=None):
+    t = Todo()
+    t.data = todo.replace('19970901T130000Z-123404@host.com', uid).replace('PRIORITY:2', f'PRIORITY:{priority}')
+    if related_to:
+        t.icalendar_component.add('RELATED-TO', related_to, parameters={'RELTYPE': 'PARENT'})
+    return t
+
+def test_procrastinate_parent_postponed_by_child():
+    """A parent auto-postponed together with its child is fetched
+    through get_relatives, i.e. as a separate object.  When the stale
+    copy of the parent comes up later in the list, it should not be
+    postponed (and saved) once more - the server would refuse the
+    stale copy, and without the precondition it would get postponed
+    twice."""
+    parent_fresh = _todo('parent')
+    parent_stale = _todo('parent')
+    child = _todo('child', related_to='parent')
+    with patch.object(child, 'save'), \
+         patch.object(child, 'set_due', side_effect=[parent_fresh, None]), \
+         patch.object(parent_fresh, 'save') as fresh_save, \
+         patch.object(parent_fresh, 'set_due', return_value=None), \
+         patch.object(parent_stale, 'save') as stale_save, \
+         patch.object(parent_stale, 'set_due', return_value=None) as stale_set_due:
+        postponed = set()
+        _procrastinate([child], '10d', check_dependent='interactive', err_callback=lambda x: None, postponed=postponed)
+        assert fresh_save.call_count == 1
+        ## another call, as from _dismiss_panic, sharing the set
+        _procrastinate([parent_stale], '10d', check_dependent='interactive', err_callback=lambda x: None, postponed=postponed)
+        assert stale_set_due.call_count == 0
+        assert stale_save.call_count == 0
+
+def test_procrastinate_parent_postponed_again_by_child():
+    """A parent already postponed still has to move when a child
+    needs more time"""
+    parent_fresh = _todo('parent')
+    child = _todo('child', related_to='parent')
+    with patch.object(child, 'save'), \
+         patch.object(child, 'set_due', side_effect=[parent_fresh, None]), \
+         patch.object(parent_fresh, 'save') as fresh_save, \
+         patch.object(parent_fresh, 'set_due', return_value=None):
+        postponed = {'parent'}
+        _procrastinate([child], '10d', check_dependent='interactive', err_callback=lambda x: None, postponed=postponed)
+        assert fresh_save.call_count == 1
+        assert postponed == {'parent', 'child'}
+
+def test_procrastinate_parent_postponed_by_child_same_list():
+    """As above, in one call with the default set, and the skip is
+    reported"""
+    parent_fresh = _todo('parent')
+    parent_stale = _todo('parent')
+    child = _todo('child', related_to='parent')
+    msgs = []
+    with patch.object(child, 'save'), \
+         patch.object(child, 'set_due', side_effect=[parent_fresh, None]), \
+         patch.object(parent_fresh, 'save'), \
+         patch.object(parent_fresh, 'set_due', return_value=None), \
+         patch.object(parent_stale, 'save') as stale_save:
+        _procrastinate([child, parent_stale], '10d', check_dependent='interactive', err_callback=msgs.append)
+        assert stale_save.call_count == 0
+        assert any('already postponed' in m for m in msgs)
+
+def _real_loads(load):
+    ## icalendar_component calls load(only_if_unloaded=True)
+    return [c for c in load.call_args_list if not c.kwargs.get('only_if_unloaded')]
+
+@pytest.mark.parametrize('err', ['ScheduleTagMismatchError', 'ETagMismatchError'])
+def test_procrastinate_reloads_on_precondition_failure(err):
+    """A task changed on the server since it was read is reloaded and
+    postponed from what the server has"""
+    from caldav.lib import error
+    t = _todo('changed')
+    def load(*args, **kwargs):
+        if not kwargs.get('only_if_unloaded'):
+            t.data = t.data.replace('DUE:19970416T045959Z', 'DUE:20530101T120000Z')
+    with patch.object(t, 'save', side_effect=[getattr(error, err)(), None]) as save, \
+         patch.object(t, 'load', side_effect=load) as load_mock, \
+         patch.object(t, 'set_due', return_value=None) as set_due:
+        _procrastinate([t], '10d', err_callback=lambda x: None)
+        assert len(_real_loads(load_mock)) == 1
+        assert save.call_count == 2
+        assert set_due.call_count == 2
+        ## from the reloaded due, not the stale one
+        assert set_due.call_args[0][0].astimezone(utc) == datetime(2053, 1, 11, 12, tzinfo=utc)
+
+def test_procrastinate_precondition_failure_twice():
+    """A 412 that persists after reloading is reported, not retried
+    forever"""
+    from caldav.lib import error
+    t = _todo('changed')
+    msgs = []
+    with patch.object(t, 'save', side_effect=error.ScheduleTagMismatchError()) as save, \
+         patch.object(t, 'load') as load, \
+         patch.object(t, 'set_due', return_value=None):
+        _procrastinate([t], '10d', err_callback=msgs.append)
+        assert len(_real_loads(load)) == 1
+        assert save.call_count == 2
+        assert any('could not' in m for m in msgs)
+
+def test_procrastinate_uid_conflict_not_retried():
+    from caldav.lib import error
+    t = _todo('changed')
+    with patch.object(t, 'save', side_effect=error.UidConflictScheduleTagMismatchError()), \
+         patch.object(t, 'load') as load, \
+         patch.object(t, 'set_due', return_value=None):
+        with pytest.raises(error.UidConflictError):
+            _procrastinate([t], '10d', err_callback=lambda x: None)
+        assert _real_loads(load) == []
+
+def test_procrastinate_deleted_meanwhile():
+    """A task deleted on the server since it was read is skipped"""
+    from caldav.lib import error
+    t = _todo('gone')
+    msgs = []
+    def load(*args, **kwargs):
+        if not kwargs.get('only_if_unloaded'):
+            raise error.NotFoundError()
+    with patch.object(t, 'save', side_effect=error.ETagMismatchError()), \
+         patch.object(t, 'load', side_effect=load), \
+         patch.object(t, 'set_due', return_value=None):
+        _procrastinate([t], '10d', err_callback=msgs.append)
+        assert any('deleted' in m for m in msgs)
+
+def test_procrastinate_without_uid():
+    """Tasks without a UID are not taken for one another"""
+    a = _todo('a')
+    b = _todo('b')
+    for t in (a, b):
+        t.icalendar_component.pop('UID')
+    with patch.object(a, 'save'), patch.object(a, 'set_due', return_value=None), \
+         patch.object(b, 'save') as b_save, patch.object(b, 'set_due', return_value=None):
+        _procrastinate([a, b], '10d')
+        assert b_save.call_count == 1
+
 def test_adjust_ical_relations():
     t = Todo()
     t.data = todo