tmbx: the description refusal skips a foreign block, like the one above it (#399)

docs/architecture/materials-and-work-links.md

@@ -87,13 +87,43 @@ per linked block:
   itself.
 
 `_dead_link_violation` (`src/tmbx/service.py`) refuses the whole commit if
-any block's handle is not in the material store, naming the block and the
-handle. This is stricter than the check at `apply` time, which lets a block
-the patch never touched keep a handle whose row is already gone — refusing
-there would hide the rest of the day from whoever has to fix it. At commit
-there is nothing left to guess from: a handle with no row means no URL to
-write, so the commit is refused until that block's link is cleared with an
-explicit null.
+a linked block's handle is not in the material store, naming the block and
+the handle — unless the block is one tmbx does not own, which this check
+skips (see below). This is stricter than the check at `apply` time, which
+lets a block the patch never touched keep a handle whose row is already
+gone — refusing there would hide the rest of the day from whoever has to
+fix it. At commit there is nothing left to guess from: a handle with no row
+means no URL to write, so the commit is refused until that block's link is
+cleared with an explicit null.
+
+`_long_description_violation` (`src/tmbx/service.py`) refuses the whole
+commit if a linked block's description is over `MAX_DESCRIPTION_CHARS`
+(`src/tmbx/calendar/port.py`). The limit falls out of the composition
+above: a linked event's *displayed* description has the material's URL
+appended to it, so the text a person actually wrote has to survive
+somewhere else — the `tmbx.desc` private property covered below — and that
+property is length-capped by the provider. Over the limit there is nowhere
+left to put the authored text, so the commit refuses rather than
+truncating it: silently shortening a description loses what somebody
+wrote, and the loss would only surface the next time the day was read
+back. The check sits here, in the service, before anything is written,
+rather than in the adapter that owns the limit (`_private_properties` in
+`src/tmbx/calendar/gcal.py`) — the adapter can only raise from inside the
+commit's event loop, past the journal and with some events already
+written, which would leave a half-committed day with no journal entry to
+explain it. The adapter keeps its own raise as a backstop for any caller
+that reaches it without going through the service.
+
+**Neither refusal checks a block tmbx does not own.** The remedy each one
+names — clear the link, or shorten the description — is a write to the
+foreign event, and `_foreign_touches` refuses exactly that: tmbx must
+never write an event it doesn't own. Checking a foreign block here would
+refuse every commit of that day forever, with no way to comply. #398
+added the skip to `_dead_link_violation`; #399, a release later, added
+the identical skip to `_long_description_violation`, for the same reason.
+The two refusals sit one screen apart in `service.py` and share this
+deadlock, so a filter added to either belongs on both — worth checking
+for, whoever adds a third commit-time refusal next.
 
 ## The private map merges, so a cleared key is sent empty, not omitted
 
@@ -106,6 +136,11 @@ back as absence. This applies to every key that predates links as much as
 to `tmbx.link` itself: a required-kind slug removed from a block, or an
 authored description that had changed, previously stayed on the event under
 the old value and could silently win over what the user actually wrote.
+`_write_event_args` (`src/tmbx/calendar/gcal.py`) used to guard this map
+behind `if private:`, which read as though the map were sometimes left
+out; it never was, since `_private_properties` returns a fixed eight-entry
+dict that is never empty, so the guard could not fail. The guard is gone
+now and the code matches what this section has always described.
 
 ## Undo restores the link, not just the block
 

src/fateforger/slack_bot/timeboxing_host.py

@@ -292,9 +292,18 @@ async def resolve(
             self._work_refs(snapshot, day),
             return_exceptions=True,
         )
-        for settled in (constraints, work):
-            if isinstance(settled, BaseException):
-                raise settled
+        # Narrowed one name at a time, rather than by looping over the pair:
+        # a loop narrows only its own variable, so `constraints` and `work`
+        # stayed `T | BaseException` for the reader and for a type checker,
+        # and `work.facts` below is an attribute access on that union. Runtime
+        # was always correct -- the loop does raise -- but nothing gates this
+        # path today, so the union was load-bearing on no one noticing.
+        # Constraints first keeps the order a caller saw when these ran in
+        # sequence, which is what the comment above promises.
+        if isinstance(constraints, BaseException):
+            raise constraints
+        if isinstance(work, BaseException):
+            raise work
         return PlanningContext(
             facts=[
                 *planning_facts(

src/tmbx/calendar/gcal.py

@@ -494,27 +494,34 @@ def _write_event_args(event: CalendarEvent, *, tz: str) -> dict[str, Any]:
     not a nested ``{dateTime, timeZone}`` object; that nested shape shows
     up only in this repo's throwaway dev seed scripts, not the tool's
     actual schema. ``extendedProperties.private`` carries identity plus
-    ``block_type``/``timing_mode``/``anchor_source``/``link``/``desc`` —
-    see the module docstring — and is included only when at least one of
-    those eight is set, since a foreign event is never written here at all
-    (the service never calls create/update for one).
+    ``block_type``/``timing_mode``/``anchor_source``/``link``/``desc`` — see
+    the module docstring — and is sent **always, with all eight keys**, even
+    when every one of them is empty.
+
+    That last part is the correction, not a detail. This said the map was
+    "included only when at least one of those eight is set", and a guard here
+    implemented it. Both stopped being true when ``_private_properties`` began
+    writing ``None`` as an empty string rather than dropping the key: it now
+    returns a fixed eight-entry map that is never falsy, so the guard could
+    not fail and the sentence described a behaviour nothing performed. An
+    event whose identity was cleared is precisely the one that must say so on
+    the wire — the server *merges* this map, so a key left out keeps its last
+    value — and ``test_an_event_with_no_identity_sends_every_key_empty`` is
+    what holds that.
 
     The description sent is not ``event.description`` verbatim: a linked
     event's url is appended to it (``_description_for``), and the authored
     text goes into ``tmbx.desc`` so the composition can be reversed on the
     way back (``_private_properties``).
     """
-    private = _private_properties(event)
-    args: dict[str, Any] = {
+    return {
         "summary": event.summary,
         "description": _description_for(event),
         "start": event.start.isoformat(timespec="seconds"),
         "end": event.end.isoformat(timespec="seconds"),
         "timeZone": tz,
+        "extendedProperties": {"private": _private_properties(event)},
     }
-    if private:
-        args["extendedProperties"] = {"private": private}
-    return args
 
 
 def _normalize_events(payload: Any) -> list[dict[str, Any]]:

src/tmbx/service.py

@@ -678,7 +678,9 @@ def _unknown_link_violation(
     )
 
 
-def _long_description_violation(plan: Plan) -> Violation | None:
+def _long_description_violation(
+    plan: Plan, foreign_handles: Collection[str]
+) -> Violation | None:
     """The refusal for a linked block whose description cannot be round-tripped.
 
     ``None`` when every linked block fits. A provider keeps a linked block's
@@ -695,6 +697,17 @@ def _long_description_violation(plan: Plan) -> Violation | None:
     committing. So this is a cost of attaching a ticket, and the message says
     which block to shorten.
 
+    **A foreign block is not checked**, for the reason ``_dead_link_violation``
+    skips one directly above: ``_write`` never touches a foreign event, so
+    there is no description to round-trip and nothing is half-answered — and
+    both remedies this message offers, shortening the description and dropping
+    the link, are updates to a foreign handle, which ``_foreign_touches``
+    refuses. Checking one would refuse every commit of that day forever with
+    no way out. The filter arrived here a release after the one below it,
+    which is the whole reason this paragraph is here: the two refusals sit one
+    screen apart and share a deadlock, so a filter added to either belongs on
+    both.
+
     It lives here, before the write, rather than in the adapter that owns the
     limit, because the adapter can only raise from inside the commit's event
     loop — past the journal, with some events already written and the delete
@@ -706,7 +719,9 @@ def _long_description_violation(plan: Plan) -> Violation | None:
     too_long = [
         block
         for block in plan.blocks
-        if block.link is not None and len(block.d) > MAX_DESCRIPTION_CHARS
+        if block.link is not None
+        and len(block.d) > MAX_DESCRIPTION_CHARS
+        and block.h not in foreign_handles
     ]
     if not too_long:
         return None
@@ -1082,7 +1097,7 @@ async def _commit_once(
             dead = _dead_link_violation(patched, materials, foreign_handles)
             if dead is not None:
                 raise PlanViolation(dead)
-            too_long = _long_description_violation(patched)
+            too_long = _long_description_violation(patched, foreign_handles)
             if too_long is not None:
                 raise PlanViolation(too_long)
         except PlanViolation as exc:

tests/unit/tmbx/test_link_reaches_the_calendar.py

@@ -467,7 +467,12 @@ def test_max_description_chars_is_exported_by_the_port():
 
 
 def _foreign_event(
-    eid: str, start_h: int, end_h: int, *, link_id: str | None = None
+    eid: str,
+    start_h: int,
+    end_h: int,
+    *,
+    link_id: str | None = None,
+    description: str = "",
 ) -> CalendarEvent:
     """A calendar event tmbx did not write: no ``uid``, so no ownership.
 
@@ -479,6 +484,7 @@ def _foreign_event(
     return CalendarEvent(
         event_id=eid,
         summary="Standup",
+        description=description,
         start=datetime(2026, 8, 17, start_h, 0),
         end=datetime(2026, 8, 17, end_h, 0),
         etag="v1",
@@ -567,6 +573,104 @@ async def test_an_owned_blocks_dead_handle_still_refuses(tmp_path, materials):
     assert DEAD_HANDLE in str(excinfo.value)
 
 
+async def test_a_foreign_blocks_long_description_does_not_refuse_the_day(
+    tmp_path, materials, known
+):
+    """The same deadlock, one screen below the one that was fixed.
+
+    ``_dead_link_violation`` learned to skip a foreign block; the refusal
+    directly after it did not. So a foreign block carrying a *live* handle and
+    an over-long description refused every commit of the day, and the two
+    remedies the message offers -- shorten the description, or drop the link --
+    are both writes to a foreign event, which ``_foreign_touches`` refuses.
+    Nothing tmbx owns is broken and the day cannot be planned: the deadlock
+    ``test_a_foreign_blocks_dead_handle_does_not_refuse_the_day`` describes,
+    reached by the other door.
+
+    Live handle, not ``DEAD_HANDLE``, so the dead-link check passes it through
+    and this refusal is the only one left that can fire.
+    """
+    calendar = RecordingCalendar(
+        {
+            "primary": [
+                _foreign_event(
+                    "f1", 9, 10, link_id=known, description="x" * (MAX_DESCRIPTION_CHARS + 1)
+                ),
+                _event("e2", "DW1", 10, 12, description="focus block"),
+            ]
+        }
+    )
+    service = await _service(tmp_path, materials, calendar)
+    _plan, snapshot = await service.read("primary", DAY)
+
+    result = await service.commit(
+        snapshot, Patch(ops=[UpdateBlock(h="DW1", d="deep focus")])
+    )
+
+    assert result.committed is True
+
+
+async def test_neither_remedy_for_a_foreign_long_description_is_available(
+    tmp_path, materials, known
+):
+    """Why the filter, and not a message telling the caller to fix it.
+
+    The half that makes it a deadlock rather than an inconvenience: both
+    remedies the refusal names are updates to a foreign handle, and both are
+    refused before they reach the calendar.
+    """
+    calendar = RecordingCalendar(
+        {
+            "primary": [
+                _foreign_event(
+                    "f1", 9, 10, link_id=known, description="x" * (MAX_DESCRIPTION_CHARS + 1)
+                ),
+                _event("e2", "DW1", 10, 12),
+            ]
+        }
+    )
+    service = await _service(tmp_path, materials, calendar)
+    plan, snapshot = await service.read("primary", DAY)
+    foreign = next(block.h for block in plan.blocks if block.link == known)
+
+    for remedy in ({"d": "short"}, {"link": None}):
+        with pytest.raises(ForeignBlockError):
+            await service.commit(
+                snapshot,
+                Patch.model_validate(
+                    {"ops": [{"op": "update", "h": foreign, **remedy}]}
+                ),
+            )
+
+
+async def test_an_owned_blocks_long_description_still_refuses(
+    tmp_path, materials, known
+):
+    """The filter narrows the check to what tmbx owns; it does not remove it."""
+    calendar = RecordingCalendar(
+        {
+            "primary": [
+                _foreign_event("f1", 9, 10),
+                _event(
+                    "e2",
+                    "DW1",
+                    10,
+                    12,
+                    link_id=known,
+                    description="x" * (MAX_DESCRIPTION_CHARS + 1),
+                ),
+            ]
+        }
+    )
+    service = await _service(tmp_path, materials, calendar)
+    _plan, snapshot = await service.read("primary", DAY)
+
+    with pytest.raises(PlanViolation) as excinfo:
+        await service.commit(snapshot, Patch(ops=[UpdateBlock(h="DW1", n="Focus")]))
+
+    assert excinfo.value.violation.kind is ViolationKind.DESCRIPTION_TOO_LONG
+
+
 # ---------------------------------------------------------------------------
 # undo puts the url back, and never refuses over a link
 # ---------------------------------------------------------------------------