[object Object]

← back to Terminal Status

TK-11835: fail-fast cross-tty paint — a busy --tty target is skipped, not hung

f18d0879bf66d44c6dde9aaa6247111d7db48b53 · 2026-09-16 11:15:19 -0700 · Steve Abrams

The --tty paint (external supervisor / dot sweeps) could hang 30-65s on a
BUSY target claude session. Measured root cause on Mac2 under overnight load:

1. tickets.discover() runs TWO ps calls per invocation (a `ps -p` argv read
   AND a `ps eww` env dump over every live session) purely to enrich the tab's
   ticket LABEL — ~4.4s under load, on EVERY paint before command dispatch, so
   N concurrent sweepers saturate the process table into the ~30s ps spiral.
2. Store.lock()'s 65s deadline is a SELF-writer backstop; a cross-tty sweeper
   inherited it and waited up to 65s while a busy target held its own per-tty
   lock during its own slow scan.

Fix, both scoped to the cross-tty / paint-legacy path only (a session painting
its OWN tab is byte-identical to before):
- Skip discover's ps enrichment via its existing argv/env seam ({} = already
  resolved). The primary events.jsonl binding + known_tickets validation still
  load fully. Cross-tty startup 3.06s -> 0.67s here (bigger under load).
- Thread the already-resolved rows into set/set_variant/repaint -> lock ->
  assert_owner so the in-lock owner check reuses main's table (no re-scan), and
  add a short env-overridable lock budget (TERMINAL_STATUS_TTY_LOCK_WAIT, 2.5s)
  so a busy target is reported+skipped fast instead of waited on 65s.

Worst-case cross-tty paint is now bounded to ~3s vs 30-65s. Ships a negative
test (test_busy_target_skips_fast_with_short_lock_wait) proving the skip fires
within budget; mutation-verified it goes RED when the wait override is ignored.
41 tests pass, selftest 4/4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QwE2Bg691aQJjEG3XS9LLw

Files touched

Diff

commit f18d0879bf66d44c6dde9aaa6247111d7db48b53
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Wed Sep 16 11:15:19 2026 -0700

    TK-11835: fail-fast cross-tty paint — a busy --tty target is skipped, not hung
    
    The --tty paint (external supervisor / dot sweeps) could hang 30-65s on a
    BUSY target claude session. Measured root cause on Mac2 under overnight load:
    
    1. tickets.discover() runs TWO ps calls per invocation (a `ps -p` argv read
       AND a `ps eww` env dump over every live session) purely to enrich the tab's
       ticket LABEL — ~4.4s under load, on EVERY paint before command dispatch, so
       N concurrent sweepers saturate the process table into the ~30s ps spiral.
    2. Store.lock()'s 65s deadline is a SELF-writer backstop; a cross-tty sweeper
       inherited it and waited up to 65s while a busy target held its own per-tty
       lock during its own slow scan.
    
    Fix, both scoped to the cross-tty / paint-legacy path only (a session painting
    its OWN tab is byte-identical to before):
    - Skip discover's ps enrichment via its existing argv/env seam ({} = already
      resolved). The primary events.jsonl binding + known_tickets validation still
      load fully. Cross-tty startup 3.06s -> 0.67s here (bigger under load).
    - Thread the already-resolved rows into set/set_variant/repaint -> lock ->
      assert_owner so the in-lock owner check reuses main's table (no re-scan), and
      add a short env-overridable lock budget (TERMINAL_STATUS_TTY_LOCK_WAIT, 2.5s)
      so a busy target is reported+skipped fast instead of waited on 65s.
    
    Worst-case cross-tty paint is now bounded to ~3s vs 30-65s. Ships a negative
    test (test_busy_target_skips_fast_with_short_lock_wait) proving the skip fires
    within budget; mutation-verified it goes RED when the wait override is ignored.
    41 tests pass, selftest 4/4.
    
    Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01QwE2Bg691aQJjEG3XS9LLw
---
 terminal_status.py      | 78 ++++++++++++++++++++++++++++++++++++++-----------
 test_terminal_status.py | 29 ++++++++++++++++++
 2 files changed, 90 insertions(+), 17 deletions(-)

diff --git a/terminal_status.py b/terminal_status.py
index 634c233..bbe8e98 100644
--- a/terminal_status.py
+++ b/terminal_status.py
@@ -683,7 +683,7 @@ class Store:
                 raise StatusError("Terminal owner changed; refusing stale writer")
 
     @contextlib.contextmanager
-    def lock(self, owner, rows=None):
+    def lock(self, owner, rows=None, wait=None):
         self.path(owner)  # Validate before building any filesystem path.
         directory = self.root / ".locks"
         directory.mkdir(parents=True, exist_ok=True, mode=0o700)
@@ -694,15 +694,28 @@ class Store:
             # With fixes 1+2 the lock is now held for ~0s, so this longer
             # deadline is a backstop that should essentially never be reached;
             # it is NOT licence to hold the lock across expensive work.
-            deadline = time.monotonic() + float(
+            #
+            # TK-11835: a `wait` OVERRIDE lets a CROSS-TTY caller (the `--tty`
+            # external supervisor / dot sweep) fail FAST instead of inheriting
+            # this 65s SELF-writer backstop. When a target session is mid-turn it
+            # fires its own working-state hooks (each grabbing THIS lock), and a
+            # holder's assert_owner fresh `ps -ax` rescan runs inside the lock --
+            # under concurrent sweep load that ps hits ~30s (see _scan_processes),
+            # so without an override a sweeper hung up to 65s per busy tab. A short
+            # override turns that into a reported skip. `wait=None` preserves the
+            # exact prior behaviour for a session painting its OWN tab.
+            budget = wait if wait is not None else float(
                 os.environ.get("TERMINAL_STATUS_LOCK_WAIT", "65"))
+            deadline = time.monotonic() + budget
             while True:
                 try:
                     fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB)
                     break
                 except BlockingIOError:
                     if time.monotonic() >= deadline:
-                        raise StatusError("Terminal status is busy; retry")
+                        raise StatusError(
+                            "Terminal %s is busy (owner mid-write); skipped after "
+                            "%.1fs -- retry when idle." % (owner.tty, budget))
                     time.sleep(0.02)
             try:
                 self.assert_owner(owner, rows)
@@ -761,7 +774,7 @@ class Store:
         return r, reason
 
     def set(self, owner, color, label="", *, issued_ns=None, ticket_update=None,
-            variant="", deferential=False):
+            variant="", deferential=False, rows=None, lock_wait=None):
         issued_ns = issued_ns if issued_ns is not None else issued_clock()
         if color not in COLORS or not valid_label(label) or variant not in ("", "monitoring", "stopped", "waiting"):
             raise StatusError("Invalid status or label")
@@ -778,7 +791,11 @@ class Store:
         # paints its own reason — otherwise the new ticket lands on the OLD label.
         caller_gave_reason = bool(label) or ticket_update is not None
         label = "" if color == "none" else (label or COLORS[color][2])
-        with self.lock(owner):
+        # TK-11835: a cross-tty caller passes the already-resolved `rows` (so the
+        # in-lock assert_owner reuses main's table instead of re-scanning) and a
+        # short `lock_wait` (so a busy target is skipped fast, not waited on 65s).
+        # Both default to None -> byte-identical to the prior self-paint path.
+        with self.lock(owner, rows=rows, wait=lock_wait):
             previous, _ = self.load(owner)
             if previous and issued_ns <= previous["issued_ns"]:
                 raise StatusError("Obsolete status update; a newer event already won")
@@ -815,7 +832,7 @@ class Store:
             # a session can still clear its own dot or reuse the tab for new work.
             if (deferential and previous and color == "green"
                     and PRIORITY.get(previous.get("state"), PRIORITY["none"]) < PRIORITY["green"]):
-                self.assert_owner(owner)
+                self.assert_owner(owner, rows)
                 try:
                     self.painter(owner, previous)
                 except (OSError, StatusError):
@@ -840,11 +857,11 @@ class Store:
                 "updated_at": dt.datetime.now(dt.timezone.utc).isoformat(),
                 "render_status": "pending", "mirror_errors": [],
             }
-            self.assert_owner(owner)
+            self.assert_owner(owner, rows)
             atomic_write(self.path(owner), json.dumps(record, indent=2) + "\n")
             error = None
             try:
-                self.assert_owner(owner)
+                self.assert_owner(owner, rows)
                 self.painter(owner, record)
                 record["render_status"] = "applied"
             except (OSError, StatusError) as exc:
@@ -861,28 +878,28 @@ class Store:
                 raise StatusError("Status saved, but display/mirror update failed; run audit")
             return record
 
-    def repaint(self, owner):
-        with self.lock(owner):
+    def repaint(self, owner, *, rows=None, lock_wait=None):
+        with self.lock(owner, rows=rows, wait=lock_wait):
             record, reason = self.load(owner)
             if record is None:
                 raise StatusError("Cannot repaint unknown status: " + reason)
-            self.assert_owner(owner)
+            self.assert_owner(owner, rows)
             self.painter(owner, record)
             # Never refresh timestamps, rewrite mirrors, or resurrect cleared state.
             return record
 
-    def set_variant(self, owner, variant):
+    def set_variant(self, owner, variant, *, rows=None, lock_wait=None):
         """Toggle ONLY the variant (e.g. the additive 🔵 stopped marker) on an existing
         status — colour, label, ticket and timestamps are preserved. This is what the
         flasher pulses, so a crashed loop can never corrupt the real dot: the worst case
         is the marker left on or off beside an otherwise-correct base colour."""
         if variant not in ("", "monitoring", "stopped", "waiting"):
             raise StatusError("Invalid variant")
-        with self.lock(owner):
+        with self.lock(owner, rows=rows, wait=lock_wait):
             record, reason = self.load(owner)
             if record is None:
                 raise StatusError("Cannot set variant on unknown status: " + reason)
-            self.assert_owner(owner)
+            self.assert_owner(owner, rows)
             color = record["state"]
             # monitoring AND waiting are green-only tints; drop either on a non-green base.
             if variant in ("monitoring", "waiting") and color != "green":
@@ -1079,8 +1096,22 @@ def main(argv=None):
         return selftest()
     store = Store()
     rows = processes()
+    # TK-11835: tickets.discover() runs TWO `ps` subprocess calls (a `ps -p` argv
+    # read AND a `ps eww` ENV dump) over every live session PURELY to enrich the
+    # tab's ticket LABEL for a session that hasn't declared one otherwise. Measured
+    # ~4.4s under overnight load (the `ps eww` env dump over ~43 pids dominates),
+    # and it runs on EVERY invocation before command dispatch -- so a cross-tty
+    # sweep pays it per paint, and N concurrent sweepers saturate the process table
+    # into the ~30s spiral this ticket reports. A CROSS-TTY paint addresses ONE
+    # target with an EXPLICIT label/ticket and never needs the argv/session-ledger
+    # FALLBACK binding, so pass discover its argv/env skip-seam ({} = "already
+    # resolved, don't scan"). The primary events.jsonl binding + known_tickets
+    # (explicit-ticket validation) still load fully; only the ps enrichment is
+    # skipped. A session painting its OWN tab keeps full enrichment (argv/env None).
+    _cross = bool(getattr(args, "tty", "")) or args.command == "paint-legacy"
     store.ticket_evidence, store.known_tickets = tickets.discover(
-        store.user_root, rows, owners(rows), ancestors)
+        store.user_root, rows, owners(rows), ancestors,
+        argv={} if _cross else None, env={} if _cross else None)
     if args.command in ("scan", "audit"):
         data = scan(store, rows)
         if args.tsv:
@@ -1126,6 +1157,17 @@ def main(argv=None):
     boot = getattr(args, "boot", False)
     guarded = args.command in ("set", "clear", "ticket", "set-variant") or (
         args.command == "current" and getattr(args, "paintable", False))
+    # TK-11835: a CROSS-TTY write (an external supervisor addressing another
+    # session with --tty, or paint-legacy) must fail FAST on a busy target rather
+    # than inherit the 65s self-writer lock backstop — a sweep across every tab
+    # cannot afford to block ~30-65s on each mid-turn session. Give the target a
+    # short, env-overridable budget and reuse main's already-resolved `rows` so
+    # the in-lock owner check does not re-scan. A session painting its OWN tab
+    # (no --tty) keeps the original path unchanged (cross_lock_wait/cross_rows None).
+    cross_tty = bool(getattr(args, "tty", "")) or args.command == "paint-legacy"
+    cross_lock_wait = float(
+        os.environ.get("TERMINAL_STATUS_TTY_LOCK_WAIT", "2.5")) if cross_tty else None
+    cross_rows = rows if cross_tty else None
     if getattr(args, "tty", ""):
         try:
             caller = current_owner(rows)
@@ -1149,7 +1191,8 @@ def main(argv=None):
             prev, _r = store.read(target)
             if not prev or prev.get("state") not in ("yellow", "purple", "orange"):
                 return 0  # not a needs-Steve stop → no 🔵 umbrella (cheap read, no paint)
-        record = store.set_variant(target, args.variant)
+        record = store.set_variant(target, args.variant,
+                                   rows=cross_rows, lock_wait=cross_lock_wait)
         print(f'/terminal-status → /dev/{target.tty} {record["title"] or "CLEARED"}')
         return 0
 
@@ -1228,7 +1271,8 @@ def main(argv=None):
                                variant=("stopped" if getattr(args, "stopped", False)
                                         else "monitoring" if getattr(args, "monitoring", False)
                                         else "waiting" if getattr(args, "waiting", False) else ""),
-                               deferential=getattr(args, "deferential", False))
+                               deferential=getattr(args, "deferential", False),
+                               rows=cross_rows, lock_wait=cross_lock_wait)
         if not getattr(args, "quiet", False):
             print(f'/terminal-status → /dev/{owner.tty} {record["title"] or "CLEARED"}')
     return 0
diff --git a/test_terminal_status.py b/test_terminal_status.py
index e097ff5..7361ffa 100644
--- a/test_terminal_status.py
+++ b/test_terminal_status.py
@@ -604,6 +604,35 @@ class StatusTests(unittest.TestCase):
         self.assertEqual(self.store.row(self.owner)["color"], "green")
         self.assertEqual(self.store.row(second.owner())["color"], "purple")
 
+    def test_busy_target_skips_fast_with_short_lock_wait(self):
+        # TK-11835 NEGATIVE TEST: a cross-tty caller must SKIP a busy target FAST
+        # (short lock_wait), not inherit the 65s self-writer backstop that made a
+        # sweep hang on every mid-turn tab. Hold the target's per-tty lock, then a
+        # set(lock_wait=short) must raise a "busy ... skipped" StatusError WELL
+        # within the budget -- not block for 65s. Goes RED if the wait override
+        # regresses (a bare set() would wait the full backstop and this fails on
+        # the elapsed-time bound / or hang the suite past the injected fault).
+        import fcntl
+        self.store.set(self.owner, "green")            # create the record + lockdir
+        lockpath = self.store.root / ".locks" / (self.owner.tty + ".lock")
+        held = open(lockpath, "a")
+        fcntl.flock(held, fcntl.LOCK_EX | fcntl.LOCK_NB)
+        try:
+            start = ts.time.monotonic()
+            with self.assertRaises(ts.StatusError) as cm:
+                self.store.set(self.owner, "purple", lock_wait=0.2)
+            elapsed = ts.time.monotonic() - start
+            self.assertIn("busy", str(cm.exception))
+            self.assertIn("skipped", str(cm.exception))
+            self.assertIn(self.owner.tty, str(cm.exception))
+            self.assertLess(elapsed, 5.0, "fail-fast: nowhere near the 65s backstop")
+        finally:
+            fcntl.flock(held, fcntl.LOCK_UN)
+            held.close()
+        # Positive control: with the lock free the same short-budget paint lands.
+        record = self.store.set(self.owner, "purple", lock_wait=0.2)
+        self.assertEqual(record["state"], "purple")
+
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)

← 06cd6c5 dot.sh: check-for-color — hard timeout + report result, neve  ·  back to Terminal Status  ·  dot.sh: auto-force retry so bridge/sub sessions paint their c3c5843 →