From 3c5487899652bd395045c8ee7705fa2a9bf40f12 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Mon, 28 Sep 2026 09:30:06 -0700 Subject: [PATCH 1/7] Keep the warm comms session alive, confirm every prompt it's sent, and cut the Slack feed to messages worth reading Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 27 ++ bin/cmux-watcher.py | 246 +++++++++++- bin/comms-listen.py | 601 +++++++++++++++++++++------- bin/comms_lib.py | 172 +++++--- bin/comms_session.py | 387 ++++++++++++++---- docs/assistant-comms-onboarding.md | 18 +- src/assistant/config.py | 4 - src/assistant/slack.py | 130 ++++-- src/assistant/subsystems/comms.py | 45 ++- tests/conftest.py | 9 + tests/test_cmux_watcher.py | 374 ++++++++++++++++- tests/test_comms_gaps.py | 38 +- tests/test_comms_lib.py | 178 +++++++- tests/test_comms_listen_delivery.py | 492 +++++++++++++++++++++++ tests/test_comms_listen_inbox.py | 174 ++++++-- tests/test_comms_listen_watchdog.py | 49 ++- tests/test_comms_session.py | 99 ++++- tests/test_comms_submission.py | 282 +++++++++++++ tests/test_comms_subsystem.py | 41 +- 19 files changed, 2879 insertions(+), 487 deletions(-) create mode 100644 tests/test_comms_listen_delivery.py create mode 100644 tests/test_comms_submission.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 14b91b9..1e02e52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,33 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` and reminders to finish older pending work before starting another task. ### Fixed +- Stop closing a healthy warm comms session whenever cmux is slow to answer. + A refused or timed-out check now leaves the session alone; only cmux saying + the workspace doesn't exist, or a successful workspace list without it, + counts as gone. Short blips get two more looks a few seconds apart. +- Submit prompts to the warm session reliably. The daemon now presses Enter by + writing a carriage return, because `surface.send_key enter` can leave the + prompt typed but unsent on a workspace that was never shown. Every boot + prompt and Slack message is confirmed in the transcript, with up to two more + Enter presses when the text is still in the prompt box. A session whose boot + prompt never lands is closed instead of being declared ready, and the session + is bound to the transcript that recorded its prompt, never the newest file. +- Keep inbound Slack messages until the warm session confirms it received them. + They're recorded on arrival, retried every 30 seconds for up to 3 hours, sent + together once the session is back, and you get one "I'll answer as soon as + it's back" note per outage. Before, a message that arrived while no session + was up was dropped for good. +- Cut the automatic Slack feed. The heartbeat pages once per outage and posts + once when it recovers, instead of every 30 minutes. Housekeeping ledger + entries stay in the brief, and a ledger pass posts at most 5 updates plus one + summary line. A workspace gets at most one ping per 15 minutes unless it asks + a real question. +- Write Slack posts in plain language: what happened first, with the workspace + title and the agent's own question or last message, and refs in a trailing + line. Screen snippets drop Claude's spinner, prompt, and status lines, and the + on-disk pattern bank inherits the built-in "CI green" mute it was missing. +- Point tests at a cmux binary that doesn't exist, so a test that misses a stub + can't drive the real cmux. - Pin the clock in the review-topic focus test so it stops failing once its fixture alert is more than 4 days old. - Download browser-check dependencies publicly so CI doesn't require Adobe's internal network. - Invalidate return notes after completed tool traffic; require review before reusing older notes. diff --git a/bin/cmux-watcher.py b/bin/cmux-watcher.py index 5560c12..90be66e 100644 --- a/bin/cmux-watcher.py +++ b/bin/cmux-watcher.py @@ -25,7 +25,10 @@ Each event's `payload.workspace_id` is a UUID, not a `workspace:NN` ref, and the tool input / screen text is redacted from the event itself — so we read the live -terminal with `cmux read-screen --workspace ` to pattern-match. Events +terminal with `cmux read-screen --workspace ` to pattern-match, and read +the agent's own last words (or its pending question) from the Claude transcript +the payload's `session_id` + `cwd` point at, so the ping reads as a person +would say it. Events arrive as `phase: "received"` then `phase: "completed"` pairs sharing one `_opencode_request_id`; we de-dup on that id so each turn is handled once. @@ -48,6 +51,9 @@ from datetime import datetime, timezone from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import agent_session # noqa: E402 + HOME = Path(os.environ.get("HOME", str(Path.home()))) ASSISTANT_DIR = Path(os.environ.get("CMUX_WATCHER_ASSISTANT_DIR", str(HOME / ".assistant"))) @@ -72,6 +78,12 @@ WS_MAP_TTL_SEC = 30 # Screen read window for pattern matching. SCREEN_LINES = 50 +# Claude Code session transcripts; the agent's last words ride on each inbox item. +CLAUDE_PROJECTS = agent_session.transcript_root(agent_session.CLAUDE, home=HOME) +# Only the transcript tail is read — a long session's JSONL runs to many MB and +# the newest turn is all the ping needs. +TRANSCRIPT_TAIL_BYTES = 256 * 1024 +LAST_MESSAGE_CHARS = 300 # Event names we care about. Everything else (PreToolUse, UserPromptSubmit, # heartbeats, acks) is ignored. @@ -105,6 +117,8 @@ "signal": "work_complete", "priority": "low"}, ], } +_DEFAULT_SUPPRESS = {p["id"]: p["suppress"] + for p in DEFAULT_PATTERN_BANK["patterns"] if "suppress" in p} def utc_iso() -> str: @@ -125,6 +139,17 @@ def log(msg: str) -> None: # ─── pattern bank ───────────────────────────────────────────────────────────── +def inherit_default_suppress(patterns: list[dict]) -> list[dict]: + """Give a loaded pattern its default's `suppress` flag when the file omits it. + + A bank written before a default gained `suppress` keeps pinging for it — the + June bank lacks it on ci-green, which sent 59 "CI green" pings. An explicit + `suppress` in the file still wins, and the file itself is never rewritten.""" + return [{**p, "suppress": _DEFAULT_SUPPRESS[p["id"]]} + if p.get("id") in _DEFAULT_SUPPRESS and "suppress" not in p else p + for p in patterns] + + class PatternBank: """The compiled pattern set, hot-reloaded by mtime. @@ -163,7 +188,7 @@ def load(self) -> None: log(f"pattern_bank: load failed ({e}); using defaults in-memory") data = DEFAULT_PATTERN_BANK self._mtime = None - self.patterns = list(data.get("patterns", [])) + self.patterns = inherit_default_suppress(data.get("patterns", [])) self._compile() def _compile(self) -> None: @@ -225,17 +250,33 @@ def cmux_available() -> bool: return rc == 0 +_TITLE_REF_SUFFIX_RE = re.compile(r"\s*\[\d+\]\s*$") +# cmux's name for a workspace nobody titled; it says nothing about the work. +_DEFAULT_WS_TITLE = "Terminal" + + +def human_title(title: str | None) -> str: + """A workspace title as a person reads it, or "" when it names no work. + cmux appends the workspace number (`Fix flaky ruler [244]`); the ref already + travels in the message footer, so the headline drops the duplicate.""" + name = _TITLE_REF_SUFFIX_RE.sub("", title or "").strip() + return "" if name == _DEFAULT_WS_TITLE else name + + class WsRefResolver: - """Maps a workspace UUID → its `workspace:NN` ref, cached with a short TTL. + """Maps a workspace UUID → its `workspace:NN` ref and human title, cached + with a short TTL. - Events carry UUIDs; the inbox payload reads nicer with the ref. A miss - forces one refresh (a freshly-spawned workspace), then falls back to the - UUID so a drop is never blocked on resolution.""" + Events carry UUIDs; the inbox payload reads nicer with the ref, and the + Slack headline reads nicer still with the title. A miss forces one refresh + (a freshly-spawned workspace), then falls back to the UUID so a drop is + never blocked on resolution.""" def __init__(self, ttl: int = WS_MAP_TTL_SEC, clock=time.time): self.ttl = ttl self._clock = clock self._map: dict[str, str] = {} + self._titles: dict[str, str] = {} self._fetched_at = 0.0 def _refresh(self) -> None: @@ -247,15 +288,25 @@ def _refresh(self) -> None: except json.JSONDecodeError: return new_map: dict[str, str] = {} + new_titles: dict[str, str] = {} for w in data.get("workspaces", []): wid = (w.get("id") or "").upper() ref = w.get("ref") if wid and ref: new_map[wid] = ref + title = human_title(w.get("title")) + if title: + new_titles[wid] = title if new_map: self._map = new_map + self._titles = new_titles self._fetched_at = self._clock() + def title(self, uuid: str | None) -> str | None: + """The workspace's human title from the cache resolve() keeps fresh — + call it after resolve() so a new workspace's title is already fetched.""" + return self._titles.get(uuid.upper()) if uuid else None + def resolve(self, uuid: str | None) -> str | None: if not uuid: return None @@ -290,22 +341,42 @@ def read_screen(workspace: str, lines: int = SCREEN_LINES) -> str: # Lines that are pure TUI chrome — box-drawing rules, the status bar, the # bypass-permissions hint — carry no signal and just bloat the phone snippet. -_CHROME_RE = re.compile(r"^[\s│─╭╮╰╯▔▕>·•⏵◀▶]+$") -_STATUS_BAR_RE = re.compile(r"bypass permissions on|shift\+tab to cycle") +_CHROME_RE = re.compile(r"^[\s│─╭╮╰╯┌┐└┘├┤┬┴┼▔▕>·•⏵◀▶┃╹╻▀▄━]+$") +_STATUS_BAR_RE = re.compile( + r"bypass permissions on|shift\+tab to cycle|Restart to update|run /restart to apply") +# Claude Code's live spinner (`✽ Boogieing… (12m 1s · ↓ 48.9k tokens)`) and its +# turn-done line (`✻ Baked for 2m 10s · done 10:40 PM`) only say the agent is or +# was busy. +_SPINNER_RE = re.compile(r"^\s*[·✢✳✶✻✽]\s+\w+(?:…|\s+for \d+(?:\.\d+)?[hms]\b)") +_STATUS_FIELD_SEP = " │ " + + +def _is_custom_status_line(s: str) -> bool: + """A custom status line (`branch │ ●1 │ context 11% │ $2.13 │ #c2f4fe01`) + joins its fields with ` │ `. A table row the agent printed has them too but + starts with `│`, so it stays.""" + return s.count(_STATUS_FIELD_SEP) >= 2 and not s.lstrip().startswith("│") + + +def _is_tui_noise(s: str) -> bool: + return bool(_CHROME_RE.match(s) or _STATUS_BAR_RE.search(s) + or _SPINNER_RE.match(s) or s.strip() == "❯" + or _is_custom_status_line(s)) def last_lines(text: str, n: int = 3) -> str: """Last n content-bearing lines of the screen, joined — the inbox snippet. - Drops blank lines, pure box-drawing / separator rules, and the cmux status - bar so the snippet reflects what the agent actually printed, not TUI chrome. - Falls back to the raw tail if filtering leaves nothing (rare).""" + Drops blank lines, pure box-drawing / separator rules, the cmux status bar, + Claude Code's spinner and turn-done lines, the empty `❯` prompt, and custom + status lines, so the snippet reflects what the agent actually printed, not + TUI chrome. Falls back to the raw tail if filtering leaves nothing (rare).""" rows = [] for ln in (text or "").splitlines(): s = ln.rstrip() if not s.strip(): continue - if _CHROME_RE.match(s) or _STATUS_BAR_RE.search(s): + if _is_tui_noise(s): continue rows.append(s) if not rows: @@ -313,6 +384,119 @@ def last_lines(text: str, n: int = 3) -> str: return "\n".join(rows[-n:]) +# ─── agent's last message (Claude transcript tail) ─────────────────────────── + +_SESSION_ID_RE = re.compile(r"[A-Za-z0-9-]+") + + +def claude_project_slug(cwd: str) -> str: + """The project dir name Claude Code uses for `cwd`: every non-alphanumeric + character of the real path becomes `-`. agent_session.project_slug maps + only `/`, so dotted or underscored paths (`.worktrees`, macOS temp dirs) + are finished here.""" + return re.sub(r"[^A-Za-z0-9-]", "-", agent_session.project_slug(cwd)) + + +def transcript_path(cwd: str | None, session_id: str | None, + projects_dir: Path = CLAUDE_PROJECTS) -> Path | None: + """Locate the Claude Code transcript for a hook payload's session. + + cmux prefixes the payload's session id with its source + (`claude-6746dc4f-…`); the file on disk is named by the bare id. The hook's + cwd is the session's current dir, which drifts from the launch dir after a + `cd`, so a miss looks for the session id under every project dir.""" + if not session_id or not _SESSION_ID_RE.fullmatch(session_id): + return None + name = f"{session_id.removeprefix('claude-')}.jsonl" + if cwd: + direct = projects_dir / claude_project_slug(cwd) / name + if direct.is_file(): + return direct + return next((d / name for d in projects_dir.iterdir() if (d / name).is_file()), None) + + +def tail_records(path: Path, max_bytes: int = TRANSCRIPT_TAIL_BYTES) -> list[dict]: + """Parsed JSONL records from the last `max_bytes` of a transcript. A line the + seek cuts mid-record has unmatched closing braces, so it fails to parse and + is skipped like any other malformed line.""" + with open(path, "rb") as f: + size = f.seek(0, os.SEEK_END) + f.seek(max(0, size - max_bytes)) + chunk = f.read() + records = [] + for ln in chunk.decode("utf-8", errors="replace").splitlines(): + try: + rec = json.loads(ln) + except json.JSONDecodeError: + continue + if isinstance(rec, dict): + records.append(rec) + return records + + +def _content_blocks(rec: dict) -> list[dict]: + msg = rec.get("message") + content = msg.get("content") if isinstance(msg, dict) else None + if not isinstance(content, list): + return [] + return [b for b in content if isinstance(b, dict)] + + +def pending_question(records: list[dict]) -> str | None: + """First question of the newest AskUserQuestion, unless it's already been + answered — an answered one means the new call isn't in the transcript yet, + and repeating the old question would mislead.""" + answered: set = set() + for rec in reversed(records): + for block in _content_blocks(rec): + if block.get("type") == "tool_result": + answered.add(block.get("tool_use_id")) + elif block.get("type") == "tool_use" and block.get("name") == "AskUserQuestion": + if block.get("id") in answered: + return None + first = next(iter((block.get("input") or {}).get("questions") or []), {}) + question = first.get("question") + return question if isinstance(question, str) and question.strip() else None + return None + + +def last_assistant_text(records: list[dict]) -> str | None: + """The newest non-empty text block the agent wrote.""" + for rec in reversed(records): + if agent_session.record_role(rec) != "assistant": + continue + for block in reversed(_content_blocks(rec)): + text = block.get("text") + if block.get("type") == "text" and isinstance(text, str) and text.strip(): + return text + return None + + +def trim_words(text: str, limit: int = LAST_MESSAGE_CHARS) -> str: + """Collapse whitespace and cut to `limit` chars on a word boundary.""" + flat = " ".join(text.split()) + if len(flat) <= limit: + return flat + return flat[:limit].rsplit(" ", 1)[0] + "…" + + +def read_last_message(cwd: str | None, session_id: str | None, *, + question: bool) -> str | None: + """What the agent last said, for the Slack ping: the pending question when + `question` (an AskUserQuestion event — the hook payload redacts its text), + else the last assistant text. Returns None on any failure so a missing or + malformed transcript never blocks the drop.""" + try: + path = transcript_path(cwd, session_id) + if path is None: + return None + records = tail_records(path) + text = pending_question(records) if question else last_assistant_text(records) + except Exception: # noqa: BLE001 — transcripts are external; the ping must still go + return None + return trim_words(text) if text else None + + # ─── event classification (pure) ────────────────────────────────────────────── def classify_event(evt: dict) -> dict | None: @@ -325,7 +509,7 @@ def classify_event(evt: dict) -> dict | None: On a relevant event returns: {"signal": "needs_input"|"turn_end", "request_id": , "workspace_id": , - "cwd": , "event_name": } + "cwd": , "session_id": , "event_name": } `turn_end` still needs a screen read + pattern match before any drop; `needs_input` is dropped unconditionally (subject to cooldown). """ @@ -348,6 +532,7 @@ def classify_event(evt: dict) -> dict | None: "request_id": request_id, "workspace_id": workspace_id, "cwd": payload.get("cwd"), + "session_id": payload.get("session_id"), "event_name": name, } @@ -360,13 +545,18 @@ def _slug(ws_ref: str | None) -> str: def drop_inbox_item(ws_ref: str | None, signal_type: str, pattern_matched: str, screen_snippet: str, - inbox_dir: Path = INBOX_DIR) -> Path: + inbox_dir: Path = INBOX_DIR, *, + ws_title: str | None = None, + last_message: str | None = None) -> Path: """Atomically write one inbox item. Returns the final path. The shape is: - {ts, event, ws_ref, signal_type, pattern_matched, screen_snippet} - Written to a unique temp file then os.replace'd so a reader never sees a - half-written file (a kqueue watcher wakes on the rename).""" + {ts, event, ws_ref, signal_type, pattern_matched, screen_snippet, + [ws_title], [last_message]} + ws_title / last_message are present only when resolved — they let the + Slack ping lead with the work in plain words. Written to a unique temp file + then os.replace'd so a reader never sees a half-written file (a kqueue + watcher wakes on the rename).""" inbox_dir.mkdir(parents=True, exist_ok=True) item = { "ts": utc_iso(), @@ -376,6 +566,10 @@ def drop_inbox_item(ws_ref: str | None, signal_type: str, "pattern_matched": pattern_matched, "screen_snippet": screen_snippet, } + if ws_title: + item["ws_title"] = ws_title + if last_message: + item["last_message"] = last_message # Unique name: ws slug + monotonic-ish stamp + pid so concurrent drops never # collide. The temp file carries the pid too so two watchers can't clobber. stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%S%f") @@ -438,10 +632,12 @@ def cooled_down(self, ws_key: str, signal_type: str) -> bool: def handle_event(evt: dict, bank: PatternBank, state: WatcherState, - resolver: WsRefResolver, *, screen_reader=read_screen) -> dict | None: + resolver: WsRefResolver, *, screen_reader=read_screen, + message_reader=read_last_message) -> dict | None: """Process one parsed event end-to-end. Returns the dropped item dict (for - tests/logging) or None when nothing was dropped. `screen_reader` is - injectable so tests don't shell out to cmux.""" + tests/logging) or None when nothing was dropped. `screen_reader` and + `message_reader` are injectable so tests don't shell out to cmux or read + real transcripts.""" cls = classify_event(evt) if cls is None: return None @@ -450,6 +646,7 @@ def handle_event(evt: dict, bank: PatternBank, state: WatcherState, workspace_id = cls["workspace_id"] ws_ref = resolver.resolve(workspace_id) + ws_title = resolver.title(workspace_id) ws_key = ws_ref or workspace_id or "unknown" if cls["signal"] == "needs_input": @@ -458,7 +655,10 @@ def handle_event(evt: dict, bank: PatternBank, state: WatcherState, return None snippet = last_lines(screen_reader(workspace_id or ws_ref or "")) pattern_matched = cls["event_name"].split(".")[-1] # Notification / AskUserQuestion - item = drop_inbox_item(ws_ref, "needs_input", pattern_matched, snippet) + last_message = message_reader(cls["cwd"], cls["session_id"], + question=pattern_matched == "AskUserQuestion") + item = drop_inbox_item(ws_ref, "needs_input", pattern_matched, snippet, + ws_title=ws_title, last_message=last_message) record_fired(pattern_matched, ws_ref, "needs_input") log(f"drop needs_input ws={ws_ref or workspace_id} via={pattern_matched} → {item.name}") return {"path": str(item), "signal_type": "needs_input", @@ -479,7 +679,9 @@ def handle_event(evt: dict, bank: PatternBank, state: WatcherState, if not state.cooled_down(ws_key, signal_type): return None snippet = last_lines(screen) - item = drop_inbox_item(ws_ref, signal_type, top.get("id", ""), snippet) + last_message = message_reader(cls["cwd"], cls["session_id"], question=False) + item = drop_inbox_item(ws_ref, signal_type, top.get("id", ""), snippet, + ws_title=ws_title, last_message=last_message) record_fired(top.get("id", ""), ws_ref, signal_type) log(f"drop {signal_type} ws={ws_ref or workspace_id} pattern={top.get('id')} → {item.name}") return {"path": str(item), "signal_type": signal_type, diff --git a/bin/comms-listen.py b/bin/comms-listen.py index 189203f..4e1fa00 100755 --- a/bin/comms-listen.py +++ b/bin/comms-listen.py @@ -5,21 +5,26 @@ one blocking loop per thread joined under a shutdown Event: 1. INBOUND (event) — REST-poll Slack (conversations.history via slack-poll.py) - for inbound messages in the configured DM/channel. On a message: feeds the - warm cmux session, which composes and sends a reply via slack-send.py. + for inbound messages in the configured DM/channel. Each message is queued + on disk and fed to the warm cmux session, which composes and sends a reply + via slack-send.py. It leaves the queue only once the session's transcript + shows it arrived; until then it's retried. 2. WATCHDOG (timer) — every WATCHDOG_INTERVAL_SEC, ensure a live warm session - exists (respawn if dead/missing). Closes the gap where a warm workspace that - died between inbound messages (cmux restart, crash, sleep) stayed dead until - the next Slack message arrived. + exists (respawn if cmux says it's gone; leave it alone if cmux doesn't + answer). Closes the gap where a warm workspace that died between inbound + messages (cmux restart, crash, sleep) stayed dead until the next Slack + message arrived. 3. OUTBOUND PINGS (event) — watch actions-ledger.jsonl for appends. On new - lines, format with comms_lib.fmt_action_line and send. No LLM — mechanical, + lines, skip housekeeping, format with comms_lib.fmt_action_line, and send + at most LEDGER_MAX_PER_PASS plus one summary line. No LLM — mechanical, fires near-instantly (~2s stat-poll floor). 4. INBOX (event) — watch ~/.assistant/inbox for cmux-watcher signals - (workspace needs input / work complete) and ping within seconds. kqueue on - macOS, stat-poll fallback elsewhere. + (workspace needs input / work complete) and ping within seconds, at most + once per workspace per INBOX_COOLDOWN_SEC unless it's a real question. + kqueue on macOS, stat-poll fallback elsewhere. 5. PROPOSALS (timer) — watch ~/.assistant/proposals.jsonl (the durable queue the lesson-extractor writes). Deliver each new pending lesson proposal to @@ -27,8 +32,9 @@ first run), asking Mukul to confirm it. No LLM. 6. HEARTBEAT PAGE (timer) — every 60s, check Assistant's heartbeat; if stale - or status ∈ {frozen, stale_world, respawn-requested}, send a templated - urgent page (30-min dedup). No LLM. + or status ∈ {frozen, stale_world, respawn-requested} for two checks in a + row, send one templated urgent page, then one message when it recovers. + No LLM. All six reuse the tested CLIs and comms_lib. Durable memory stays in conversation.jsonl, so a crash + KeepAlive respawn loses nothing. @@ -79,8 +85,6 @@ def _load_doctor(): BIN = REPO / "bin" WARM_PROMPT = REPO / "prompts" / "prompt-assistant-comms-warm.md" -REPLY_WAIT_SEC = int(os.environ.get("COMMS_REPLY_WAIT_SEC", "120")) - SLACK_POLL = BIN / "slack-poll.py" SLACK_SEND = BIN / "slack-send.py" CONVERSATION = BIN / "conversation.py" @@ -90,7 +94,28 @@ def _load_doctor(): SLACK_POLL_INTERVAL_SEC = int(os.environ.get("COMMS_SLACK_POLL_SEC", "3")) LEDGER_POLL_SEC = float(os.environ.get("COMMS_LEDGER_POLL_SEC", "2")) HEARTBEAT_CHECK_SEC = int(os.environ.get("COMMS_HEARTBEAT_CHECK_SEC", "60")) -HEARTBEAT_DEDUP_SEC = 1800 +HEARTBEAT_CONFIRM_CHECKS = 2 + +# Inbound messages wait on disk until the warm session confirms it received +# them. The slack cursor moves past a message as soon as it's polled, so before +# this queue a message that arrived while no session was up was lost for good +# (2026-09-27: two "are you alive?" messages). Undelivered messages are retried +# every PENDING_RETRY_SEC and given up after PENDING_MAX_AGE_SEC, when an +# answer would no longer help. +PENDING_RETRY_SEC = float(os.environ.get("COMMS_PENDING_RETRY_SEC", "30")) +PENDING_MAX_AGE_SEC = float(os.environ.get("COMMS_PENDING_MAX_AGE_SEC", str(3 * 3600))) +RESTART_NOTICE = ("My chat session isn't responding right now. I'll answer as soon " + "as it's back.") +RESTART_NOTICE_AFTER_SEC = float(os.environ.get("COMMS_RESTART_NOTICE_AFTER_SEC", "60")) + +# At most this many action updates go out per ledger pass; the rest collapse +# into one summary line (2026-09-27: 129 posts in two minutes). +LEDGER_MAX_PER_PASS = int(os.environ.get("COMMS_LEDGER_MAX_PER_PASS", "5")) + +# One workspace gets at most one ping per window, whatever the signal. A real +# question (AskUserQuestion) always goes through (2026-09-28: one workspace was +# pinged 21 times in six hours, a median of three minutes apart). +INBOX_COOLDOWN_SEC = float(os.environ.get("COMMS_INBOX_COOLDOWN_SEC", "900")) # Proposals are a durable queue, not a live event, so we poll on a slow cadence # (they're written at most a few times a day by the pulse-throttled extractor). @@ -173,13 +198,40 @@ def cli(argv: list[str], timeout: int = 30, env: dict | None = None) -> tuple[in # --------------------------------------------------------------------------- inbound -def ensure_warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False) -> dict | None: - """Return a live warm-session record, spawning one if none is alive. +# Results of _warm_session, beyond the session record itself. +SESSION_ALIVE = "alive" +SESSION_SPAWNED = "spawned" +SESSION_UNREACHABLE = "unreachable" # cmux isn't answering; the session is left alone +SESSION_NONE = "none" - respawn_on_stale (inbound path only): also respawn a session that is alive - but whose recorded model id no longer matches the current backend (a - `claude-backend` toggle since spawn). The watchdog passes False so it never - closes a live session out from under an active reply. +# When cmux first stopped answering in the current episode, so the log gets one +# line per episode instead of one per check. +_cmux_silent_since: float | None = None + + +def _note_cmux_answer(answered: bool) -> None: + global _cmux_silent_since + now = time.time() + if not answered and _cmux_silent_since is None: + _cmux_silent_since = now + log("cmux isn't answering — leaving the warm session alone until it does") + elif answered and _cmux_silent_since is not None: + log(f"cmux answering again after {int(now - _cmux_silent_since)}s") + _cmux_silent_since = None + + +def _warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False, + force_respawn: bool = False) -> tuple[dict | None, str]: + """The live warm-session record and how it was obtained, spawning one if + none is alive. + + A session is replaced only when cmux says its workspace is gone, or when + it's alive and the caller asks: respawn_on_stale (inbound path only) for a + session whose model id no longer matches the current backend, or + force_respawn for one that didn't accept a typed prompt. When cmux doesn't + answer at all, the session is left alone — a refused or timed-out check + says nothing about the workspace (2026-09-27: every such check used to + close a healthy session and spawn another onto a stalled cmux). On respawn, close the prior warm workspace first so we never leak Claude processes. close_own_workspace is title-guarded — it only ever closes an @@ -192,93 +244,220 @@ def ensure_warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = Fals with _warm_session_lock: sess = comms_session.read_session(paths) if sess: - alive = comms_session.cmux_alive(paths, sess["ws_ref"]) - if alive: - # A stale-but-alive session (its model id no longer matches the - # current backend after a `claude-backend` toggle) still WORKS on - # its old backend, so only the INBOUND path respawns it — - # respawn_on_stale=True — and does so BEFORE that worker feeds its - # message, so it never drops its OWN in-flight reply. The liveness - # watchdog leaves a live session alone (respawn_on_stale=False): - # closing it there could race an active reply, and it isn't - # counted toward the watchdog backoff, so a flapping resolver - # would churn cmux. - # - # Residual (unreachable in the 1:1-DM config, one channel worker): - # feed() runs outside _warm_session_lock, so if a SECOND channel - # worker respawned this shared session mid-reply, the first - # worker's reply could drop. One-shot per toggle and self-healing - # (the inbound turn is already in conversation.jsonl). Widen the - # lock over feed() only if multi-channel ever goes hot. - if not respawn_on_stale or comms_session.warm_session_model_is_current(paths, sess): - return sess - why = f"model stale ({sess.get('model')!r} — backend changed since spawn)" + state = comms_session.workspace_state(paths, sess["ws_ref"]) + _note_cmux_answer(state != comms_session.UNKNOWN) + if state == comms_session.UNKNOWN: + return sess, SESSION_UNREACHABLE + if state == comms_session.ALIVE: + # A stale-but-alive session still WORKS on its old backend, so + # only the INBOUND path respawns it, BEFORE it feeds its own + # message. The watchdog leaves a live session alone: closing it + # there could race an active reply. + if force_respawn: + why = "didn't accept the typed message" + elif respawn_on_stale and not comms_session.warm_session_model_is_current(paths, sess): + why = f"model stale ({sess.get('model')!r} — backend changed since spawn)" + else: + return sess, SESSION_ALIVE else: why = "gone" log(f"warm session {sess['ws_ref']} {why} — closing it and respawning") comms_session.close_own_workspace(paths, sess["ws_ref"], log=log) comms_session.clear_session_registry(paths) - return comms_session.spawn_session(paths, WARM_PROMPT, log=log) - - -def reply_to_message(paths: comms_lib.Paths, sess: dict, rec: dict) -> dict: - """Warm reply: record inbound, feed the message to the warm session, wait for - its reply turn in the transcript, then /clear if context >= 50%. Returns the - (possibly refreshed) session record.""" - channel = rec.get("channel") - text = rec.get("text", "") - msg_ts = rec.get("msg_ts") - reply_to = rec.get("reply_to") - - # Record the inbound turn first — survives even if the session stalls. - in_args = [str(CONVERSATION), "append", "--channel", str(channel), - "--direction", "in", "--text", text] - if msg_ts is not None: - in_args += ["--msg-ts", str(msg_ts)] - if reply_to is not None: - in_args += ["--reply-to", str(reply_to)] - cli(in_args, timeout=10) + spawned = comms_session.spawn_session(paths, WARM_PROMPT, log=log) + return spawned, (SESSION_SPAWNED if spawned else SESSION_NONE) + +def ensure_warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False) -> dict | None: + """The live warm-session record, spawning one if none is alive (see + _warm_session).""" + return _warm_session(paths, respawn_on_stale=respawn_on_stale)[0] + + +def feed_text(recs: list[dict], channel: str) -> str: + """The user turn that hands inbound Slack message(s) to the warm session. + + The header carries the newest message's ts; its `msg_ts=` doubles as the + marker that confirms the turn landed. Messages that piled up while the + session was down go in one turn, so the session answers them together.""" + ts = recs[-1].get("msg_ts") + header = f"[slack channel={channel} msg_ts={ts} send_cli={SLACK_SEND}]" + if len(recs) == 1: + return f"{header} {recs[0].get('text', '')}" + parts = " ".join(f"({i}) {r.get('text', '')}" for i, r in enumerate(recs, 1)) + return (f"{header} {len(recs)} messages arrived while your session was down. " + f"Answer them together in one reply. {parts}") + + +def reply_to_message(paths: comms_lib.Paths, sess: dict, + recs: list[dict]) -> tuple[bool, dict]: + """Hand inbound message(s) to the warm session and confirm its transcript + recorded them, then /clear if context >= 50%. Returns (delivered, the + possibly refreshed session record). + + Confirmation looks for the message's marker in the bound transcript first, + then in any transcript in the session's project folder, and rebinds the + session to wherever it landed.""" + channel = str(recs[-1].get("channel")) + marker = f"msg_ts={recs[-1].get('msg_ts')}" # Thread the session's provider through every transcript-root / context call: # a Droid session writes under ~/.factory/sessions with no usage block, so a # claude default here would read the wrong root and never clear (G3). agent = sess.get("agent") or agent_session.CLAUDE - transcript = sess.get("transcript_path") or comms_session.newest_transcript(sess["cwd"], agent) - before_lines = comms_session.transcript_line_count(transcript) if transcript else 0 - - # Feed the message as a user turn. The warm session's boot prompt tells it - # how to reconstruct context, reply via slack-send.py, and record the out - # turn. This is a 1:1 channel — the session replies at TOP LEVEL (no - # threading), so the header only needs the channel + send CLI. - feed_text = ( - f"[slack channel={channel} msg_ts={msg_ts} send_cli={SLACK_SEND}] {text}" - ) - t0 = time.time() - comms_session.feed(paths, sess["surface_ref"], feed_text) + project_dir = comms_session.project_dir_for_cwd(sess["cwd"], agent) + bound = sess.get("transcript_path") + since = time.time() - 1 + found: list[str] = [] + + def confirmed() -> bool: + if bound and comms_session.transcript_has_submission(bound, marker): + hit = bound + else: + hit = comms_session.find_submission(project_dir, marker, since) + if hit: + found.append(hit) + return hit is not None - grew = False - while time.time() - t0 < REPLY_WAIT_SEC: - time.sleep(2) - if transcript and comms_session.transcript_line_count(transcript) > before_lines: - grew = True - break - if not transcript: - transcript = comms_session.newest_transcript(sess["cwd"], agent) - log(f"reply channel={channel} msg={msg_ts} grew={grew} wall_ms={int((time.time()-t0)*1000)}") + t0 = time.time() + delivered = comms_session.submit(paths, sess["surface_ref"], feed_text(recs, channel), + marker, confirmed) + log(f"reply channel={channel} msg={recs[-1].get('msg_ts')} n={len(recs)} " + f"submitted={delivered} wall_ms={int((time.time() - t0) * 1000)}") + if not delivered: + return False, sess + transcript = found[-1] # Context management: clear-and-resume at >= 50% (claude) or the size proxy # (droid). should_clear + clear_session are provider-aware; for droid a # "clear" is a lossless respawn since durable memory lives in conversation.jsonl. # clear_session owns the registry update and returns the refreshed record. - if transcript and comms_session.should_clear(transcript, agent=agent): + if comms_session.should_clear(transcript, agent=agent): log(f"context threshold reached ({agent}) — clear-and-resume") - return comms_session.clear_session(paths, sess, WARM_PROMPT, agent=agent, log=log) + return True, comms_session.clear_session(paths, sess, WARM_PROMPT, agent=agent, log=log) - if transcript and transcript != sess.get("transcript_path"): + if transcript != bound: + log(f"warm session transcript rebound to {transcript}") comms_session.write_session(paths, sess["ws_ref"], sess["surface_ref"], sess["cwd"], transcript) sess = comms_session.read_session(paths) or sess - return sess + return True, sess + + +# --------------------------------------------------------------------------- pending inbound + +_pending_lock = threading.Lock() + + +def _pending_path(paths: comms_lib.Paths) -> Path: + return paths.comms_dir / "pending-inbound.json" + + +def read_pending(paths: comms_lib.Paths) -> list[dict]: + try: + data = json.loads(_pending_path(paths).read_text()) + except (OSError, json.JSONDecodeError): + return [] + return [r for r in data if isinstance(r, dict)] if isinstance(data, list) else [] + + +def _write_pending(paths: comms_lib.Paths, recs: list[dict]) -> None: + p = _pending_path(paths) + p.parent.mkdir(parents=True, exist_ok=True) + tmp = p.with_suffix(".json.tmp") + tmp.write_text(json.dumps(recs, indent=2)) + os.replace(tmp, p) + + +def add_pending(paths: comms_lib.Paths, rec: dict) -> None: + """Queue an inbound message until the warm session confirms it. A message + already queued (same msg_ts) isn't added twice.""" + with _pending_lock: + recs = read_pending(paths) + if any(r.get("msg_ts") == rec.get("msg_ts") for r in recs): + return + _write_pending(paths, [*recs, rec]) + + +def remove_pending(paths: comms_lib.Paths, msg_tss: list) -> None: + drop = set(msg_tss) + with _pending_lock: + _write_pending(paths, [r for r in read_pending(paths) if r.get("msg_ts") not in drop]) + + +def message_age_sec(rec: dict, now: float) -> float: + """Seconds since Slack received the message (its ts is epoch seconds).""" + try: + return now - float(rec.get("msg_ts")) + except (TypeError, ValueError): + return 0.0 + + +def split_pending(recs: list[dict], channel: str, now: float, + max_age: float) -> tuple[list[dict], list[dict]]: + """(still worth answering, too old to answer) for one channel's queued + messages, oldest first.""" + mine = sorted((r for r in recs if str(r.get("channel") or "default") == channel), + key=lambda r: message_age_sec(r, now), reverse=True) + fresh = [r for r in mine if message_age_sec(r, now) <= max_age] + expired = [r for r in mine if message_age_sec(r, now) > max_age] + return fresh, expired + + +def _record_inbound(rec: dict) -> None: + """Append the inbound turn to conversation.jsonl as soon as it arrives, so + it survives even if it's never delivered.""" + args = [str(CONVERSATION), "append", "--channel", str(rec.get("channel")), + "--direction", "in", "--text", rec.get("text", "")] + if rec.get("msg_ts") is not None: + args += ["--msg-ts", str(rec["msg_ts"])] + if rec.get("reply_to") is not None: + args += ["--reply-to", str(rec["reply_to"])] + cli(args, timeout=10) + + +def _restart_notice_path(paths: comms_lib.Paths) -> Path: + return paths.comms_dir / "restart-notice.json" + + +def _notify_restart_once(paths: comms_lib.Paths, channel: str, env: dict | None) -> None: + """Tell the user, once per outage, that their message is waiting. The marker + is written before sending, so a failing send can't repeat every retry.""" + marker = _restart_notice_path(paths) + if marker.exists(): + return + marker.write_text(json.dumps({"ts": time.time(), "channel": channel})) + rc, _out, err = cli(_send_args(RESTART_NOTICE, "reply", channel, None), timeout=30, env=env) + log(f"restart notice sent to {channel}" if rc == 0 + else f"restart notice rc={rc} err={err.strip()[:160]}") + + +def _deliver_pending(paths: comms_lib.Paths, channel: str, env: dict | None) -> bool: + """Try to hand this channel's queued messages to the warm session. Returns + True when nothing is left waiting.""" + now = time.time() + fresh, expired = split_pending(read_pending(paths), channel, now, PENDING_MAX_AGE_SEC) + if expired: + for r in expired: + log(f"inbound msg={r.get('msg_ts')} still undelivered after " + f"{comms_lib.fmt_age(int(message_age_sec(r, now)))} — giving up on it") + remove_pending(paths, [r.get("msg_ts") for r in expired]) + if not fresh: + return True + sess, how = _warm_session(paths, respawn_on_stale=True) + delivered = False + if sess and how != SESSION_UNREACHABLE: + delivered, _sess = reply_to_message(paths, sess, fresh) + if not delivered: + _warm_session(paths, force_respawn=True) + if delivered: + remove_pending(paths, [r.get("msg_ts") for r in fresh]) + _restart_notice_path(paths).unlink(missing_ok=True) + return True + log(f"inbound: {len(fresh)} message(s) waiting for the warm session " + f"(session={how}); retrying in {int(PENDING_RETRY_SEC)}s") + if message_age_sec(fresh[0], now) >= RESTART_NOTICE_AFTER_SEC: + _notify_restart_once(paths, channel, env) + return False def _poll_thread(stop: threading.Event, env: dict, msg_queue: queue.Queue) -> None: @@ -308,23 +487,33 @@ def _poll_thread(stop: threading.Event, env: dict, msg_queue: queue.Queue) -> No stop.wait(SLACK_POLL_INTERVAL_SEC) -def _channel_worker(channel_id: str, ch_queue: queue.Queue, stop: threading.Event) -> None: - """Per-channel worker: serializes replies for one channel while other - channels run concurrently.""" +def _channel_worker(channel_id: str, ch_queue: queue.Queue, stop: threading.Event, + env: dict | None = None) -> None: + """Per-channel worker: queues each inbound message on disk, delivers the + queue, and retries every PENDING_RETRY_SEC until the warm session confirms + it. Serializes replies for one channel while other channels run + concurrently.""" paths = comms_lib.Paths.from_env() - sess = ensure_warm_session(paths, respawn_on_stale=True) + waiting = bool(split_pending(read_pending(paths), channel_id, time.time(), + PENDING_MAX_AGE_SEC)[0]) + last_try = 0.0 while not stop.is_set(): try: rec = ch_queue.get(timeout=1) except queue.Empty: - continue - log(f"inbound channel={channel_id} msg={rec.get('msg_ts')} " - f"text={rec.get('text','')[:80]!r}") - sess = ensure_warm_session(paths, respawn_on_stale=True) - if not sess: - log(f"no warm session — skipping msg={rec.get('msg_ts')}") - continue - sess = reply_to_message(paths, sess, rec) + rec = None + if rec is not None: + log(f"inbound channel={channel_id} msg={rec.get('msg_ts')} " + f"text={rec.get('text', '')[:80]!r}") + _record_inbound(rec) + add_pending(paths, rec) + waiting = True + if waiting and (rec is not None or time.time() - last_try >= PENDING_RETRY_SEC): + last_try = time.time() + try: + waiting = not _deliver_pending(paths, channel_id, env) + except Exception as e: # noqa: BLE001 — one bad pass must never kill the channel + log(f"inbound delivery error (will retry): {type(e).__name__}: {e}") def inbound_loop(stop: threading.Event, env: dict) -> None: @@ -335,28 +524,35 @@ def inbound_loop(stop: threading.Event, env: dict) -> None: comms_session.reconcile_warm_workspaces(paths, keep=sess["ws_ref"], log=log) channel_workers: dict[str, tuple[queue.Queue, threading.Thread]] = {} - msg_queue: queue.Queue = queue.Queue() - poller = threading.Thread(target=_poll_thread, args=(stop, env, msg_queue), - name="inbound-poller", daemon=True) - poller.start() - while not stop.is_set(): - try: - rec = msg_queue.get(timeout=1) - except queue.Empty: - continue - channel_id = str(rec.get("channel") or "default") + def worker_for(channel_id: str) -> queue.Queue: if channel_id not in channel_workers: ch_q: queue.Queue = queue.Queue() t = threading.Thread( target=_channel_worker, - args=(channel_id, ch_q, stop), + args=(channel_id, ch_q, stop, env), name=f"inbound-{channel_id}", daemon=True, ) t.start() channel_workers[channel_id] = (ch_q, t) - channel_workers[channel_id][0].put(rec) + return channel_workers[channel_id][0] + + # Messages still queued from before a restart get their workers right away. + for channel_id in sorted({str(r.get("channel") or "default") for r in read_pending(paths)}): + worker_for(channel_id) + + msg_queue: queue.Queue = queue.Queue() + poller = threading.Thread(target=_poll_thread, args=(stop, env, msg_queue), + name="inbound-poller", daemon=True) + poller.start() + + while not stop.is_set(): + try: + rec = msg_queue.get(timeout=1) + except queue.Empty: + continue + worker_for(str(rec.get("channel") or "default")).put(rec) # --------------------------------------------------------------------------- warm-session liveness watchdog @@ -367,12 +563,16 @@ def watchdog_tick(paths: comms_lib.Paths) -> str: and tests. NEVER raises — a transient cmux error (cmux briefly down, a spawn timeout) must not kill the watchdog thread; it logs and retries on the next tick. Delegates the actual liveness check + respawn to - ensure_warm_session under _warm_session_lock, so this never races an - inbound-driven respawn.""" + _warm_session under _warm_session_lock, so this never races an + inbound-driven respawn. "cmux-unresponsive" means the session was left + alone because cmux didn't answer; it backs off like a failure so a long + cmux stall isn't probed every minute.""" try: - sess = ensure_warm_session(paths) + sess, how = _warm_session(paths) except Exception as e: # noqa: BLE001 — watchdog must survive any error return f"error:{type(e).__name__}" + if how == SESSION_UNREACHABLE: + return "cmux-unresponsive" return "alive" if sess else "no-session" @@ -417,6 +617,9 @@ def watchdog_loop(stop: threading.Event, env: dict) -> None: # --------------------------------------------------------------------------- outbound pings +HOUSEKEEPING_KINDS = ("decision-transition", "strategist-autopause", "stranded", "skipped") + + def _suppress_reason(entry: dict) -> str | None: """Return a reason string if this ledger entry should NOT be broadcast to Slack, or None if it should. Pure decision logic (no I/O) — the daemon owns @@ -440,6 +643,13 @@ def _suppress_reason(entry: dict) -> str | None: # brief.RECEIPT_KINDS and CommsSubsystem._broadcast_entry. if kind in ("event-drop", "decision-auto-done", "merge-dispatched"): return f"receipt kind={kind} (pull-only, shown in brief)" + # Housekeeping the brief and dashboard already show: expiring stale + # decisions, pausing the strategist, nudging a stalled workspace, and skip + # records. Pushing them buried real updates (2026-09-27: 129 "decision + # expired" posts in two minutes). Keep in sync with + # CommsSubsystem._broadcast_entry. + if kind in HOUSEKEEPING_KINDS: + return f"housekeeping kind={kind} (pull-only, shown in brief)" if kind == "self-update" and "skip" in key: return "self-update-skip" if kind in ("lesson-proposal", "lesson_proposal") or key.startswith("lesson-proposal"): @@ -452,9 +662,47 @@ def _suppress_reason(entry: dict) -> str | None: return None +def plan_broadcast(entries: list[dict], + max_send: int = LEDGER_MAX_PER_PASS) -> tuple[list[dict], list[tuple[dict, str]], int]: + """Split one ledger pass into (entries to post, suppressed entries with + their reasons, how many more were held back past the per-pass cap).""" + suppressed = [] + postable = [] + for entry in entries: + reason = _suppress_reason(entry) + if reason is None: + postable.append(entry) + else: + suppressed.append((entry, reason)) + return postable[:max_send], suppressed, max(0, len(postable) - max_send) + + +def fmt_overflow(n: int) -> str: + return (f"…and {n} more Assistant update{'s' if n != 1 else ''}. " + f"They're on the dashboard at http://127.0.0.1:9876.") + + +def _mirror_sent(out: str, body: str) -> None: + """Record each message slack-send posted as an out turn in + conversation.jsonl.""" + for line in out.strip().splitlines(): + try: + sent = json.loads(line) + except json.JSONDecodeError: + continue + if sent.get("muted") or not sent.get("message_id"): + continue + convo_id = sent.get("channel") + if convo_id: + cli([str(CONVERSATION), "append", "--channel", str(convo_id), + "--direction", "out", "--text", body, "--kind", "action", + "--msg-ts", str(sent["message_id"])], timeout=10) + + def ledger_loop(stop: threading.Event, env: dict) -> None: """Watch actions-ledger.jsonl; broadcast each new entry to the configured - target. stat-poll (2s) — simple and dependency-free.""" + target, at most LEDGER_MAX_PER_PASS per pass plus one summary line for the + rest. stat-poll (2s) — simple and dependency-free.""" paths = comms_lib.Paths.from_env() comms_lib.initialize_cursor_if_missing(paths) log("ledger loop started (slack)") @@ -465,35 +713,27 @@ def ledger_loop(stop: threading.Event, env: dict) -> None: except Exception as e: # noqa: BLE001 log(f"ledger read error: {e}") entries = [] - for entry in entries: + to_send, suppressed, overflow = plan_broadcast(entries, LEDGER_MAX_PER_PASS) + for entry, reason in suppressed: + log(f"suppressed broadcast key={entry.get('key', '')}: {reason}") + if (to_send or overflow) and not target: + log(f"no target configured — skipping {len(to_send) + overflow} broadcast(s)") + to_send, overflow = [], 0 + for entry in to_send: key = entry.get("key", "") - reason = _suppress_reason(entry) - if reason is not None: - log(f"suppressed broadcast key={key}: {reason}") - continue - if not target: - log(f"no target configured — skipping broadcast key={key}") - continue body = comms_lib.fmt_action_line(entry) - send_argv = _send_args(body, "action", target, key) - rc, out, err = cli(send_argv, timeout=30, env=env) + rc, out, err = cli(_send_args(body, "action", target, key), timeout=30, env=env) if rc != 0: log(f"ledger broadcast rc={rc} key={key} err={err.strip()[:160]}") continue - # Mirror each sent broadcast into conversation.jsonl as an out turn. - for line in out.strip().splitlines(): - try: - sent = json.loads(line) - except json.JSONDecodeError: - continue - if sent.get("muted") or not sent.get("message_id"): - continue - convo_id = sent.get("channel") - if convo_id: - cli([str(CONVERSATION), "append", "--channel", str(convo_id), - "--direction", "out", "--text", body, "--kind", "action", - "--msg-ts", str(sent["message_id"])], timeout=10) + _mirror_sent(out, body) log(f"broadcast key={key}") + if overflow: + body = fmt_overflow(overflow) + rc, out, err = cli(_send_args(body, "action", target, None), timeout=30, env=env) + if rc == 0: + _mirror_sent(out, body) + log(f"broadcast overflow summary for {overflow} update(s) rc={rc}") stop.wait(LEDGER_POLL_SEC) @@ -600,6 +840,39 @@ def _signal_age_sec(item: dict, path: Path, now: float) -> float: return 0.0 +def inbox_should_ping(item: dict, last_ping_ts: float | None, now: float, + cooldown: float = INBOX_COOLDOWN_SEC) -> bool: + """True if this workspace signal should reach Slack: a real question always + does; anything else only when the workspace hasn't been pinged within the + cooldown.""" + if item.get("pattern_matched") == "AskUserQuestion": + return True + return last_ping_ts is None or now - last_ping_ts >= cooldown + + +def _cooldown_path(paths: comms_lib.Paths) -> Path: + return paths.comms_dir / "inbox-cooldown.json" + + +def _read_cooldown(paths: comms_lib.Paths) -> dict[str, float]: + try: + data = json.loads(_cooldown_path(paths).read_text()) + except (OSError, json.JSONDecodeError): + return {} + return {k: v for k, v in data.items() if isinstance(v, (int, float))} if isinstance(data, dict) else {} + + +def _write_cooldown(paths: comms_lib.Paths, last_ping: dict[str, float], now: float) -> None: + """Persist per-workspace last-ping times, dropping ones past the cooldown so + the file never grows without bound.""" + keep = {k: v for k, v in last_ping.items() if now - v < INBOX_COOLDOWN_SEC} + p = _cooldown_path(paths) + p.parent.mkdir(parents=True, exist_ok=True) + tmp = p.with_suffix(".json.tmp") + tmp.write_text(json.dumps(keep)) + os.replace(tmp, p) + + def _drain_inbox_once(env: dict) -> int: """Read every cmux-*.json in the inbox, ping, delete it. Returns the number of items PINGED. Stale signals (older than INBOX_MAX_AGE_SEC) are deleted @@ -611,8 +884,10 @@ def _drain_inbox_once(env: dict) -> int: paths = comms_lib.Paths.from_env() target = _target(paths) now = time.time() + last_ping = _read_cooldown(paths) n = 0 stale = 0 + held = 0 for p in sorted(INBOX_DIR.glob(INBOX_GLOB)): try: raw = p.read_text() @@ -639,6 +914,14 @@ def _drain_inbox_once(env: dict) -> int: if not target: log(f"inbox: no target configured — leaving {p.name} for retry") continue + ws_key = str(item.get("ws_ref") or "ws") + if not inbox_should_ping(item, last_ping.get(ws_key), now): + try: + p.unlink() + except OSError: + pass + held += 1 + continue body = comms_lib.fmt_workspace_signal(item) ledger_key = f"{item.get('ws_ref') or 'ws'}:{item.get('signal_type') or item.get('signal') or 'signal'}" send_argv = _send_args(body, "action", target, ledger_key) @@ -651,10 +934,16 @@ def _drain_inbox_once(env: dict) -> int: except OSError: pass n += 1 + last_ping[ws_key] = now log(f"inbox: pinged {item.get('signal_type') or item.get('signal')} " f"ws={item.get('ws_ref')} ({p.name})") + if n: + _write_cooldown(paths, last_ping, now) if stale: log(f"inbox: dropped {stale} stale signal(s) older than {int(INBOX_MAX_AGE_SEC)}s (no ping)") + if held: + log(f"inbox: held back {held} signal(s) from workspaces pinged in the last " + f"{int(INBOX_COOLDOWN_SEC)}s (no ping)") return n @@ -704,9 +993,24 @@ def inbox_loop(stop: threading.Event, env: dict) -> None: # --------------------------------------------------------------------------- heartbeat page +def heartbeat_action(unhealthy_checks: int, paged: bool, + confirm: int = HEARTBEAT_CONFIRM_CHECKS) -> str | None: + """"page" once Assistant's main loop has looked stopped for `confirm` + checks in a row, "recover" at the first healthy check after a page, else + None — one message per outage instead of one every half hour + (2026-09-14→27: 621 identical pages). The confirmation keeps a pulse that + runs a minute late from paging and recovering in back-to-back posts.""" + if unhealthy_checks >= confirm and not paged: + return "page" + if paged and unhealthy_checks == 0: + return "recover" + return None + + def heartbeat_loop(stop: threading.Event, env: dict) -> None: paths = comms_lib.Paths.from_env() - last_alert = 0 + paged_last_ts: int | None = None # the stale heartbeat's last pulse when we paged + unhealthy_checks = 0 log("heartbeat loop started (slack)") while not stop.is_set(): try: @@ -721,19 +1025,22 @@ def heartbeat_loop(stop: threading.Event, env: dict) -> None: except json.JSONDecodeError: hb = {} last_ts = int(hb.get("last_pulse_ts") or 0) - if last_ts > 0: + if last_ts > 0 and target: age = int(time.time()) - last_ts - stale = age > stale_sec - bad = hb.get("status") in {"frozen", "stale_world", "respawn-requested"} - now = int(time.time()) - if (stale or bad) and now - last_alert >= HEARTBEAT_DEDUP_SEC and target: + unhealthy = (age > stale_sec + or hb.get("status") in {"frozen", "stale_world", "respawn-requested"}) + unhealthy_checks = unhealthy_checks + 1 if unhealthy else 0 + action = heartbeat_action(unhealthy_checks, paged_last_ts is not None) + if action == "page": body = comms_lib.fmt_heartbeat_alert(hb, age) - send_argv = _send_args(body, "urgent", target, None) - rc, _, err = cli(send_argv, timeout=30, env=env) - last_alert = now + rc, _, _err = cli(_send_args(body, "urgent", target, None), timeout=30, env=env) + paged_last_ts = last_ts log(f"heartbeat-stale page age={age}s rc={rc}") - elif not (stale or bad): - last_alert = 0 # healthy → re-arm + elif action == "recover": + body = comms_lib.fmt_heartbeat_recovered(hb, max(0, last_ts - paged_last_ts)) + rc, _, _err = cli(_send_args(body, "action", target, None), timeout=30, env=env) + paged_last_ts = None + log(f"heartbeat recovered rc={rc}") comms_lib.write_comms_heartbeat(paths, status="active", pulse_idx=0, note="listen-daemon") stop.wait(HEARTBEAT_CHECK_SEC) diff --git a/bin/comms_lib.py b/bin/comms_lib.py index ff42ecf..42c313f 100644 --- a/bin/comms_lib.py +++ b/bin/comms_lib.py @@ -215,39 +215,120 @@ def escape_mrkdwn(s: str) -> str: return s.replace("&", "&").replace("<", "<").replace(">", ">") +def _ref_footer(*refs: Any) -> str: + """The trailing italic line: IDs and labels you only need to look something + up, kept after the message itself. Empty and placeholder values are left out.""" + parts = [escape_mrkdwn(str(r)) for r in refs if r not in (None, "", "-", "?")] + return f"_{' · '.join(parts)}_" if parts else "" + + +def _quote(text: str) -> str: + """Slack blockquote, so quoted words (the agent's, or recorded evidence) + read apart from Assistant's own sentence. Blank lines are skipped, so empty + text quotes to "", which the formatters' line join drops.""" + return "\n".join(f"> {ln}" for ln in text.splitlines() if ln.strip()) + + +def _clip(text: str, limit: int) -> str: + text = text.strip() + return text if len(text) <= limit else text[:limit].rstrip() + "…" + + +# kind → (what Assistant did, as a past-tense phrase; what it set out to do). +# Covers the kinds that still reach Slack after comms-listen's suppression +# (routine, receipt, and housekeeping kinds never post); anything else reads +# generically. +_ACTION_PHRASES: dict[str, tuple[str, str]] = { + "ready_for_merge": ("asked a workspace to merge its PR", "ask a workspace to merge its PR"), + "self-update": ("updated Assistant to the latest code", "update Assistant to the latest code"), + "self-update-syntax-fail": ("updated Assistant to the latest code", + "update Assistant to the latest code"), + "strategist-context": ("started researching a decision that's waiting on you", + "research a decision that's waiting on you"), + "strategist-context-wrote": ("added background to your brief for a decision that's waiting on you", + "add background to your brief for a decision that's waiting on you"), + "strategist-autounpause": ("turned suggestion drafting back on", + "turn suggestion drafting back on"), + "goal-edit": ("updated your goals", "update your goals"), + "policy-bootstrap-upgrade": ("added new built-in rules for handling events", + "add new built-in rules for handling events"), +} +_GENERIC_ACTION = ("took an automatic step", "take an automatic step") + + +def _action_sentence(kind: str, outcome: str) -> str: + """One plain sentence: what Assistant did and whether it worked.""" + did, attempt = _ACTION_PHRASES.get(kind, _GENERIC_ACTION) + if outcome == "verified": + return f"I {did}." + if outcome == "failed": + return f"I tried to {attempt}, but it didn't work." + if outcome == "rejected": + return f"I tried to {attempt}, but it was turned down." + if outcome == "skipped": + return f"I didn't {attempt} this time." + return f"I tried to {attempt} (result: {escape_mrkdwn(outcome)})." + + def fmt_action_line(entry: dict[str, Any]) -> str: - """Render one ledger entry for Slack. screen_read evidence is flagged - because Assistant itself rejects it — the flag travels with the message.""" - kind = entry.get("kind", "?") - key = entry.get("key", "?") - ws = entry.get("ws_ref") or "-" - td = entry.get("td") or "-" - outcome = entry.get("outcome", "?") - via = entry.get("verified_via") or "?" - pulse = entry.get("pulse_idx", "?") - evidence = (entry.get("evidence") or "")[:200] - via_marker = "(!)screen_read" if via == "screen_read" else via - outcome_marker = { - "verified": "ok", "failed": "fail", "skipped": "skip", "rejected": "rej", - }.get(outcome, outcome) - return ( - f"*[{escape_mrkdwn(str(kind))}]* {outcome_marker} `{escape_mrkdwn(str(key))}`\n" - f"ws={escape_mrkdwn(str(ws))} td={escape_mrkdwn(str(td))} pulse={pulse} " - f"via={escape_mrkdwn(via_marker)}\n" - f"_{escape_mrkdwn(evidence)}_" - ) + """Render one ledger entry for Slack: a plain sentence about what Assistant + did, then the refs. When something went wrong the recorded evidence is the + news, so it's quoted under the sentence; otherwise it's machine detail and + leads the footer. screen_read evidence is flagged because Assistant itself + rejects it as proof — the flag travels with the message.""" + kind = str(entry.get("kind") or "?") + outcome = str(entry.get("outcome") or "?") + evidence = _clip(entry.get("evidence") or "", 200) + went_wrong = outcome in ("failed", "rejected") + lines = [_action_sentence(kind, outcome)] + if entry.get("verified_via") == "screen_read": + lines.append("Heads up: I only confirmed this by reading the screen, " + "which isn't reliable proof.") + if went_wrong: + lines.append(_quote(escape_mrkdwn(evidence))) + pulse = entry.get("pulse_idx") + lines.append(_ref_footer( + None if went_wrong else " ".join(evidence.split()), + entry.get("ws_ref"), kind, entry.get("key"), entry.get("td"), + f"pulse {pulse}" if pulse is not None else None)) + return "\n".join(ln for ln in lines if ln) + + +# Heartbeat statuses the pager alerts on even when the heartbeat is fresh. +_HEARTBEAT_STATUS_NOTES = { + "frozen": "it reports that it's frozen", + "stale_world": "it's working from an out-of-date view of your workspaces", + "respawn-requested": "it asked to be restarted", +} def fmt_heartbeat_alert(hb: dict[str, Any], age_sec: int) -> str: - ws = str(hb.get('ws_ref', '?')) - status = str(hb.get('status', '?')) - last = str(hb.get('last_pulse_iso', '?')) + """Page when the pulse loop stops or reports a bad status. A bad status can + arrive while runs are still recent, so that case names the status instead + of claiming the loop stopped.""" + last = escape_mrkdwn(str(hb.get("last_pulse_iso") or "unknown")) age = fmt_age(age_sec) - return ( - f"*Assistant heartbeat stale*\n" - f"ws={escape_mrkdwn(ws)} status={escape_mrkdwn(status)}\n" - f"last pulse {age} ago ({escape_mrkdwn(last)})" - ) + note = _HEARTBEAT_STATUS_NOTES.get(str(hb.get("status"))) + if note: + return (f"*Assistant's main loop needs a look* — {note}. Last run {age} ago " + f"({last}). I'll post again when it's back.") + return (f"*Assistant's main loop has stopped* — no run for {age} (last run {last}). " + f"I'll post again when it's back.") + + +def fmt_heartbeat_recovered(hb: dict[str, Any], down_sec: int) -> str: + """The all-clear that follows a heartbeat page, so a page never dangles.""" + latest = hb.get("last_pulse_iso") + since = f" (latest run {escape_mrkdwn(str(latest))})" if latest else "" + return f"*Assistant's main loop is running again* after {fmt_age(down_sec)}{since}." + + +# signal_type → what the workspace is doing, as the rest of the headline. +_SIGNAL_HEADLINES = { + "needs_input": "needs your input.", + "work_complete": "looks done.", + "pattern_match": "showed something I watch for.", +} def fmt_workspace_signal(item: dict[str, Any]) -> str: @@ -255,24 +336,25 @@ def fmt_workspace_signal(item: dict[str, Any]) -> str: Slack. The watcher drops these the instant cmux reports a workspace needs input or finished a notable turn — so the ping arrives in seconds. - Lead with the outcome (what the workspace needs / did), then the workspace - ref, then the screen snippet — the work first, the infra label second.""" + Lead with the work: the workspace's title and what it needs, then the + agent's own last words (or the screen snippet when those are missing). + The workspace ref and signal name go last, in the footer.""" signal_type = item.get("signal_type") or item.get("signal") or "?" - ws_ref = item.get("ws_ref") or "?" - pattern = item.get("pattern_matched") or item.get("signal") or "?" - snippet = (item.get("screen_snippet") or "").strip() - headline = { - "needs_input": "needs your input", - "work_complete": "work looks complete", - "pattern_match": "hit a watched signal", - }.get(signal_type, signal_type) - body = ( - f"*{escape_mrkdwn(str(ws_ref))} {escape_mrkdwn(str(headline))}*\n" - f"signal=`{escape_mrkdwn(str(pattern))}`" - ) - if snippet: - body += f"\n_{escape_mrkdwn(snippet[:400])}_" - return body + pattern = item.get("pattern_matched") or item.get("signal") + # A `*` inside the title would close the bold early. + title = (item.get("ws_title") or "").replace("*", "").strip() + who = f"*{escape_mrkdwn(title)}*" if title else "A workspace" + message = _clip(item.get("last_message") or "", 400) + snippet = _clip(item.get("screen_snippet") or "", 400) + asking = pattern == "AskUserQuestion" + if asking and message: + lines = [f"{who} is asking you: {escape_mrkdwn(message)}"] + else: + headline = ("has a question for you." if asking + else _SIGNAL_HEADLINES.get(signal_type, "sent an update.")) + lines = [f"{who} {headline}", _quote(escape_mrkdwn(message or snippet))] + lines.append(_ref_footer(item.get("ws_ref"), pattern)) + return "\n".join(ln for ln in lines if ln) def fmt_lesson_proposal(entry: dict[str, Any]) -> str: diff --git a/bin/comms_session.py b/bin/comms_session.py index 68257eb..c9913a3 100644 --- a/bin/comms_session.py +++ b/bin/comms_session.py @@ -12,9 +12,11 @@ This module splits cleanly: - PURE logic (registry r/w, transcript reply-extraction, should_clear, - newest-transcript resolution) — unit-tested, no cmux. - - cmux I/O (spawn, feed, clear) — thin wrappers over the same RPC pattern - pulse.py uses to drive Assistant. Validated live, not mocked. + submission confirmation, prompt-box parsing, workspace liveness) — + unit-tested, no cmux. + - cmux I/O (spawn, submit, clear, liveness probes) — thin wrappers over the + same RPC pattern pulse.py uses to drive Assistant. Validated live, not + mocked. Transport-agnostic: this file knows nothing about Slack vs any other transport. The daemon composes the per-message feed string; this only manages the session. @@ -152,6 +154,24 @@ def _positive_int_env(name: str, default: int) -> int: # soon as the marker appears. READY_ATTEMPTS = _positive_int_env("COMMS_READY_ATTEMPTS", 90) +# The boot banner is the first thing Claude paints, before the prompt box exists, +# and keys typed that early can be lost. After the banner, wait up to this many +# seconds for the bottom status bar — the sign the prompt box is drawn. +INPUT_READY_SEC = _positive_int_env("COMMS_INPUT_READY_SEC", 15) +_INPUT_READY_RE = {agent_session.CLAUDE: re.compile(r"⏵⏵ bypass permissions on")} + +# Every prompt the daemon types is confirmed against the transcript. Enter can be +# lost or turn into a newline (2026-09-27: a boot prompt and a Slack message sat +# typed in the box for two hours), so up to SUBMIT_ATTEMPTS Enters are tried, +# waiting SUBMIT_WAIT_SEC after each for the transcript to record the prompt. +SUBMIT_ATTEMPTS = _positive_int_env("COMMS_SUBMIT_ATTEMPTS", 3) +SUBMIT_WAIT_SEC = _positive_int_env("COMMS_SUBMIT_WAIT_SEC", 15) + +# How many times to look at a workspace cmux didn't answer about, and how long to +# wait between looks, before calling its state unknown. +LIVENESS_ATTEMPTS = _positive_int_env("COMMS_LIVENESS_ATTEMPTS", 3) +LIVENESS_RETRY_SEC = _positive_int_env("COMMS_LIVENESS_RETRY_SEC", 3) + # --------------------------------------------------------------------------- registry (pure) @@ -220,14 +240,6 @@ def project_dir_for_cwd(cwd: str, agent: str = agent_session.CLAUDE) -> Path: return agent_session.confirm_dir(agent, cwd, home=HOME) -def newest_transcript(cwd: str, agent: str = agent_session.CLAUDE) -> str | None: - pdir = project_dir_for_cwd(cwd, agent) - if not pdir.is_dir(): - return None - jsonls = sorted(pdir.glob("*.jsonl"), key=lambda p: p.stat().st_mtime, reverse=True) - return str(jsonls[0]) if jsonls else None - - def last_assistant_text(transcript_path: str | Path) -> str | None: """Extract the most recent assistant turn's text content from a transcript. Returns None if there is no assistant turn yet. Schema-agnostic: the role is @@ -266,20 +278,6 @@ def last_assistant_text(transcript_path: str | Path) -> str | None: return last_text -def transcript_line_count(transcript_path: str | Path) -> int: - """Count non-blank lines — a cheap 'has the transcript grown?' signal used - to detect that the session produced a new turn after we fed it.""" - p = Path(transcript_path) - if not p.exists(): - return 0 - n = 0 - with open(p) as f: - for line in f: - if line.strip(): - n += 1 - return n - - def should_clear(transcript_path: str | Path, threshold: float = CLEAR_THRESHOLD, agent: str = agent_session.CLAUDE, @@ -301,6 +299,189 @@ def should_clear(transcript_path: str | Path, return comms_lib.context_fraction(tokens) >= threshold +# --------------------------------------------------------------------------- submission (pure) + +# Only the transcript tail is read: a submitted prompt is always among the +# newest records, and warm transcripts grow to megabytes. +TRANSCRIPT_TAIL_BYTES = 262_144 +_RULE_RE = re.compile(r"^\s*─{8,}\s*$") +_WS_RE = re.compile(r"\s+") + + +def input_box_text(screen: str) -> str | None: + """The text in Claude's prompt box, or None when the screen shows no box. + + The box is the region between the last two horizontal rules, and its first + line starts with ❯. Earlier prompts in the scrollback also start with ❯ but + aren't fenced by rules, so they never match. Wrapped lines are joined.""" + lines = screen.splitlines() + rules = [i for i, line in enumerate(lines) if _RULE_RE.match(line)] + if len(rules) < 2: + return None + body = lines[rules[-2] + 1:rules[-1]] + if not body or not body[0].lstrip().startswith("❯"): + return None + first = body[0].lstrip()[1:] + return " ".join(part.strip() for part in [first, *body[1:]]).strip() + + +def box_holds(box: str | None, marker: str) -> bool: + """True if the prompt box still holds text the daemon typed: its marker + (compared without whitespace, so a line wrap inside it still matches) or a + collapsed paste.""" + if not box: + return False + return (_WS_RE.sub("", marker) in _WS_RE.sub("", box) + or "[Pasted text" in box) + + +def _prompt_text(rec: dict) -> str | None: + """The prompt text of a submitted user turn or a prompt queued while the + session was busy; None for every other record, including tool results.""" + if rec.get("type") == "queue-operation": + content = rec.get("content") + return content if isinstance(content, str) else None + if agent_session.record_role(rec) != "user": + return None + msg = rec.get("message") + content = msg.get("content") if isinstance(msg, dict) else None + if isinstance(content, str): + return content + if isinstance(content, list): + return "".join(b.get("text", "") for b in content + if isinstance(b, dict) and b.get("type") == "text") + return None + + +def transcript_has_submission(path: str | Path, marker: str) -> bool: + """True if the transcript at `path` records a prompt containing `marker`. + + Headless `claude -p` transcripts never count: the proofgate Stop hook runs + one in this cwd after every warm turn, and it quotes the warm session's + prompts back.""" + try: + with open(path, "rb") as f: + f.seek(0, os.SEEK_END) + f.seek(max(0, f.tell() - TRANSCRIPT_TAIL_BYTES)) + tail = f.read().decode("utf-8", "replace") + except OSError: + return False + for line in tail.splitlines(): + if marker not in line: + continue + try: + rec = json.loads(line) + except json.JSONDecodeError: + continue + if not isinstance(rec, dict) or rec.get("entrypoint") == "sdk-cli": + continue + text = _prompt_text(rec) + if text and marker in text: + return True + return False + + +def find_submission(project_dir: Path, marker: str, since: float) -> str | None: + """Path of the newest transcript in `project_dir` changed at or after + `since` that records a prompt containing `marker`, or None. + + Searches instead of trusting a remembered path, so a session that was + cleared, resumed, or bound to the wrong file is found again. Subagent + transcripts are skipped.""" + if not project_dir.is_dir(): + return None + candidates = [] + for p in project_dir.rglob("*.jsonl"): + if "subagents" in p.parts: + continue + try: + mtime = p.stat().st_mtime + except OSError: + continue + if mtime >= since: + candidates.append((mtime, p)) + for _mtime, p in sorted(candidates, reverse=True): + if transcript_has_submission(p, marker): + return str(p) + return None + + +def submit_until_confirmed(send_text, press_enter, read_box, confirmed, marker: str, + attempts: int = SUBMIT_ATTEMPTS, + wait_sec: float = SUBMIT_WAIT_SEC, + sleep=time.sleep, clock=time.monotonic) -> bool: + """Type a prompt once, press Enter, and wait until `confirmed()` sees it in + the transcript. Returns whether it did. + + If confirmation doesn't come and the box still holds the daemon's marker — + Enter was lost or became a newline — press Enter again, up to `attempts` + presses in all. The text is never retyped and Enter is never pressed on a + box that doesn't hold the marker, so a retry can't double-send a prompt or + submit someone else's half-typed input. All I/O is injected.""" + send_text() + sleep(0.5) + press_enter() + for attempt in range(attempts): + deadline = clock() + wait_sec + while clock() < deadline: + if confirmed(): + return True + sleep(1) + if attempt + 1 < attempts and box_holds(read_box(), marker): + press_enter() + return confirmed() + + +def boot_instruction(boot_prompt: Path, nonce: str) -> str: + """The prompt that boots a warm session. The nonce makes each boot's text + unique, so confirmation can't match an earlier session's boot turn.""" + return f"Read {boot_prompt} in full and execute every instruction in it. [boot {nonce}]" + + +# --------------------------------------------------------------------------- liveness (pure) + +ALIVE = "alive" +GONE = "gone" +UNKNOWN = "unknown" + +# What cmux prints for a workspace ref it doesn't know. Every other failure — a +# refused socket connection, a timeout — means cmux didn't answer, which says +# nothing about the workspace (2026-09-27: a napping cmux refused connections +# for up to two hours, and each refusal used to close a healthy warm session). +_MISSING_WORKSPACE_MARKERS = ("invalid_params", "Missing or invalid workspace") + + +def classify_tree_result(rc: int, err: str) -> str: + """ALIVE, GONE, or UNKNOWN for one `cmux tree --workspace` result.""" + if rc == 0: + return ALIVE + if any(m in (err or "") for m in _MISSING_WORKSPACE_MARKERS): + return GONE + return UNKNOWN + + +def ref_listed(text: str, ws_ref: str) -> bool: + """True if `ws_ref` appears as a whole ref in `text`, so workspace:25 + doesn't match workspace:258.""" + return re.search(rf"{re.escape(ws_ref)}(?!\d)", text) is not None + + +def resolve_workspace_state(probe, attempts: int = LIVENESS_ATTEMPTS, + retry_sec: float = LIVENESS_RETRY_SEC, + sleep=time.sleep) -> str: + """Run `probe` until it returns ALIVE or GONE, waiting `retry_sec` between + tries; UNKNOWN if cmux never gives an answer. Short cmux blips clear within + a few seconds, so one retry saves a session a single failed look would + have closed.""" + for attempt in range(attempts): + state = probe() + if state != UNKNOWN: + return state + if attempt + 1 < attempts: + sleep(retry_sec) + return UNKNOWN + + # --------------------------------------------------------------------------- cmux I/O (live) # # Same RPC pattern pulse.py uses. Kept thin; validated live, not mocked. @@ -323,10 +504,23 @@ def _surface_read_text(paths: comms_lib.Paths, surface_ref: str, lines: int = 20 return d.get("text", "") or "" -def cmux_alive(paths: comms_lib.Paths, ws_ref: str) -> bool: # pragma: no cover - live cmux I/O - rc, _, _ = comms_lib.run_cmd( +def _probe_workspace(paths: comms_lib.Paths, ws_ref: str) -> str: # pragma: no cover - live cmux I/O + """One look at a workspace. When `tree` fails without saying the ref is + unknown, a successful `list-workspaces` still settles it either way.""" + rc, _, err = comms_lib.run_cmd( [str(paths.cmux_bin), "tree", "--workspace", ws_ref, "--json"], timeout=10) - return rc == 0 + state = classify_tree_result(rc, err) + if state != UNKNOWN: + return state + rc, out, _ = comms_lib.run_cmd([str(paths.cmux_bin), "list-workspaces"], timeout=10) + if rc != 0: + return UNKNOWN + return ALIVE if ref_listed(out, ws_ref) else GONE + + +def workspace_state(paths: comms_lib.Paths, ws_ref: str) -> str: # pragma: no cover - live cmux I/O + """ALIVE, GONE, or UNKNOWN (cmux didn't answer) for a warm workspace.""" + return resolve_workspace_state(lambda: _probe_workspace(paths, ws_ref)) def close_own_workspace(paths: comms_lib.Paths, ws_ref: str, log=lambda m: None) -> None: # pragma: no cover - live cmux I/O @@ -347,7 +541,7 @@ def close_own_workspace(paths: comms_lib.Paths, ws_ref: str, log=lambda m: None) if rc != 0: return # Title guard: only ever close our own warm session, never an arbitrary ref. - is_warm = any(ws_ref in line and SESSION_TITLE in line for line in out.splitlines()) + is_warm = any(ref_listed(line, ws_ref) and SESSION_TITLE in line for line in out.splitlines()) if not is_warm: log(f"skip close {ws_ref}: not a '{SESSION_TITLE}' workspace (ref reissued?)") return @@ -357,13 +551,52 @@ def close_own_workspace(paths: comms_lib.Paths, ws_ref: str, log=lambda m: None) else f"close {ws_ref} rc={rc}: {err.strip()[:120]}") -def feed(paths: comms_lib.Paths, surface_ref: str, text: str) -> None: # pragma: no cover - live cmux I/O - """Type text into the warm session and submit. Strip trailing newline first - (send_text streams keystrokes; a trailing \\n auto-submits mid-paste), then - an explicit Enter — exactly pulse.py's delivery sequence.""" - _cmux_rpc(paths, "surface.send_text", {"surface_id": surface_ref, "text": text.rstrip("\n")}) - time.sleep(0.5) - _cmux_rpc(paths, "surface.send_key", {"surface_id": surface_ref, "key": "enter"}) +def send_enter(paths: comms_lib.Paths, surface_ref: str) -> None: # pragma: no cover - live cmux I/O + """Submit whatever is in the prompt box by writing a carriage return to the + terminal. `surface.send_key enter` reports success on a warm workspace that + was never shown on screen, yet the prompt stays unsent (reproduced + 2026-09-28 on a --focus false workspace); a "\\r" through send_text submits + it at once.""" + _cmux_rpc(paths, "surface.send_text", {"surface_id": surface_ref, "text": "\r"}) + + +def submit(paths: comms_lib.Paths, surface_ref: str, text: str, marker: str, + confirmed) -> bool: # pragma: no cover - live cmux I/O + """Type text into the warm session and press Enter until `confirmed()` sees + it in the transcript (see submit_until_confirmed). The trailing newline is + stripped: send_text streams keystrokes, so a trailing \\n would submit + mid-paste.""" + return submit_until_confirmed( + send_text=lambda: _cmux_rpc(paths, "surface.send_text", + {"surface_id": surface_ref, "text": text.rstrip("\n")}), + press_enter=lambda: send_enter(paths, surface_ref), + read_box=lambda: input_box_text(_surface_read_text(paths, surface_ref, lines=60)), + confirmed=confirmed, + marker=marker, + ) + + +def deliver_boot(paths: comms_lib.Paths, surface_ref: str, cwd: str, boot_prompt: Path, + agent: str) -> str | None: # pragma: no cover - live cmux I/O + """Type the boot prompt and return the transcript that recorded it, or None + if it was never submitted. The session is bound to that exact file — never + to whichever transcript happens to be newest, which on 2026-09-27/28 was + often another session's.""" + nonce = f"{time.strftime('%Y%m%dT%H%M%SZ', time.gmtime())}-{os.getpid()}" + marker = f"[boot {nonce}]" + project_dir = project_dir_for_cwd(cwd, agent) + since = time.time() - 1 + found: list[str] = [] + + def confirmed() -> bool: + hit = find_submission(project_dir, marker, since) + if hit: + found.append(hit) + return hit is not None + + if not submit(paths, surface_ref, boot_instruction(boot_prompt, nonce), marker, confirmed): + return None + return found[-1] if found else None def clear_session(paths: comms_lib.Paths, sess: dict, boot_prompt: Path, @@ -374,12 +607,14 @@ def clear_session(paths: comms_lib.Paths, sess: dict, boot_prompt: Path, conversation.jsonl (the boot prompt tells the session to reconstruct it), so a reset loses nothing. - claude — in-place /clear + resume: send /clear (as text, then an explicit - Enter keystroke; a trailing newline inside send_text does NOT reliably submit - a slash command), POLL for the post-clear "Welcome back" screen (feeding - during the ~2s reset window gets keystrokes swallowed), re-deliver the boot - prompt, then update the registry with the new transcript. The workspace and - surface are unchanged. + claude — in-place /clear + resume: send /clear (as text, then a separate + carriage return; a trailing newline inside the same send_text does NOT + reliably submit a slash command), POLL the bottom of the screen for the + post-clear welcome and an empty prompt box (feeding during the ~2s reset + window gets keystrokes swallowed; a full-history read could match the old + banner), re-deliver the boot prompt, then bind the registry to the + transcript that recorded it. The workspace and surface are unchanged. If + the boot prompt never lands, fall back to a lossless respawn. droid — respawn: Droid has no /clear slash command with the same semantics, so the lossless equivalent is to close this warm workspace and spawn a fresh @@ -393,22 +628,23 @@ def clear_session(paths: comms_lib.Paths, sess: dict, boot_prompt: Path, surface_ref = sess["surface_ref"] _cmux_rpc(paths, "surface.send_text", {"surface_id": surface_ref, "text": "/clear"}) time.sleep(0.5) - _cmux_rpc(paths, "surface.send_key", {"surface_id": surface_ref, "key": "enter"}) + send_enter(paths, surface_ref) for _ in range(15): time.sleep(1) - screen = _surface_read_text(paths, surface_ref) - if "Welcome back" in screen or "Tips for getting started" in screen: + screen = _surface_read_text(paths, surface_ref, lines=40) + welcomed = "Welcome back" in screen or "Tips for getting started" in screen + if welcomed and input_box_text(screen) == "": break time.sleep(1) - instruction = f"Read {boot_prompt} in full and execute every instruction in it." - feed(paths, surface_ref, instruction) - - new_t = newest_transcript(sess["cwd"], agent) - if new_t: - write_session(paths, sess["ws_ref"], surface_ref, sess["cwd"], new_t, - agent=agent) + transcript = deliver_boot(paths, surface_ref, sess["cwd"], boot_prompt, agent) + if not transcript: + log(f"boot prompt after /clear never submitted in {sess['ws_ref']} — respawning") + close_own_workspace(paths, sess["ws_ref"], log=log) + clear_session_registry(paths) + return spawn_session(paths, boot_prompt, log=log, agent=agent) or sess + write_session(paths, sess["ws_ref"], surface_ref, sess["cwd"], transcript, agent=agent) return read_session(paths) or sess @@ -646,9 +882,9 @@ def spawn_session(paths: comms_lib.Paths, boot_prompt: Path, log=lambda m: None, Claude's behavior is byte-identical to before while Droid gets its own.""" agent = agent or agent_session.warm_agent() cmux = str(paths.cmux_bin) - rc, _, _ = comms_lib.run_cmd([cmux, "ping"], timeout=10) + rc, _, err = comms_lib.run_cmd([cmux, "ping"], timeout=10) if rc != 0: - log("cmux not running — cannot spawn warm session") + log(f"cmux isn't answering ({err.strip()[:120]}) — cannot spawn warm session") return None cwd = str(DISPATCH_CWD) @@ -694,19 +930,15 @@ def spawn_session(paths: comms_lib.Paths, boot_prompt: Path, log=lambda m: None, return None surface_ref = sm.group(0) - project_dir = project_dir_for_cwd(cwd, agent) - project_dir.mkdir(parents=True, exist_ok=True) - before = {p.name for p in project_dir.glob("*.jsonl")} - # Readiness gate: poll the boot screen for the per-agent ready marker # (banner or status bar), answering the first-launch trust prompt if/when it # shows. Both are delegated to await_ready so the ordering is unit-tested. def _answer_trust() -> None: _cmux_rpc(paths, "surface.send_text", {"surface_id": surface_ref, "text": "1"}) # send_text streams keystrokes; give "1" a beat to land before Enter so - # the selection isn't submitted empty (mirrors feed()'s proven pattern). + # the selection isn't submitted empty (mirrors submit()'s pattern). time.sleep(0.5) - _cmux_rpc(paths, "surface.send_key", {"surface_id": surface_ref, "key": "enter"}) + send_enter(paths, surface_ref) ready, trust_answered = await_ready( read_screen=lambda: _surface_read_text(paths, surface_ref), @@ -724,28 +956,21 @@ def _answer_trust() -> None: _abandon_failed_spawn(paths, ws_ref, log=log) return None - # Deliver the responder boot prompt by reference. - instruction = f"Read {boot_prompt} in full and execute every instruction in it." - feed(paths, surface_ref, instruction) - - # Confirm submission via a new transcript carrying the prompt path. - sig = str(boot_prompt)[:60] - transcript = None - for _ in range(30): - for name in {p.name for p in project_dir.glob("*.jsonl")} - before: - try: - if sig in (project_dir / name).read_text(): - transcript = str(project_dir / name) - break - except OSError: - continue - if transcript: - break - time.sleep(1) - + input_ready_re = _INPUT_READY_RE.get(agent) + if input_ready_re: + await_ready( + read_screen=lambda: _surface_read_text(paths, surface_ref, lines=40), + ready_re=input_ready_re, trust_marker=None, answer_trust=lambda: None, + attempts=INPUT_READY_SEC, + ) + + # Deliver the responder boot prompt by reference, and keep the session only + # if its transcript shows the prompt was submitted. + transcript = deliver_boot(paths, surface_ref, cwd, boot_prompt, agent) if not transcript: - transcript = newest_transcript(cwd, agent) - log(f"warm session {ws_ref} spawned but boot submission unconfirmed") + log(f"{agent} boot prompt never submitted in {ws_ref}/{surface_ref}") + _abandon_failed_spawn(paths, ws_ref, log=log) + return None # Record the EXACT id this session launched with — the same `resolved` tuple # _warm_launch used above, captured once so a mid-boot backend toggle can't diff --git a/docs/assistant-comms-onboarding.md b/docs/assistant-comms-onboarding.md index 139afb9..8474611 100644 --- a/docs/assistant-comms-onboarding.md +++ b/docs/assistant-comms-onboarding.md @@ -4,10 +4,10 @@ It runs as a single **event-driven daemon**, `bin/comms-listen.py`, kept alive by a `KeepAlive` LaunchAgent (no `StartInterval` — it listens, it does not tick). The daemon runs four concurrent loops in threads: -- **Inbound** — REST-polls Slack (`conversations.history`, ~3s) for new messages in your DM/channel and feeds each to a **warm cmux Claude session** that replies in seconds. -- **Outbound pings** — watches `actions-ledger.jsonl` for appends; formats + sends each new verified/failed action. No LLM, ~2s latency floor. -- **Inbox** — watches `~/.assistant/inbox` for cmux-watcher signals ("workspace needs input" / "work complete") and pings within seconds. kqueue-driven on macOS. -- **Heartbeat page** — every 60s, checks Assistant's heartbeat; pages you (urgent, templated) if it's stale or `status ∈ {frozen, stale_world, respawn-requested}`. No LLM, 30-min dedup. +- **Inbound** — REST-polls Slack (`conversations.history`, ~3s) for new messages in your DM/channel and feeds each to a **warm cmux Claude session** that replies in seconds. Each message waits in `comms/pending-inbound.json` until the session's transcript shows it arrived. If the session is down, the message is retried every 30s for up to 3 hours, and you get one "I'll answer as soon as it's back" note per outage. +- **Outbound pings** — watches `actions-ledger.jsonl` for appends; formats + sends each new verified/failed action in plain language. Housekeeping (expired decisions, stranded nudges, skips, strategist pauses) stays in the brief. At most 5 posts per pass, then one summary line. No LLM, ~2s latency floor. +- **Inbox** — watches `~/.assistant/inbox` for cmux-watcher signals ("workspace needs input" / "work complete") and pings within seconds, at most once per workspace per 15 minutes unless it's a real question. kqueue-driven on macOS. +- **Heartbeat page** — every 60s, checks Assistant's heartbeat. If it's stale or `status ∈ {frozen, stale_world, respawn-requested}` for two checks in a row, pages you once (urgent, templated), then posts once when it recovers. No LLM. Durable memory lives entirely on disk (`conversation.jsonl` + poll cursors), so a crash and `KeepAlive` respawn loses nothing. The channel is a **flat 1:1 line** — the assistant replies at top level, not in threads. @@ -35,12 +35,12 @@ Replies come from a **warm cmux Claude session** (Sonnet, scoped `--add-dir`) th | Trigger | Slack message | |---|---| -| Assistant appends a verified action to its ledger | `*[cleanup]* ok `assistant:close-clean:workspace:117`` … `via=jsonl_transcript` | -| Same, but evidence is `screen_read` (Assistant rejects this as weak) | `(!)screen_read` flag in `via=` | -| A workspace needs your input / finishes work | `* needs your input* signal=`…`` | -| Assistant heartbeat stale (>10 min) or status flips to `frozen`/`stale_world`/`respawn-requested` | `*Assistant heartbeat stale* status=frozen last pulse 12m ago` | +| Assistant appends a verified action to its ledger | `I asked a workspace to merge its PR.` with the refs in a trailing italic line | +| Same, but evidence is `screen_read` (Assistant rejects this as weak) | `Heads up: I only confirmed this by reading the screen, which isn't reliable proof.` | +| A workspace asks you a question / needs input / finishes work | `** is asking you: ` or `** needs your input.` with the agent's last message quoted | +| Assistant heartbeat stale (>20 min) or status flips to `frozen`/`stale_world`/`respawn-requested` | `*Assistant's main loop has stopped* — no run for 25m …`, then `*Assistant's main loop is running again* after 3h.` | -Heartbeat alerts dedupe at 30 min. Messages are Slack `mrkdwn`. +Messages are Slack `mrkdwn`. ## What you can text back diff --git a/src/assistant/config.py b/src/assistant/config.py index 909f5e1..4a6a59a 100644 --- a/src/assistant/config.py +++ b/src/assistant/config.py @@ -35,7 +35,6 @@ DEFAULT_STALE_HEARTBEAT_SEC = 1200 DEFAULT_HEARTBEAT_CHECK_SEC = 60 DEFAULT_LEDGER_POLL_SEC = 2.0 -DEFAULT_HEARTBEAT_DEDUP_SEC = 1800 # ─── fleet dispatch caps — THE single source of truth (Keel M4/M14) ────────── # @@ -83,7 +82,6 @@ class Config: stale_heartbeat_sec: int = DEFAULT_STALE_HEARTBEAT_SEC heartbeat_check_sec: int = DEFAULT_HEARTBEAT_CHECK_SEC ledger_poll_sec: float = DEFAULT_LEDGER_POLL_SEC - heartbeat_dedup_sec: int = DEFAULT_HEARTBEAT_DEDUP_SEC # slack comms (CommsSubsystem). bot_token is a property (env), never a field. target: str = "" @@ -212,8 +210,6 @@ def load(cls, path: str | Path | None = None, *, DEFAULT_HEARTBEAT_CHECK_SEC)), ledger_poll_sec=float(daemon.get("ledger_poll_sec", DEFAULT_LEDGER_POLL_SEC)), - heartbeat_dedup_sec=int(daemon.get("heartbeat_dedup_sec", - DEFAULT_HEARTBEAT_DEDUP_SEC)), target=target, allowed_targets=allowed, ) diff --git a/src/assistant/slack.py b/src/assistant/slack.py index ed4d911..a8504db 100644 --- a/src/assistant/slack.py +++ b/src/assistant/slack.py @@ -36,27 +36,84 @@ def escape_mrkdwn(s: str) -> str: return s.replace("&", "&").replace("<", "<").replace(">", ">") +def _ref_footer(*refs: Any) -> str: + """The trailing italic line: IDs and labels you only need to look something + up, kept after the message itself. Empty and placeholder values are left out.""" + parts = [escape_mrkdwn(str(r)) for r in refs if r not in (None, "", "-", "?")] + return f"_{' · '.join(parts)}_" if parts else "" + + +def _quote(text: str) -> str: + """Slack blockquote, so the recorded evidence reads apart from the sentence. + Blank lines are skipped, so empty text quotes to "", which the line join + drops.""" + return "\n".join(f"> {ln}" for ln in text.splitlines() if ln.strip()) + + +def _clip(text: str, limit: int) -> str: + text = text.strip() + return text if len(text) <= limit else text[:limit].rstrip() + "…" + + +# kind → (what Assistant did, as a past-tense phrase; what it set out to do). +# Covers the kinds that still reach Slack after the broadcast suppression +# (routine, receipt, and housekeeping kinds never post); anything else reads +# generically. +_ACTION_PHRASES: dict[str, tuple[str, str]] = { + "ready_for_merge": ("asked a workspace to merge its PR", "ask a workspace to merge its PR"), + "self-update": ("updated Assistant to the latest code", "update Assistant to the latest code"), + "self-update-syntax-fail": ("updated Assistant to the latest code", + "update Assistant to the latest code"), + "strategist-context": ("started researching a decision that's waiting on you", + "research a decision that's waiting on you"), + "strategist-context-wrote": ("added background to your brief for a decision that's waiting on you", + "add background to your brief for a decision that's waiting on you"), + "strategist-autounpause": ("turned suggestion drafting back on", + "turn suggestion drafting back on"), + "goal-edit": ("updated your goals", "update your goals"), + "policy-bootstrap-upgrade": ("added new built-in rules for handling events", + "add new built-in rules for handling events"), +} +_GENERIC_ACTION = ("took an automatic step", "take an automatic step") + + +def _action_sentence(kind: str, outcome: str) -> str: + """One plain sentence: what Assistant did and whether it worked.""" + did, attempt = _ACTION_PHRASES.get(kind, _GENERIC_ACTION) + if outcome == "verified": + return f"I {did}." + if outcome == "failed": + return f"I tried to {attempt}, but it didn't work." + if outcome == "rejected": + return f"I tried to {attempt}, but it was turned down." + if outcome == "skipped": + return f"I didn't {attempt} this time." + return f"I tried to {attempt} (result: {escape_mrkdwn(outcome)})." + + def fmt_action_line(entry: dict[str, Any]) -> str: - """Render one actions-ledger entry for Slack. screen_read evidence is flagged - because the Assistant itself rejects it — the flag travels with the message.""" - kind = entry.get("kind", "?") - key = entry.get("key", "?") - ws = entry.get("ws_ref") or "-" - td = entry.get("td") or "-" - outcome = entry.get("outcome", "?") - via = entry.get("verified_via") or "?" - pulse = entry.get("pulse_idx", "?") - evidence = (entry.get("evidence") or "")[:200] - via_marker = "(!)screen_read" if via == "screen_read" else via - outcome_marker = { - "verified": "ok", "failed": "fail", "skipped": "skip", "rejected": "rej", - }.get(outcome, outcome) - return ( - f"*[{escape_mrkdwn(str(kind))}]* {outcome_marker} `{escape_mrkdwn(str(key))}`\n" - f"ws={escape_mrkdwn(str(ws))} td={escape_mrkdwn(str(td))} pulse={pulse} " - f"via={escape_mrkdwn(via_marker)}\n" - f"_{escape_mrkdwn(evidence)}_" - ) + """Render one actions-ledger entry for Slack: a plain sentence about what + Assistant did, then the refs. When something went wrong the recorded + evidence is the news, so it's quoted under the sentence; otherwise it's + machine detail and leads the footer. screen_read evidence is flagged because + the Assistant itself rejects it as proof — the flag travels with the + message.""" + kind = str(entry.get("kind") or "?") + outcome = str(entry.get("outcome") or "?") + evidence = _clip(entry.get("evidence") or "", 200) + went_wrong = outcome in ("failed", "rejected") + lines = [_action_sentence(kind, outcome)] + if entry.get("verified_via") == "screen_read": + lines.append("Heads up: I only confirmed this by reading the screen, " + "which isn't reliable proof.") + if went_wrong: + lines.append(_quote(escape_mrkdwn(evidence))) + pulse = entry.get("pulse_idx") + lines.append(_ref_footer( + None if went_wrong else " ".join(evidence.split()), + entry.get("ws_ref"), kind, entry.get("key"), entry.get("td"), + f"pulse {pulse}" if pulse is not None else None)) + return "\n".join(ln for ln in lines if ln) def fmt_age(seconds: int) -> str: @@ -71,14 +128,33 @@ def fmt_age(seconds: int) -> str: return f"{seconds // 86400}d" +# Heartbeat statuses the pager alerts on even when the heartbeat is fresh. +_HEARTBEAT_STATUS_NOTES = { + "frozen": "it reports that it's frozen", + "stale_world": "it's working from an out-of-date view of your workspaces", + "respawn-requested": "it asked to be restarted", +} + + def fmt_heartbeat_alert(hb: dict[str, Any], age_sec: int) -> str: - return ( - f"*Assistant heartbeat stale*\n" - f"ws={escape_mrkdwn(str(hb.get('ws_ref', '?')))} " - f"status={escape_mrkdwn(str(hb.get('status', '?')))}\n" - f"last pulse {fmt_age(age_sec)} ago " - f"({escape_mrkdwn(str(hb.get('last_pulse_iso', '?')))})" - ) + """Page when the pulse loop stops or reports a bad status. A bad status can + arrive while runs are still recent, so that case names the status instead + of claiming the loop stopped.""" + last = escape_mrkdwn(str(hb.get("last_pulse_iso") or "unknown")) + age = fmt_age(age_sec) + note = _HEARTBEAT_STATUS_NOTES.get(str(hb.get("status"))) + if note: + return (f"*Assistant's main loop needs a look* — {note}. Last run {age} ago " + f"({last}). I'll post again when it's back.") + return (f"*Assistant's main loop has stopped* — no run for {age} (last run {last}). " + f"I'll post again when it's back.") + + +def fmt_heartbeat_recovered(hb: dict[str, Any], down_sec: int) -> str: + """The all-clear that follows a heartbeat page, so a page never dangles.""" + latest = hb.get("last_pulse_iso") + since = f" (latest run {escape_mrkdwn(str(latest))})" if latest else "" + return f"*Assistant's main loop is running again* after {fmt_age(down_sec)}{since}." # ─── HTTP (the only network egress) ─────────────────────────────────────────── diff --git a/src/assistant/subsystems/comms.py b/src/assistant/subsystems/comms.py index dbc9f5a..f215054 100644 --- a/src/assistant/subsystems/comms.py +++ b/src/assistant/subsystems/comms.py @@ -20,7 +20,7 @@ 2. Heartbeat pager — every heartbeat_check_sec, read Assistant's pulse heartbeat.json; if stale (age > stale_heartbeat_sec) or status is bad, - send a templated urgent page, deduped to heartbeat_dedup_sec. + send one templated urgent page, then one message when it recovers. When Slack isn't configured (no token / no target / target not allowlisted), both jobs still run their read loops but skip the actual send — so an @@ -36,6 +36,8 @@ from . import Subsystem from .. import brief, conversation, ledger, slack +HOUSEKEEPING_KINDS = ("decision-transition", "strategist-autopause", "stranded", "skipped") + class CommsSubsystem(Subsystem): name = "comms" @@ -47,7 +49,7 @@ def __init__(self, *args, send_enabled: bool = True, **kwargs): self._send_enabled = send_enabled and self.config.has_slack self._reader = ledger.LedgerReader( self.config.ledger_path, self.config.ledger_cursor_path) - self._last_alert = 0 + self._paged_last_ts: int | None = None # the stale heartbeat's last pulse when paged self._broadcasts = 0 self._pages = 0 self._threads: list[threading.Thread] = [] @@ -100,6 +102,11 @@ def _broadcast_entry(self, entry: dict) -> None: if kind in brief.RECEIPT_KINDS: self.log.debug("suppressed receipt broadcast kind=%s key=%s", kind, key) return + # Housekeeping the brief already shows; mirrors comms-listen.py's + # HOUSEKEEPING_KINDS. + if kind in HOUSEKEEPING_KINDS: + self.log.debug("suppressed housekeeping broadcast kind=%s key=%s", kind, key) + return if kind == "self-update" and "skip" in key: self.log.debug("suppressed self-update-skip broadcast key=%s", key) return @@ -147,23 +154,27 @@ def _check_heartbeat(self) -> None: age = int(time.time()) - last_ts stale = age > self.config.stale_heartbeat_sec bad = hb.get("status") in {"frozen", "stale_world", "respawn-requested"} - now = int(time.time()) - if (stale or bad) and now - self._last_alert >= self.config.heartbeat_dedup_sec: - body = slack.fmt_heartbeat_alert(hb, age) - if self._send_enabled: - try: - slack.send(body, self.config.target, token=self.config.bot_token, - allowed=self.config.allowed_targets, kind="urgent") - except RuntimeError as e: - self.log.warning("heartbeat page failed target=%s: %s", - self.config.target, str(e)[:160]) - else: - self.log.info("would page: heartbeat stale age=%ss (send disabled)", age) - self._last_alert = now + if (stale or bad) and self._paged_last_ts is None: + self._send_heartbeat(slack.fmt_heartbeat_alert(hb, age), "urgent") + self._paged_last_ts = last_ts self._pages += 1 self.log.warning("heartbeat-stale page age=%ss", age) - elif not (stale or bad): - self._last_alert = 0 # healthy → re-arm + elif not (stale or bad) and self._paged_last_ts is not None: + down = max(0, last_ts - self._paged_last_ts) + self._send_heartbeat(slack.fmt_heartbeat_recovered(hb, down), "action") + self._paged_last_ts = None + self.log.info("heartbeat recovered after %ss", down) + + def _send_heartbeat(self, body: str, kind: str) -> None: + if not self._send_enabled: + self.log.info("would send heartbeat %s (send disabled)", kind) + return + try: + slack.send(body, self.config.target, token=self.config.bot_token, + allowed=self.config.allowed_targets, kind=kind) + except RuntimeError as e: + self.log.warning("heartbeat %s failed target=%s: %s", + kind, self.config.target, str(e)[:160]) def _read_heartbeat(self) -> dict: p = self.config.heartbeat_path diff --git a/tests/conftest.py b/tests/conftest.py index 958a974..2fd327e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,16 @@ """pytest config — make the bin/ scripts and the src/ package importable from tests.""" +import os import sys from pathlib import Path _ROOT = Path(__file__).resolve().parent.parent sys.path.insert(0, str(_ROOT / "bin")) sys.path.insert(0, str(_ROOT / "src")) + +# Point CMUX_BIN — which the comms daemon, the pulse, and the watchers use for +# every cmux call — at a binary that doesn't exist, so a test that misses a +# stub fails fast instead of driving the real cmux. On 2026-09-28 an unstubbed +# watchdog test spawned a live warm workspace and typed into it. Set before any +# test module imports, so import-time CMUX_BIN constants pick it up too; a test +# that needs a fake cmux still overrides it with monkeypatch.setenv. +os.environ["CMUX_BIN"] = str(_ROOT / "tests" / "no-real-cmux-in-tests") diff --git a/tests/test_cmux_watcher.py b/tests/test_cmux_watcher.py index 26c8ba7..9cbe89f 100644 --- a/tests/test_cmux_watcher.py +++ b/tests/test_cmux_watcher.py @@ -95,6 +95,42 @@ def test_priority_ordering(self): hits = bank.match("a thing happened") self.assertEqual(hits[0]["id"], "highp") # high sorts first + def _write_june_bank(self, ci_green_extra=None): + """The on-disk bank written before ci-green gained `suppress`.""" + ci_green = {"id": "ci-green", "regex": r"CI (is )?green", + "signal": "work_complete", "priority": "medium", + **(ci_green_extra or {})} + self.bank_path.write_text(json.dumps({"version": 1, "patterns": [ + ci_green, + {"id": "pr-opened", "regex": r"PR #\d+ opened", + "signal": "work_complete", "priority": "high"}, + ]})) + + def test_old_bank_inherits_default_suppress(self): + self._write_june_bank() + before = self.bank_path.read_text() + bank = self._bank() + by_id = {p["id"]: p for p in bank.patterns} + self.assertIs(by_id["ci-green"]["suppress"], True) + # pr-opened has no suppress in the defaults, so nothing is invented. + self.assertNotIn("suppress", by_id["pr-opened"]) + self.assertEqual(self.bank_path.read_text(), before, "the user's file must not be rewritten") + + def test_old_bank_ci_green_turn_end_is_silent(self): + self._write_june_bank() + bank = self._bank() + res = self.mod.handle_event( + _evt("agent.hook.Stop", request_id="rCI"), bank, + self.mod.WatcherState(cooldown_sec=0), FakeResolver(), + screen_reader=lambda ws: "All done — CI is green", + message_reader=MessageReaderSpy()) + self.assertIsNone(res, "a suppressed default must stay quiet in an old bank") + + def test_explicit_suppress_in_file_wins(self): + self._write_june_bank({"suppress": False}) + by_id = {p["id"]: p for p in self._bank().patterns} + self.assertIs(by_id["ci-green"]["suppress"], False) + class TestInboxDrop(unittest.TestCase): def setUp(self): @@ -128,6 +164,18 @@ def test_inbox_drop_atomic_and_shape(self): self.assertEqual(item["pattern_matched"], "awaiting-review") self.assertIn("ts", item) self.assertIn("screen_snippet", item) + self.assertNotIn("ws_title", item, "unresolved title must be omitted, not null") + self.assertNotIn("last_message", item) + + def test_inbox_drop_carries_title_and_last_message(self): + path = self.mod.drop_inbox_item( + "workspace:244", "needs_input", "AskUserQuestion", "snippet", + inbox_dir=self.inbox, ws_title="Green E2E Suite", + last_message="Should I rebase or merge main?") + item = json.loads(path.read_text()) + self.assertEqual(item["ws_title"], "Green E2E Suite") + self.assertEqual(item["last_message"], "Should I rebase or merge main?") + self.assertEqual(item["screen_snippet"], "snippet") def test_inbox_filename_prefix(self): path = self.mod.drop_inbox_item( @@ -139,7 +187,8 @@ def test_inbox_filename_prefix(self): # ─── cmux-watcher: event classification + end-to-end handling ───────────────── -def _evt(name, *, request_id="r1", workspace_id="UUID-1", cwd="/x", phase="completed"): +def _evt(name, *, request_id="r1", workspace_id="UUID-1", cwd="/x", phase="completed", + session_id="sess-1"): return { "type": "event", "name": name, @@ -148,11 +197,38 @@ def _evt(name, *, request_id="r1", workspace_id="UUID-1", cwd="/x", phase="compl "_opencode_request_id": request_id, "workspace_id": workspace_id, "cwd": cwd, + "session_id": session_id, "phase": phase, }, } +class FakeResolver: + """Never shells out: every UUID resolves to one ref and (optionally) a title.""" + + def __init__(self, ref="workspace:99", title=None): + self.ref = ref + self._title = title + + def resolve(self, uuid): + return self.ref if uuid else None + + def title(self, uuid): + return self._title if uuid else None + + +class MessageReaderSpy: + """Stands in for read_last_message; records each call's arguments.""" + + def __init__(self, reply=None): + self.reply = reply + self.calls = [] + + def __call__(self, cwd, session_id, *, question): + self.calls.append((cwd, session_id, question)) + return self.reply + + class TestEventHandling(unittest.TestCase): def setUp(self): self._tmp = TemporaryDirectory() @@ -170,13 +246,10 @@ def setUp(self): def tearDown(self): self._tmp.cleanup() - def _components(self): + def _components(self, title=None): bank = self.mod.PatternBank(self.assistant / "pattern_bank.json") state = self.mod.WatcherState(cooldown_sec=0) - # Resolver that never shells out: identity map. - resolver = mock.Mock() - resolver.resolve = lambda u: ("workspace:99" if u else None) - return bank, state, resolver + return bank, state, FakeResolver(title=title) def test_ack_and_heartbeat_ignored(self): self.assertIsNone(self.mod.classify_event({"type": "ack"})) @@ -194,6 +267,47 @@ def test_needs_input_always_drops(self): self.assertEqual(res["signal_type"], "needs_input") self.assertEqual(res["pattern_matched"], "AskUserQuestion") + def test_classify_passes_session_id_through(self): + cls = self.mod.classify_event(_evt("agent.hook.Stop", session_id="S-42")) + self.assertEqual(cls["session_id"], "S-42") + + def test_ask_user_question_reads_pending_question(self): + bank, state, resolver = self._components(title="Fix archself deferral door") + reader = MessageReaderSpy(reply="Should I rebase or merge main?") + res = self.mod.handle_event( + _evt("agent.hook.AskUserQuestion", cwd="/w/repo", session_id="S-1"), + bank, state, resolver, screen_reader=lambda ws: "Which option?", + message_reader=reader) + self.assertEqual(reader.calls, [("/w/repo", "S-1", True)]) + item = json.loads(Path(res["path"]).read_text()) + self.assertEqual(item["ws_title"], "Fix archself deferral door") + self.assertEqual(item["last_message"], "Should I rebase or merge main?") + + def test_notification_reads_last_text_not_question(self): + bank, state, resolver = self._components() + reader = MessageReaderSpy() + res = self.mod.handle_event( + _evt("agent.hook.Notification", request_id="rN2", session_id="S-2"), + bank, state, resolver, screen_reader=lambda ws: "waiting", + message_reader=reader) + self.assertEqual(reader.calls, [("/x", "S-2", False)]) + item = json.loads(Path(res["path"]).read_text()) + self.assertNotIn("ws_title", item) + self.assertNotIn("last_message", item) + + def test_turn_end_drop_carries_title_and_last_text(self): + bank, state, resolver = self._components(title="Green E2E Suite") + reader = MessageReaderSpy(reply="Opened the PR; CI is running.") + res = self.mod.handle_event( + _evt("agent.hook.Stop", request_id="rS2", session_id="S-3"), + bank, state, resolver, + screen_reader=lambda ws: "Done. PR #321 opened for review.", + message_reader=reader) + self.assertEqual(reader.calls, [("/x", "S-3", False)]) + item = json.loads(Path(res["path"]).read_text()) + self.assertEqual(item["ws_title"], "Green E2E Suite") + self.assertEqual(item["last_message"], "Opened the PR; CI is running.") + def test_notification_drops_needs_input(self): bank, state, resolver = self._components() res = self.mod.handle_event( @@ -241,8 +355,7 @@ def test_request_id_dedup(self): def test_cooldown_suppresses_repeat(self): bank = self.mod.PatternBank(self.assistant / "pattern_bank.json") state = self.mod.WatcherState(cooldown_sec=3600) # long cooldown - resolver = mock.Mock() - resolver.resolve = lambda u: "workspace:5" + resolver = FakeResolver(ref="workspace:5") a = self.mod.handle_event( _evt("agent.hook.Notification", request_id="c1"), bank, state, resolver, screen_reader=lambda ws: "x") @@ -264,6 +377,251 @@ def test_malformed_event_no_crash(self): self.fail(f"handle_event raised on {bad!r}: {e}") +# ─── screen snippet filtering ───────────────────────────────────────────────── + +class TestScreenSnippet(unittest.TestCase): + def setUp(self): + self._tmp = TemporaryDirectory() + home = Path(self._tmp.name) + self.mod = load_module("cmux_watcher_snip", "bin/cmux-watcher.py", { + "HOME": str(home), + "CMUX_WATCHER_ASSISTANT_DIR": str(home / ".assistant"), + "CMUX_PATTERN_BANK": str(home / ".assistant" / "pattern_bank.json"), + }) + + def tearDown(self): + self._tmp.cleanup() + + def test_drops_claude_code_chrome_keeps_content(self): + # Every noise line sits after real content, so a filter that stops + # working pushes its line into the snippet. + screen = "\n".join([ + "⏺ Merged #367 after both reviews came back clean.", + " │ PR │ State │ Tests │", + "❯ Great, leave this as a comment on #273", + "✽ Boogieing… (12m 1s · ↓ 48.9k tokens)", + "✻ Baked for 2m 10s · done 10:40 PM", + "✻ Brewed for 15m 33s · done 8:28 AM · 4 messages hidden (/focus to show)", + "❯", + "❯ ", + " architect-ffp/wt-348 fix/x │ ●114 ●1 │ context 43% │ $85.54 │ #6746dc4f", + "wt-ptt │ context 58% │ $129.24 │ #6746dc4f", + " ⏵⏵ bypass permissions on (shift+tab to cycle)", + "─────────────────────", + " " * 60 + "✔ Update installed · Restart to update", + " v1.0.85 downloaded · run /restart to apply · ? help", + "┃", + "╹▀▀▀▀▀▀▀▀━━━", + "╻▄▄▄▄▄▄▄▄", + " ┌────┬────┐", + " ├────┼────┤", + " └────┴────┘", + ]) + self.assertEqual(self.mod.last_lines(screen, n=10), "\n".join([ + "⏺ Merged #367 after both reviews came back clean.", + " │ PR │ State │ Tests │", + "❯ Great, leave this as a comment on #273", + ])) + + def test_keeps_lines_that_only_look_like_chrome(self): + screen = "\n".join([ + "⏺ Checking CI… still running", + "- Fixed for 3 users, reverted for 2m users", + "· Reading 3 files…", + "a │ b", + ]) + self.assertEqual(self.mod.last_lines(screen, n=10), screen) + + +# ─── workspace title resolution ─────────────────────────────────────────────── + +class TestWsRefResolver(unittest.TestCase): + def setUp(self): + self._tmp = TemporaryDirectory() + home = Path(self._tmp.name) + self.mod = load_module("cmux_watcher_res", "bin/cmux-watcher.py", { + "HOME": str(home), + "CMUX_WATCHER_ASSISTANT_DIR": str(home / ".assistant"), + "CMUX_PATTERN_BANK": str(home / ".assistant" / "pattern_bank.json"), + }) + + def tearDown(self): + self._tmp.cleanup() + + def test_resolves_ref_and_human_title(self): + # The live `cmux rpc workspace.list` shape: title carries a " [NN]" suffix. + listing = {"window_id": "W", "workspaces": [ + {"id": "aaaa-1", "ref": "workspace:244", "title": "Green E2E Suite [244]"}, + {"id": "bbbb-2", "ref": "workspace:7", "title": ""}, + {"id": "cccc-3", "ref": "workspace:259", "title": "Terminal [259]"}, + ]} + with mock.patch.object(self.mod, "_run", return_value=(0, json.dumps(listing), "")): + resolver = self.mod.WsRefResolver(clock=lambda: 1000.0) + self.assertEqual(resolver.resolve("AAAA-1"), "workspace:244") + self.assertEqual(resolver.title("AAAA-1"), "Green E2E Suite") + self.assertEqual(resolver.resolve("bbbb-2"), "workspace:7") + self.assertIsNone(resolver.title("bbbb-2"), "an empty title is not stored") + self.assertIsNone(resolver.title("cccc-3"), "cmux's default name says nothing") + self.assertIsNone(resolver.title(None)) + + def test_human_title_strips_only_the_ref_suffix(self): + self.assertEqual(self.mod.human_title("Fix [WIP] ruler [12]"), "Fix [WIP] ruler") + self.assertEqual(self.mod.human_title(None), "") + + +# ─── agent's last message (transcript tail) ─────────────────────────────────── + +def _assistant(*blocks): + return {"type": "assistant", "message": {"role": "assistant", "content": list(blocks)}} + + +def _user(*blocks): + return {"type": "user", "message": {"role": "user", "content": list(blocks)}} + + +def _ask(tool_id, question): + return {"type": "tool_use", "id": tool_id, "name": "AskUserQuestion", + "input": {"questions": [{"question": question, "header": "h", + "options": [{"label": "a"}, {"label": "b"}]}]}} + + +def _answer(tool_id): + return {"type": "tool_result", "tool_use_id": tool_id, "content": "a"} + + +class TestLastMessage(unittest.TestCase): + def setUp(self): + self._tmp = TemporaryDirectory() + self.home = Path(self._tmp.name) + self.mod = load_module("cmux_watcher_msg", "bin/cmux-watcher.py", { + "HOME": str(self.home), + "CMUX_WATCHER_ASSISTANT_DIR": str(self.home / ".assistant"), + "CMUX_PATTERN_BANK": str(self.home / ".assistant" / "pattern_bank.json"), + }) + self.projects = self.home / ".claude" / "projects" + + def tearDown(self): + self._tmp.cleanup() + + def _transcript(self, slug, session_id, records): + d = self.projects / slug + d.mkdir(parents=True, exist_ok=True) + p = d / f"{session_id}.jsonl" + p.write_text("".join(json.dumps(r) + "\n" for r in records)) + return p + + def test_project_slug_maps_every_non_alphanumeric(self): + cwd = self.home / "dev" / "assistant" / ".worktrees" / "comms_fix" + cwd.mkdir(parents=True) + expected = os.path.realpath(str(cwd)).replace("/", "-").replace(".", "-").replace("_", "-") + self.assertEqual(self.mod.claude_project_slug(str(cwd)), expected) + self.assertIn("--worktrees-comms-fix", expected) + + def test_transcript_path_direct_hit_skips_the_scan(self): + cwd = "/Users/me/dev/assistant/.worktrees/x" + want = self._transcript(self.mod.claude_project_slug(cwd), "sess-1", []) + with mock.patch.object(Path, "iterdir", side_effect=AssertionError("scanned all dirs")): + self.assertEqual(self.mod.transcript_path(cwd, "sess-1", self.projects), want) + + def test_transcript_path_finds_session_after_cwd_drift(self): + want = self._transcript("-Users-me-launch-dir", "sess-2", []) + self.assertEqual(self.mod.transcript_path("/Users/me/elsewhere", "sess-2", self.projects), want) + self.assertEqual(self.mod.transcript_path(None, "sess-2", self.projects), want) + self.assertIsNone(self.mod.transcript_path("/Users/me/elsewhere", "sess-404", self.projects)) + + def test_transcript_path_rejects_unsafe_or_missing_session_id(self): + self._transcript("-a", "ok", []) + # Without the id check this path-walks back into -a/ and finds ok.jsonl. + self.assertIsNone(self.mod.transcript_path("/a", "../-a/ok", self.projects)) + self.assertIsNone(self.mod.transcript_path("/a", None, self.projects)) + + def test_tail_records_reads_only_the_tail_and_skips_junk(self): + p = self.projects / "t.jsonl" + p.parent.mkdir(parents=True) + old = json.dumps({"n": "old", "pad": "x" * 500}) + p.write_text("\n".join([old, "not json", "[1, 2]", json.dumps({"n": "new"})]) + "\n") + self.assertEqual(self.mod.tail_records(p, max_bytes=200), [{"n": "new"}]) + self.assertEqual([r["n"] for r in self.mod.tail_records(p)], ["old", "new"]) + + def test_pending_question_returns_unanswered_newest(self): + records = [ + _assistant(_ask("t1", "Old question?")), + _user(_answer("t1")), + _assistant({"type": "text", "text": "Two ways to go."}), + _assistant(_ask("t2", "Should I rebase or merge main?")), + ] + self.assertEqual(self.mod.pending_question(records), "Should I rebase or merge main?") + + def test_pending_question_none_when_newest_is_answered(self): + records = [_assistant(_ask("t1", "Old question?")), _user(_answer("t1"))] + self.assertIsNone(self.mod.pending_question(records)) + + def test_pending_question_none_for_missing_or_blank_text(self): + blank = _ask("t1", " ") + empty = {"type": "tool_use", "id": "t2", "name": "AskUserQuestion", "input": {}} + self.assertIsNone(self.mod.pending_question([_assistant(blank)])) + self.assertIsNone(self.mod.pending_question([_assistant(empty)])) + self.assertIsNone(self.mod.pending_question([_assistant({"type": "text", "text": "hi"})])) + + def test_last_assistant_text_skips_user_and_non_text_blocks(self): + records = [ + _assistant({"type": "text", "text": "Opened the PR."}), + _assistant({"type": "text", "text": " "}, {"type": "tool_use", "name": "Bash", + "input": {}}), + {"type": "assistant", "message": {"role": "assistant", "content": "plain string"}}, + {"type": "summary", "summary": "not a turn"}, + _assistant({"type": "tool_result", "text": "tool output, not the agent"}), + _user({"type": "text", "text": "the user's reply"}), + ] + self.assertEqual(self.mod.last_assistant_text(records), "Opened the PR.") + self.assertIsNone(self.mod.last_assistant_text([_user({"type": "text", "text": "x"})])) + + def test_trim_words(self): + self.assertEqual(self.mod.trim_words("a\n\n b"), "a b") + long = "word " * 100 + trimmed = self.mod.trim_words(long, limit=23) + self.assertEqual(trimmed, "word word word word…") + + def test_read_last_message_end_to_end(self): + cwd = "/Users/me/dev/proj" + records = [ + _assistant({"type": "text", "text": "I found two ways.\n\nPick one."}), + _assistant(_ask("t9", "Should I rebase or merge main?")), + ] + self._transcript(self.mod.claude_project_slug(cwd), "sess-9", records) + self.assertEqual(self.mod.read_last_message(cwd, "sess-9", question=True), + "Should I rebase or merge main?") + self.assertEqual(self.mod.read_last_message(cwd, "sess-9", question=False), + "I found two ways. Pick one.") + self.assertIsNone(self.mod.read_last_message(cwd, "sess-404", question=False)) + + def test_live_payload_shape_finds_the_question(self): + # Live cmux payloads prefix the id (`claude-`) while the file is + # `.jsonl`, and the hook cwd drifts from the launch project dir. + uuid = "6746dc4f-9eca-4f38-9eb6-89a126ff3b53" + self._transcript("-Users-me-dev-architect-ffp", uuid, + [_assistant(_ask("t1", "Should I rebase or merge main?"))]) + bank = self.mod.PatternBank(self.home / ".assistant" / "pattern_bank.json") + res = self.mod.handle_event( + _evt("agent.hook.AskUserQuestion", cwd="/private/tmp/wt-261", + session_id=f"claude-{uuid}"), + bank, self.mod.WatcherState(cooldown_sec=0), FakeResolver(title="T"), + screen_reader=lambda ws: "Which option?") + item = json.loads(Path(res["path"]).read_text()) + self.assertEqual(item["last_message"], "Should I rebase or merge main?") + + def test_read_last_message_none_when_nothing_to_say(self): + cwd = "/Users/me/dev/quiet" + self._transcript(self.mod.claude_project_slug(cwd), "sess-q", + [_user({"type": "text", "text": "hello?"})]) + self.assertIsNone(self.mod.read_last_message(cwd, "sess-q", question=False)) + + def test_read_last_message_never_raises(self): + # No ~/.claude/projects at all: the scan fails, the drop must not. + self.assertIsNone(self.mod.read_last_message("/nowhere", "sess-x", question=False)) + self.assertFalse(self.projects.exists()) + + # ─── pattern hot-reload ──────────────────────────────────────────────────────── class TestPatternHotReload(unittest.TestCase): diff --git a/tests/test_comms_gaps.py b/tests/test_comms_gaps.py index 1d8ffd5..f93484b 100644 --- a/tests/test_comms_gaps.py +++ b/tests/test_comms_gaps.py @@ -118,7 +118,7 @@ def test_load_bedrock_env_no_zprofile(tmp_path: Path): def test_fmt_workspace_signal_pattern_match_label(): body = cl.fmt_workspace_signal({"ws_ref": "ws:2", "signal_type": "pattern_match", "pattern_matched": "BUILD FAILED"}) - assert "hit a watched signal" in body and "BUILD FAILED" in body + assert "showed something I watch for" in body and "BUILD FAILED" in body # ─── slack-send CLI error paths ───────────────────────────────────────────── @@ -160,40 +160,6 @@ def http(token, method, params): assert rc == 1 and "ratelimited" in err.getvalue() -# ─── comms-listen inbound reply flow (the core, previously untested) ───────── - -def test_reply_to_message_records_inbound_and_feeds_session(paths: cl.Paths, monkeypatch): - listen = _load("comms_listen", "comms-listen.py") - cs = sys.modules["comms_session"] - - # monkeypatch.setattr auto-restores after the test, so the real comms_session - # module is left pristine for test_comms_session.py (no cross-file leak). - fed = {} - monkeypatch.setattr(cs, "newest_transcript", lambda cwd, agent=None: "/tmp/fake.jsonl") - monkeypatch.setattr(cs, "transcript_line_count", lambda t: 0) - monkeypatch.setattr(cs, "should_clear", lambda t, agent=None: False) - monkeypatch.setattr(cs, "feed", lambda paths, surface, text: fed.setdefault("feed", text)) - monkeypatch.setattr(cs, "write_session", lambda *a, **k: None) - monkeypatch.setattr(cs, "read_session", lambda paths: None) - - calls = [] - monkeypatch.setattr(listen, "cli", lambda argv, timeout=30, env=None: (calls.append(argv) or (0, "[]", ""))) - monkeypatch.setattr(listen.time, "sleep", lambda s: None) - monkeypatch.setattr(listen, "REPLY_WAIT_SEC", 0) - - sess = {"ws_ref": "workspace:1", "surface_ref": "surface:1", - "cwd": "/tmp", "transcript_path": "/tmp/fake.jsonl"} - rec = {"channel": "C0", "text": "how's the fleet?", "msg_ts": "100.1", "reply_to": None} - listen.reply_to_message(paths, sess, rec) - - # inbound turn recorded via conversation.py append - assert any("conversation.py" in a[0] and "append" in a for a in calls), calls - # message fed to the warm session with a flat (no thread_root) slack header - assert "how's the fleet?" in fed["feed"] - assert "slack channel=C0" in fed["feed"] and "send_cli=" in fed["feed"] - assert "thread_root" not in fed["feed"] # 1:1 flat model - - def test_suppress_reason_matrix(): listen = _load("comms_listen", "comms-listen.py") supp = listen._suppress_reason @@ -204,6 +170,8 @@ def test_suppress_reason_matrix(): assert supp({"kind": "self-update", "key": "self-update-skip-p1"}) assert supp({"kind": "lesson-proposal", "key": "lesson-proposal:1"}) assert supp({"kind": "x", "key": "lesson-proposal-abc"}) + assert supp({"kind": "decision-transition", "key": "decision:d1:open->expired"}) + assert supp({"kind": "skipped", "key": "workspace:205-ready_for_merge", "outcome": "failed"}) # NOT suppressed — real actionable events, incl. self-update FAILURES. assert supp({"kind": "cleanup", "key": "assistant:close:ws:5", "outcome": "verified"}) is None assert supp({"kind": "self-update", "key": "self-update-fail-p8002", diff --git a/tests/test_comms_lib.py b/tests/test_comms_lib.py index 349a706..d8aaed5 100644 --- a/tests/test_comms_lib.py +++ b/tests/test_comms_lib.py @@ -6,6 +6,7 @@ import comms_lib as cl import pytest +from assistant import slack @pytest.fixture @@ -69,32 +70,181 @@ def test_bot_token_from_env(): # ─── formatting ───────────────────────────────────────────────────────────── def test_fmt_action_line_flags_screen_read(): - entry = {"kind": "cleanup", "key": "assistant:close:ws:5", "ws_ref": "ws:5", + entry = {"kind": "ready_for_merge", "key": "workspace:5-ready_for_merge", "ws_ref": "ws:5", "outcome": "verified", "verified_via": "screen_read", "pulse_idx": 3, - "evidence": "closed & "} - line = cl.fmt_action_line(entry) - assert "*[cleanup]* ok" in line - assert "(!)screen_read" in line - # mrkdwn escaping of the evidence's angle brackets + ampersand - assert "<clean>" in line and "&" in line + "td": "td-12", "evidence": "sent '/merge-when-ready' & "} + # A verified step's evidence is machine detail: it leads the footer. + assert cl.fmt_action_line(entry) == ( + "I asked a workspace to merge its PR.\n" + "Heads up: I only confirmed this by reading the screen, which isn't reliable proof.\n" + "_sent '/merge-when-ready' & <ok> · ws:5 · ready_for_merge · " + "workspace:5-ready_for_merge · td-12 · pulse 3_") -def test_fmt_action_line_maps_outcomes(): - assert "fail" in cl.fmt_action_line({"outcome": "failed"}) - assert "rej" in cl.fmt_action_line({"outcome": "rejected"}) +def test_fmt_action_line_observer_proof_has_no_warning(): + line = cl.fmt_action_line({"kind": "goal-edit", "outcome": "verified", + "verified_via": "observer"}) + assert line == "I updated your goals.\n_goal-edit_" + +def test_fmt_action_line_failure_quotes_the_evidence(): + entry = {"kind": "self-update", "key": "self-update-fail-p3326", "ws_ref": "(launchd)", + "outcome": "failed", + "evidence": "fetch failed:\n\n fatal: couldn't find remote ref " + "x" * 300} + lines = cl.fmt_action_line(entry).splitlines() + assert lines[0] == "I tried to update Assistant to the latest code, but it didn't work." + assert lines[1] == "> fetch failed:" + assert lines[2].startswith("> fatal: couldn't find remote ref x") and lines[2].endswith("x…") + assert lines[3] == "_(launchd) · self-update · self-update-fail-p3326_" -def test_fmt_heartbeat_alert(): - body = cl.fmt_heartbeat_alert({"ws_ref": "ws:1", "status": "frozen", + +def test_fmt_action_line_maps_outcomes(): + upd = {"kind": "self-update"} + assert cl.fmt_action_line({**upd, "outcome": "verified"}).startswith( + "I updated Assistant to the latest code.") + assert cl.fmt_action_line({**upd, "outcome": "rejected", "evidence": "no"}) == ( + "I tried to update Assistant to the latest code, but it was turned down.\n" + "> no\n_self-update_") + assert cl.fmt_action_line({**upd, "outcome": "skipped"}).startswith( + "I didn't update Assistant to the latest code this time.") + assert cl.fmt_action_line({**upd, "outcome": "odd"}).startswith( + "I tried to update Assistant to the latest code (result: odd<x>).") + + +def test_fmt_action_line_names_strategist_research_plainly(): + line = cl.fmt_action_line({ + "kind": "strategist-context-wrote", "key": "strategist:context-wrote:dec-ca7d", + "ws_ref": "(strategist)", "outcome": "verified", + "evidence": "pre-researched decision dec-ca7d context (draft-only, surfaced in brief)"}) + assert line.splitlines()[0] == ( + "I added background to your brief for a decision that's waiting on you.") + assert "dec-ca7d" not in line.splitlines()[0], "the decision id is a ref: footer only" + + +def test_fmt_action_line_unknown_kind_and_bare_entry(): + assert cl.fmt_action_line({"kind": "brand-new-kind", "outcome": "verified"}) == ( + "I took an automatic step.\n_brand-new-kind_") + # No evidence, key, ws, td, or pulse → no quote line and no empty refs. + assert cl.fmt_action_line({}) == "I tried to take an automatic step (result: ?)." + + +def test_fmt_heartbeat_alert_stopped(): + body = cl.fmt_heartbeat_alert({"ws_ref": "(launchd)", "status": "running", + "last_pulse_iso": "2026-09-28T10:02:00Z"}, 1500) + assert body == ("*Assistant's main loop has stopped* — no run for 25m " + "(last run 2026-09-28T10:02:00Z). I'll post again when it's back.") + + +@pytest.mark.parametrize("status,words", [ + ("frozen", "it reports that it's frozen"), + ("stale_world", "it's working from an out-of-date view of your workspaces"), + ("respawn-requested", "it asked to be restarted"), +]) +def test_fmt_heartbeat_alert_bad_status_in_words(status, words): + body = cl.fmt_heartbeat_alert({"status": status, "last_pulse_iso": "2026-07-05T00:00:00Z"}, 720) - assert "heartbeat stale" in body and "status=frozen" in body and "12m ago" in body + assert body == (f"*Assistant's main loop needs a look* — {words}. Last run 12m ago " + f"(2026-07-05T00:00:00Z). I'll post again when it's back.") + + +def test_fmt_heartbeat_alert_missing_last_run(): + assert "(last run unknown)" in cl.fmt_heartbeat_alert({}, 60) + + +def test_fmt_heartbeat_recovered(): + assert cl.fmt_heartbeat_recovered({"last_pulse_iso": "2026-09-28T13:05:00Z"}, 3 * 3600) == ( + "*Assistant's main loop is running again* after 3h0m (latest run 2026-09-28T13:05:00Z).") + assert cl.fmt_heartbeat_recovered({}, 1500) == ( + "*Assistant's main loop is running again* after 25m.") def test_fmt_workspace_signal_handles_both_key_names(): # cmux-watcher writes "signal"/"signal_type" — accept either. body = cl.fmt_workspace_signal({"ws_ref": "ws:2", "signal": "needs_input", "screen_snippet": "waiting for input"}) - assert "needs your input" in body and "waiting for input" in body + assert body == "A workspace needs your input.\n> waiting for input\n_ws:2 · needs_input_" + + +def test_fmt_workspace_signal_question_leads_with_title_and_question(): + body = cl.fmt_workspace_signal({ + "ws_ref": "workspace:244", "signal_type": "needs_input", + "pattern_matched": "AskUserQuestion", "ws_title": "Fix archself deferral door", + "last_message": "Should I rebase or merge main?", "screen_snippet": "1. Rebase"}) + assert body == ("*Fix archself deferral door* is asking you: Should I rebase or merge main?\n" + "_workspace:244 · AskUserQuestion_") + + +def test_fmt_workspace_signal_question_without_text_falls_back_to_snippet(): + body = cl.fmt_workspace_signal({ + "ws_ref": "workspace:3", "signal_type": "needs_input", + "pattern_matched": "AskUserQuestion", "screen_snippet": "1. Rebase\n2. Merge"}) + assert body == ("A workspace has a question for you.\n> 1. Rebase\n> 2. Merge\n" + "_workspace:3 · AskUserQuestion_") + + +def test_fmt_workspace_signal_prefers_last_message_over_snippet(): + body = cl.fmt_workspace_signal({ + "ws_ref": "workspace:244", "signal_type": "needs_input", + "pattern_matched": "Notification", "ws_title": "Green E2E Suite", + "last_message": "Can I run the full suite? It takes 40 min.", + "screen_snippet": "✽ Boogieing…"}) + assert body == ("*Green E2E Suite* needs your input.\n" + "> Can I run the full suite? It takes 40 min.\n" + "_workspace:244 · Notification_") + + +def test_fmt_workspace_signal_headlines_by_signal(): + def first_line(signal_type): + return cl.fmt_workspace_signal({"ws_title": "T", "signal_type": signal_type}).split("\n")[0] + assert first_line("work_complete") == "*T* looks done." + assert first_line("pattern_match") == "*T* showed something I watch for." + assert first_line("mystery") == "*T* sent an update." + + +def test_fmt_workspace_signal_escapes_and_caps_dynamic_text(): + body = cl.fmt_workspace_signal({ + "ws_title": "a", "signal_type": "work_complete", "last_message": "x&" + "y" * 600}) + assert body.startswith("*a<b>* looks done.\n> x&") + assert body.count("y") == 398 and body.endswith("y…") + asked = cl.fmt_workspace_signal({"pattern_matched": "AskUserQuestion", + "last_message": "" + "z" * 600}) + assert asked.startswith("A workspace is asking you: <q>") and asked.count("z") == 397 + + +def test_fmt_workspace_signal_title_star_cannot_break_bold(): + body = cl.fmt_workspace_signal({"ws_title": "Fix *all* flakes", "signal_type": "work_complete"}) + assert body == "*Fix all flakes* looks done." + + +def test_fmt_workspace_signal_bare_item_has_no_footer(): + assert cl.fmt_workspace_signal({}) == "A workspace sent an update." + + +_PARITY_ENTRIES = [ + {"kind": "ready_for_merge", "key": "k", "ws_ref": "ws:5", "outcome": "verified", + "verified_via": "screen_read", "pulse_idx": 3, "td": "td-1", "evidence": "a & "}, + {"kind": "strategist-context", "key": "strategist:context:d", "outcome": "verified"}, + {"kind": "self-update", "outcome": "failed", "evidence": "fetch failed\n\n" + "x" * 300}, + {"kind": "goal-edit", "outcome": "rejected"}, + {"kind": "policy-bootstrap-upgrade", "outcome": "skipped"}, + {"kind": "new-kind", "outcome": "weird"}, + {}, +] +_PARITY_HEARTBEATS = [ + ({"status": "running", "last_pulse_iso": "2026-09-28T10:02:00Z"}, 1500), + ({"status": "frozen"}, 60), + ({"status": "respawn-requested", "last_pulse_iso": "x= len(script): + stop.set() + return True + paths.heartbeat.write_text(json.dumps({"last_pulse_ts": script[ticks["n"]]})) + return False + + monkeypatch.setattr(stop, "wait", fake_wait) + listen.heartbeat_loop(stop, {}) + sent = _sends(calls) + assert sent[0].startswith("PAGE") and len(sent) == 2 + assert sent[1] == f"BACK {script[-1] - stale}" + kinds = [a[a.index("--kind") + 1] for a in calls if "slack-send.py" in a[0]] + assert kinds == ["urgent", "action"] + + +def test_heartbeat_loop_bad_status_pages_and_missing_config_is_quiet(env, monkeypatch): + paths, calls = env + paths.heartbeat.write_text(json.dumps({"last_pulse_ts": int(time.time()), "status": "frozen"})) + monkeypatch.setattr(cl, "fmt_heartbeat_alert", lambda hb, age: "PAGE") + stop = threading.Event() + ticks = {"n": 0} + + def fake_wait(timeout=None): + ticks["n"] += 1 + if ticks["n"] == 2: + paths.config.unlink() + paths.heartbeat.write_text("{broken") + if ticks["n"] >= 4: + stop.set() + return True + return False + + monkeypatch.setattr(stop, "wait", fake_wait) + listen.heartbeat_loop(stop, {}) + assert _sends(calls) == ["PAGE"] + + +# ─── ledger ───────────────────────────────────────────────────────────────── + + +def test_housekeeping_kinds_are_suppressed(): + for kind in ("decision-transition", "strategist-autopause", "stranded", "skipped"): + assert listen._suppress_reason({"kind": kind, "key": "k", "outcome": "failed"}) + assert listen._suppress_reason({"kind": "cleanup", "key": "k", "outcome": "verified"}) is None + + +def test_subsystem_mirrors_housekeeping_kinds(): + assert subsystem_comms.HOUSEKEEPING_KINDS == listen.HOUSEKEEPING_KINDS + + +def test_plan_broadcast_caps_the_pass(): + entries = ([{"kind": "decision-transition", "key": f"d{i}"} for i in range(3)] + + [{"kind": "cleanup", "key": f"c{i}"} for i in range(8)]) + to_send, suppressed, overflow = listen.plan_broadcast(entries, max_send=5) + assert [e["key"] for e in to_send] == ["c0", "c1", "c2", "c3", "c4"] + assert len(suppressed) == 3 and overflow == 3 + assert listen.plan_broadcast([], max_send=5) == ([], [], 0) + + +def test_fmt_overflow_plural(): + assert listen.fmt_overflow(1).startswith("…and 1 more Assistant update.") + assert listen.fmt_overflow(4).startswith("…and 4 more Assistant updates.") + + +def _ledger(paths, entries): + paths.ledger.write_text("".join(json.dumps(e) + "\n" for e in entries)) + paths.cursor.write_text("0") + + +def _one_pass(monkeypatch): + stop = threading.Event() + monkeypatch.setattr(stop, "wait", lambda timeout=None: stop.set() or True) + return stop + + +def test_ledger_loop_posts_the_cap_then_one_summary(env, monkeypatch): + paths, calls = env + _ledger(paths, [{"kind": "cleanup", "key": f"c{i}", "outcome": "verified"} for i in range(7)] + + [{"kind": "decision-transition", "key": "d1"}]) + monkeypatch.setattr(listen, "LEDGER_MAX_PER_PASS", 5) + monkeypatch.setattr(cl, "fmt_action_line", lambda e: f"ACTION {e['key']}") + listen.ledger_loop(_one_pass(monkeypatch), {}) + sent = _sends(calls) + assert sent[:5] == [f"ACTION c{i}" for i in range(5)] + assert sent[5] == listen.fmt_overflow(2) and len(sent) == 6 + mirrored = [a for a in calls if "conversation.py" in a[0]] + assert len(mirrored) == 6, "each post, summary included, is mirrored as an out turn" + assert "suppressed broadcast key=d1" in _log(paths) + + +def test_ledger_loop_without_target_sends_nothing(env, monkeypatch): + paths, calls = env + paths.config.write_text(json.dumps({"slack": {"target": "", "allowed_targets": []}})) + _ledger(paths, [{"kind": "cleanup", "key": "c1", "outcome": "verified"}]) + listen.ledger_loop(_one_pass(monkeypatch), {}) + assert _sends(calls) == [] + assert "no target configured — skipping 1 broadcast(s)" in _log(paths) + + +def test_ledger_loop_logs_failed_sends(env, monkeypatch): + paths, _ = env + _ledger(paths, [{"kind": "cleanup", "key": f"c{i}", "outcome": "verified"} for i in range(2)]) + monkeypatch.setattr(listen, "LEDGER_MAX_PER_PASS", 1) + monkeypatch.setattr(listen, "cli", lambda argv, timeout=30, env=None: (1, "", "slack down")) + listen.ledger_loop(_one_pass(monkeypatch), {}) + log = _log(paths) + assert "ledger broadcast rc=1 key=c0 err=slack down" in log + assert "broadcast overflow summary for 1 update(s) rc=1" in log + + +def test_mirror_sent_skips_muted_and_unparseable_lines(env): + paths, calls = env + listen._mirror_sent("not json\n" + json.dumps({"muted": True}) + "\n" + + json.dumps({"message_id": "1", "channel": None}), "body") + assert calls == [] + + +# ─── inbox cooldown ───────────────────────────────────────────────────────── + + +def test_inbox_should_ping(): + assert listen.inbox_should_ping({"pattern_matched": "Notification"}, None, 100.0, 900) + assert not listen.inbox_should_ping({"pattern_matched": "Notification"}, 50.0, 100.0, 900) + assert listen.inbox_should_ping({"pattern_matched": "stranded"}, 50.0, 1000.0, 900) + assert listen.inbox_should_ping({"pattern_matched": "AskUserQuestion"}, 99.0, 100.0, 900) + + +def _signal(inbox: Path, name: str, **fields): + item = {"ts": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()), "event": "workspace_signal", + "signal_type": "needs_input", "screen_snippet": "", **fields} + (inbox / f"cmux-{name}.json").write_text(json.dumps(item)) + + +def test_inbox_pings_a_workspace_once_per_window_except_questions(env, monkeypatch): + """workspace:244 got 21 pings in six hours. Now the second and third signals + in the window are held back, but a real question still goes through, and + the window survives a daemon restart (it's on disk).""" + paths, calls = env + inbox = listen.INBOX_DIR + monkeypatch.setattr(cl, "fmt_workspace_signal", lambda item: f"PING {item['pattern_matched']}") + _signal(inbox, "a1", ws_ref="workspace:244", pattern_matched="Notification") + _signal(inbox, "a2", ws_ref="workspace:244", pattern_matched="stranded") + _signal(inbox, "b1", ws_ref="workspace:7", pattern_matched="Notification") + assert listen._drain_inbox_once({}) == 2 + _signal(inbox, "a3", ws_ref="workspace:244", pattern_matched="Notification") + _signal(inbox, "a4", ws_ref="workspace:244", pattern_matched="AskUserQuestion") + assert listen._drain_inbox_once({}) == 1 + assert _sends(calls) == ["PING Notification", "PING Notification", "PING AskUserQuestion"] + assert list(inbox.glob("cmux-*.json")) == [], "held-back signals are removed, not retried" + assert "held back 1 signal(s)" in _log(paths) + + +def test_cooldown_file_prunes_old_entries_and_tolerates_corruption(env): + paths, _ = env + listen._write_cooldown(paths, {"workspace:1": 0.0, "workspace:2": 1000.0}, now=1100.0) + assert listen._read_cooldown(paths) == {"workspace:2": 1000.0} + (paths.comms_dir / "inbox-cooldown.json").write_text("[1, 2]") + assert listen._read_cooldown(paths) == {} + (paths.comms_dir / "inbox-cooldown.json").write_text("{bad") + assert listen._read_cooldown(paths) == {} + (paths.comms_dir / "inbox-cooldown.json").write_text(json.dumps({"a": "x", "b": 5})) + assert listen._read_cooldown(paths) == {"b": 5} diff --git a/tests/test_comms_listen_inbox.py b/tests/test_comms_listen_inbox.py index d4ac441..aacd3f3 100644 --- a/tests/test_comms_listen_inbox.py +++ b/tests/test_comms_listen_inbox.py @@ -248,33 +248,34 @@ def test_drain_proposals_backlog_skipped_on_first_run(env_proposals): # ─── reply_to_message threads the session provider (G3 integration) ────────── # # A persisted droid session must thread agent="droid" into every transcript-root -# / context call — newest_transcript, should_clear, and the clear_session -# delegation — else it reads the wrong root and never clears. Zero real -# cmux/network: every comms_session touchpoint + cli() is monkeypatched. +# / context call — the project folder searched for the submission, should_clear, +# and the clear_session delegation — else it reads the wrong root and never +# clears. Zero real cmux/network: every comms_session touchpoint is +# monkeypatched. @pytest.fixture def env_reply(monkeypatch): - """Stub every cmux/network touchpoint reply_to_message can reach; record the - agent threaded into each provider-aware call. transcript grows on the first - poll so the reply loop breaks without real sleeps.""" - monkeypatch.setattr(listen.time, "sleep", lambda *a, **k: None) - monkeypatch.setattr(listen, "cli", lambda *a, **k: (0, "", "")) - - rec = {} - monkeypatch.setattr(listen.comms_session, "feed", lambda *a, **k: None) - - def fake_newest(cwd, agent="claude"): - rec["newest_agent"] = agent - return "/warm-t.jsonl" - monkeypatch.setattr(listen.comms_session, "newest_transcript", fake_newest) - - counts = iter([0, 5, 5, 5]) - monkeypatch.setattr(listen.comms_session, "transcript_line_count", - lambda t: next(counts, 5)) + """Stub every cmux touchpoint reply_to_message can reach; record the agent + threaded into each provider-aware call and what was typed.""" + rec = {"typed": [], "found": "/warm-t.jsonl", "submitted": True} + + def fake_submit(paths, surface_ref, text, marker, confirmed): + rec["typed"].append((surface_ref, text, marker)) + return rec["submitted"] and confirmed() + monkeypatch.setattr(listen.comms_session, "submit", fake_submit) + + def fake_project_dir(cwd, agent="claude"): + rec["project_dir_agent"] = agent + return Path("/proj") + monkeypatch.setattr(listen.comms_session, "project_dir_for_cwd", fake_project_dir) + monkeypatch.setattr(listen.comms_session, "transcript_has_submission", + lambda path, marker: path == rec.get("bound_hit")) + monkeypatch.setattr(listen.comms_session, "find_submission", + lambda d, marker, since: rec["found"]) def fake_should_clear(transcript, agent="claude"): - rec["should_clear_agent"] = agent + rec["should_clear"] = (transcript, agent) return rec.get("_clear", True) monkeypatch.setattr(listen.comms_session, "should_clear", fake_should_clear) @@ -286,43 +287,85 @@ def fake_clear(paths, sess, boot_prompt, agent="claude", log=None): return refreshed monkeypatch.setattr(listen.comms_session, "clear_session", fake_clear) - def fake_write(*a, **k): - rec["wrote"] = True + def fake_write(paths, ws, surface, cwd, transcript, **k): + rec["wrote"] = transcript monkeypatch.setattr(listen.comms_session, "write_session", fake_write) monkeypatch.setattr(listen.comms_session, "read_session", - lambda paths: {"agent": "droid"}) + lambda paths: {"agent": "droid", "transcript_path": rec.get("wrote")}) return rec, refreshed -def _droid_sess(): +def _droid_sess(transcript=None): return {"ws_ref": "workspace:5", "surface_ref": "surface:3", "cwd": "/cwd", - "transcript_path": None, "agent": "droid"} + "transcript_path": transcript, "agent": "droid"} + + +INBOUND = [{"channel": "C0", "text": "hi", "msg_ts": "1.1", "reply_to": None}] + + +def test_reply_threads_droid_agent_into_context_calls(env_reply, env_inbox): + rec, _ = env_reply + listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), INBOUND) + assert rec["project_dir_agent"] == "droid" + assert rec["should_clear"] == ("/warm-t.jsonl", "droid") -def test_reply_threads_droid_agent_into_context_calls(env_reply): +def test_reply_types_the_header_and_waits_for_its_marker(env_reply, env_inbox): rec, _ = env_reply - inbound = {"channel": "C0", "text": "hi", "msg_ts": "1.1", "reply_to": None} - listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), inbound) - assert rec["newest_agent"] == "droid" - assert rec["should_clear_agent"] == "droid" + listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), INBOUND) + [(surface, text, marker)] = rec["typed"] + assert surface == "surface:3" + assert text == f"[slack channel=C0 msg_ts=1.1 send_cli={listen.SLACK_SEND}] hi" + assert marker == "msg_ts=1.1" -def test_reply_delegates_to_clear_session_and_returns_refreshed(env_reply): +def test_reply_delegates_to_clear_session_and_returns_refreshed(env_reply, env_inbox): rec, refreshed = env_reply - inbound = {"channel": "C0", "text": "hi", "msg_ts": "1.1", "reply_to": None} - out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), inbound) + out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), INBOUND) assert rec["clear_agent"] == "droid" - assert out == refreshed + assert out == (True, refreshed) -def test_reply_no_clear_writes_session_and_skips_clear(env_reply): +def test_reply_rebinds_to_the_transcript_that_recorded_the_message(env_reply, env_inbox): + """The recorded transcript was another session's (2026-09-28: ws:258 was + bound to ws:256's file). The message lands elsewhere, and the session is + rebound to where it landed.""" rec, _ = env_reply rec["_clear"] = False - inbound = {"channel": "C0", "text": "hi", "msg_ts": "1.1", "reply_to": None} - out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), inbound) + delivered, out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), + _droid_sess("/wrong.jsonl"), INBOUND) + assert delivered is True assert "clear_agent" not in rec, "should_clear False must not delegate to clear_session" - assert rec.get("wrote") is True - assert out == {"agent": "droid"} + assert rec["wrote"] == "/warm-t.jsonl" + assert out["transcript_path"] == "/warm-t.jsonl" + + +def test_reply_keeps_the_bound_transcript_when_the_message_lands_there(env_reply, env_inbox): + rec, _ = env_reply + rec["_clear"] = False + rec["bound_hit"] = "/bound.jsonl" + rec["found"] = "/somewhere-else.jsonl" + sess = _droid_sess("/bound.jsonl") + delivered, out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), sess, INBOUND) + assert delivered is True and out is sess + assert "wrote" not in rec, "no rebind when the bound transcript recorded it" + + +def test_reply_not_delivered_returns_false_and_leaves_the_session(env_reply, env_inbox): + rec, _ = env_reply + rec["submitted"] = False + sess = _droid_sess() + assert listen.reply_to_message(listen.comms_lib.Paths.from_env(), sess, INBOUND) == (False, sess) + assert "should_clear" not in rec and "wrote" not in rec + + +def test_feed_text_combines_messages_that_piled_up(): + recs = [{"text": "Are you alive?", "msg_ts": "1.0"}, + {"text": "Pulse active now?", "msg_ts": "2.0"}] + text = listen.feed_text(recs, "C0") + assert text.startswith(f"[slack channel=C0 msg_ts=2.0 send_cli={listen.SLACK_SEND}] ") + assert "2 messages arrived while your session was down" in text + assert "(1) Are you alive? (2) Pulse active now?" in text # ─── preflight doctor loader (regression: bare `import assistant_doctor` could @@ -350,14 +393,16 @@ def test_load_doctor_runs_slack_checks(): # ─── ensure_warm_session: respawn when the live session's model went stale ────── -def _stub_warm(monkeypatch, *, alive: bool, model_current: bool): +def _stub_warm(monkeypatch, *, alive: bool | None, model_current: bool): """Stub the comms_session machinery ensure_warm_session drives; return a dict - recording which lifecycle calls fired.""" + recording which lifecycle calls fired. alive=None means cmux didn't answer.""" rec = {"closed": [], "cleared": False, "spawned": False} sess = {"ws_ref": "workspace:18", "agent": "claude", "model": "us.anthropic.claude-sonnet-4-6[1m]"} + state = {True: listen.comms_session.ALIVE, False: listen.comms_session.GONE, + None: listen.comms_session.UNKNOWN}[alive] monkeypatch.setattr(listen.comms_session, "read_session", lambda p: sess) - monkeypatch.setattr(listen.comms_session, "cmux_alive", lambda p, ref: alive) + monkeypatch.setattr(listen.comms_session, "workspace_state", lambda p, ref: state) monkeypatch.setattr(listen.comms_session, "warm_session_model_is_current", lambda p, s: model_current) monkeypatch.setattr(listen.comms_session, "close_own_workspace", @@ -412,3 +457,46 @@ def test_ensure_warm_session_respawns_when_gone(env_inbox, monkeypatch): rec, _ = _stub_warm(monkeypatch, alive=False, model_current=True) listen.ensure_warm_session(cl.Paths.from_env()) assert rec["closed"] == ["workspace:18"] and rec["spawned"] is True + + +def test_warm_session_left_alone_when_cmux_doesnt_answer(env_inbox, monkeypatch): + """2026-09-27: a napping cmux refused connections, and each refused check + closed a healthy warm session and spawned another onto the stalled cmux. A + check cmux doesn't answer must keep the session and spawn nothing, even on + the inbound path with a stale model. Mutation probe: treat UNKNOWN like + GONE and the session is closed and respawned.""" + listen._cmux_silent_since = None + rec, sess = _stub_warm(monkeypatch, alive=None, model_current=False) + out, how = listen._warm_session(cl.Paths.from_env(), respawn_on_stale=True) + assert out is sess and how == listen.SESSION_UNREACHABLE + assert rec["closed"] == [] and rec["spawned"] is False and rec["cleared"] is False + log = (cl.Paths.from_env().comms_dir / "comms-listen.log").read_text() + assert log.count("cmux isn't answering") == 1 + listen._warm_session(cl.Paths.from_env()) + log = (cl.Paths.from_env().comms_dir / "comms-listen.log").read_text() + assert log.count("cmux isn't answering") == 1, "one log line per silent episode" + + +def test_warm_session_logs_when_cmux_answers_again(env_inbox, monkeypatch): + listen._cmux_silent_since = listen.time.time() - 42 + rec, sess = _stub_warm(monkeypatch, alive=True, model_current=True) + out, how = listen._warm_session(cl.Paths.from_env()) + assert out is sess and how == listen.SESSION_ALIVE + assert listen._cmux_silent_since is None + assert "cmux answering again after" in ( + cl.Paths.from_env().comms_dir / "comms-listen.log").read_text() + + +def test_warm_session_force_respawn_replaces_a_live_session(env_inbox, monkeypatch): + """A live session that didn't accept a typed message is replaced, so the + queued message lands in a fresh one.""" + rec, _ = _stub_warm(monkeypatch, alive=True, model_current=True) + out, how = listen._warm_session(cl.Paths.from_env(), force_respawn=True) + assert rec["closed"] == ["workspace:18"] and rec["spawned"] is True + assert how == listen.SESSION_SPAWNED and out["ws_ref"] == "workspace:19" + + +def test_warm_session_reports_a_failed_spawn(env_inbox, monkeypatch): + rec, _ = _stub_warm(monkeypatch, alive=False, model_current=True) + monkeypatch.setattr(listen.comms_session, "spawn_session", lambda p, prompt, log=None: None) + assert listen._warm_session(cl.Paths.from_env()) == (None, listen.SESSION_NONE) diff --git a/tests/test_comms_listen_watchdog.py b/tests/test_comms_listen_watchdog.py index 5d820e0..ffb79b0 100644 --- a/tests/test_comms_listen_watchdog.py +++ b/tests/test_comms_listen_watchdog.py @@ -55,38 +55,52 @@ def env(tmp_path: Path, monkeypatch): # ─── watchdog_tick: the per-pass contract ─────────────────────────────────── -def test_watchdog_tick_calls_ensure_warm_session(env, monkeypatch): - """watchdog_tick delegates to ensure_warm_session exactly once with the - paths it was given. Mutation probe: if the watchdog stopped calling - ensure_warm_session (e.g. the body was stubbed out), calls would be empty.""" +def test_watchdog_tick_calls_warm_session(env, monkeypatch): + """watchdog_tick delegates to _warm_session exactly once with the paths it + was given. Mutation probe: if the watchdog stopped calling _warm_session + (e.g. the body was stubbed out), calls would be empty.""" calls = [] - def fake_ensure(paths): + def fake_warm(paths): calls.append(paths) - return {"ws_ref": "workspace:1"} + return {"ws_ref": "workspace:1"}, listen.SESSION_ALIVE - monkeypatch.setattr(listen, "ensure_warm_session", fake_ensure) + monkeypatch.setattr(listen, "_warm_session", fake_warm) status = listen.watchdog_tick(env) assert calls == [env] assert status == "alive" +def test_watchdog_tick_spawned_counts_as_alive(env, monkeypatch): + monkeypatch.setattr(listen, "_warm_session", + lambda paths: ({"ws_ref": "workspace:2"}, listen.SESSION_SPAWNED)) + assert listen.watchdog_tick(env) == "alive" + + def test_watchdog_tick_no_session(env, monkeypatch): - """ensure_warm_session returning None (cmux down, spawn failed) must surface - as 'no-session', NOT 'alive'. Mutation probe: if watchdog_tick always + """_warm_session returning no session (spawn failed) must surface as + 'no-session', NOT 'alive'. Mutation probe: if watchdog_tick always returned 'alive' (e.g. `return 'alive'` unconditionally), this fails.""" - monkeypatch.setattr(listen, "ensure_warm_session", lambda paths: None) + monkeypatch.setattr(listen, "_warm_session", lambda paths: (None, listen.SESSION_NONE)) assert listen.watchdog_tick(env) == "no-session" +def test_watchdog_tick_reports_unresponsive_cmux_distinctly(env, monkeypatch): + """When cmux doesn't answer, the session is kept, and the tick says so + instead of claiming it's alive or missing.""" + monkeypatch.setattr(listen, "_warm_session", + lambda paths: ({"ws_ref": "workspace:3"}, listen.SESSION_UNREACHABLE)) + assert listen.watchdog_tick(env) == "cmux-unresponsive" + + def test_watchdog_tick_survives_exception(env, monkeypatch): - """A transient cmux error inside ensure_warm_session must NOT propagate — the + """A transient cmux error inside _warm_session must NOT propagate — the watchdog thread would die and never retry. Mutation probe: removing the try/except (letting the exception raise) makes this test raise instead of returning an 'error:...' status.""" def boom(paths): raise RuntimeError("cmux RPC timed out") - monkeypatch.setattr(listen, "ensure_warm_session", boom) + monkeypatch.setattr(listen, "_warm_session", boom) status = listen.watchdog_tick(env) assert status.startswith("error:") assert "RuntimeError" in status @@ -97,7 +111,7 @@ def test_watchdog_tick_error_status_names_exception_type(env, monkeypatch): actionable (a ValueError vs a TimeoutError mean different things). Mutation probe: if the handler returned a generic 'error' without the type name, 'OSError' would be absent.""" - monkeypatch.setattr(listen, "ensure_warm_session", + monkeypatch.setattr(listen, "_warm_session", lambda paths: (_ for _ in ()).throw(OSError("nope"))) assert "OSError" in listen.watchdog_tick(env) @@ -121,9 +135,9 @@ def test_watchdog_loop_self_heals_across_ticks(env, monkeypatch): stop.is_set()`), the second call would never happen and calls would be 1.""" monkeypatch.setattr(listen.time, "sleep", lambda *a, **k: None) - seq = iter([None, {"ws_ref": "workspace:9"}]) + seq = iter([(None, listen.SESSION_NONE), ({"ws_ref": "workspace:9"}, listen.SESSION_SPAWNED)]) calls = [] - monkeypatch.setattr(listen, "ensure_warm_session", + monkeypatch.setattr(listen, "_warm_session", lambda paths: (calls.append(1), next(seq))[1]) # Stop the loop after two `stop.wait` returns by pre-setting the event the # second time. We do that by making stop.wait set the event after the 2nd @@ -177,8 +191,9 @@ def test_watchdog_loop_backs_off_on_repeated_failure(env, monkeypatch): waited WATCHDOG_INTERVAL_SEC, the captured waits would be all 60s.""" monkeypatch.setattr(listen.time, "sleep", lambda *a, **k: None) # fail, fail, fail, then alive. - seq = iter([None, None, None, {"ws_ref": "workspace:9"}]) - monkeypatch.setattr(listen, "ensure_warm_session", lambda paths: next(seq)) + none = (None, listen.SESSION_NONE) + seq = iter([none, none, none, ({"ws_ref": "workspace:9"}, listen.SESSION_ALIVE)]) + monkeypatch.setattr(listen, "_warm_session", lambda paths: next(seq)) waits: list[float] = [] stop = threading.Event() diff --git a/tests/test_comms_session.py b/tests/test_comms_session.py index 5924b0e..b30da82 100644 --- a/tests/test_comms_session.py +++ b/tests/test_comms_session.py @@ -152,13 +152,6 @@ def test_last_assistant_text_none_when_no_assistant(tmp_path: Path): assert cs.last_assistant_text(t) is None -def test_transcript_line_count(tmp_path: Path): - t = tmp_path / "t.jsonl" - t.write_text("a\n\nb\n \nc\n") - assert cs.transcript_line_count(t) == 3 - assert cs.transcript_line_count(tmp_path / "missing.jsonl") == 0 - - def test_should_clear_uses_threshold(tmp_path: Path): t = tmp_path / "t.jsonl" t.write_text(json.dumps({"message": {"usage": {"input_tokens": 600_000}}}) + "\n") @@ -178,10 +171,6 @@ def test_project_dir_for_cwd_slug(): assert d.name == cwd.replace("/", "-") -def test_newest_transcript_none_for_missing(tmp_path: Path): - assert cs.newest_transcript(str(tmp_path / "nowhere")) is None - - # ─── droid schema parity (G2 read) ────────────────────────────────────────── def test_last_assistant_text_droid_schema(tmp_path: Path): @@ -332,6 +321,10 @@ def test_should_clear_claude_default_agent_matches_usage_path(tmp_path: Path): # tested here by stubbing every cmux-touching helper with fakes that record. +RULE = "─" * 40 +WELCOME_SCREEN = f"Welcome back!\n{RULE}\n❯ \n{RULE}\n ⏵⏵ bypass permissions on" + + def _stub_cmux(monkeypatch): """Neutralize every cmux/sleep touchpoint clear_session can reach and return recorders. No real cmux RPC, no wall-clock sleeps.""" @@ -339,11 +332,15 @@ def _stub_cmux(monkeypatch): rpc_calls: list = [] monkeypatch.setattr(cs, "_cmux_rpc", lambda p, method, params, timeout=15: rpc_calls.append((method, params))) - monkeypatch.setattr(cs, "_surface_read_text", lambda *a, **k: "Welcome back") + monkeypatch.setattr(cs, "_surface_read_text", lambda *a, **k: WELCOME_SCREEN) feeds: list = [] - monkeypatch.setattr(cs, "feed", lambda p, s, text: feeds.append(text)) - monkeypatch.setattr(cs, "newest_transcript", lambda cwd, agent: "/new-t.jsonl") - calls: dict = {} + + def fake_boot(p, surface_ref, cwd, boot_prompt, agent): + feeds.append((surface_ref, boot_prompt)) + return feeds_result["transcript"] + feeds_result = {"transcript": "/new-t.jsonl"} + monkeypatch.setattr(cs, "deliver_boot", fake_boot) + calls: dict = {"boot_result": feeds_result} def fake_close(p, ws, log=lambda m: None): calls["close"] = ws @@ -380,11 +377,30 @@ def test_clear_session_claude_clears_in_place_and_returns_refreshed(paths: cl.Pa assert any(m == "surface.send_text" and params.get("text") == "/clear" for m, params in rpc_calls), "claude branch must send /clear" assert "spawn_agent" not in calls, "claude branch must not spawn a new session" + assert feeds == [("surface:3", Path("/boot.md"))], "the boot prompt is re-delivered once" assert out["transcript_path"] == "/new-t.jsonl" assert out["ws_ref"] == "workspace:5" assert out["agent"] == ag.CLAUDE +def test_clear_session_claude_respawns_when_boot_never_lands(paths: cl.Paths, monkeypatch): + """If the boot prompt after /clear is never submitted, the session would sit + with no instructions; clear_session falls back to a lossless respawn instead + of recording a transcript that never received the prompt. + + Mutation probe: drop the `if not transcript` fallback and the stale session + is returned with transcript_path None — the close/spawn asserts fail.""" + rpc_calls, feeds, calls, respawned = _stub_cmux(monkeypatch) + calls["boot_result"]["transcript"] = None + cs.write_session(paths, "workspace:5", "surface:3", "/cwd", "/old.jsonl") + sess = cs.read_session(paths) + out = cs.clear_session(paths, sess, Path("/boot.md"), agent=ag.CLAUDE) + assert calls["close"] == "workspace:5" + assert calls["spawn_agent"] == ag.CLAUDE + assert out == respawned + assert cs.read_session(paths) is None, "the dead session's registry entry is cleared" + + # ─── instance-scoped warm-workspace reconcile (reconcile bug fix) ───────────── def test_spawned_ledger_roundtrip(paths: cl.Paths): @@ -593,6 +609,59 @@ def _no_scan(p): assert closed == ["workspace:300"], "only the just-created workspace is closed" +def _spawn_env(tmp_path, monkeypatch, ws="workspace:310", surface="surface:310"): + """A spawn whose cmux calls all succeed and whose boot screen is ready.""" + home = tmp_path / "home" + (home / ".assistant").mkdir(parents=True) + paths = cl.Paths.from_env({"HOME": str(home), "COMMS_HOME": str(home)}) + + def fake_run_cmd(cmd, timeout=30): + if "new-workspace" in cmd: + return 0, f"{ws}\n", "" + if "list-pane-surfaces" in cmd: + return 0, f"{surface}\n", "" + return 0, "", "" + + monkeypatch.setattr(cl, "run_cmd", fake_run_cmd) + monkeypatch.setattr(cs, "await_ready", lambda **kw: (True, False)) + closed: list[str] = [] + monkeypatch.setattr(cs, "close_own_workspace", + lambda p, w, log=lambda m: None: closed.append(w)) + monkeypatch.setattr(cs, "reconcile_warm_workspaces", lambda p, keep, log=lambda m: None: None) + return paths, closed + + +def test_spawn_session_closes_workspace_when_boot_never_submitted(tmp_path, monkeypatch): + """2026-09-27/28: 16 of 30 warm sessions came up with the boot prompt typed + but never submitted, and were still declared ready. A spawn whose boot prompt + never reaches the transcript must now fail and close its workspace. + + Mutation probe: restore the old "log unconfirmed and carry on" path and the + spawn returns a session record — both asserts fail.""" + paths, closed = _spawn_env(tmp_path, monkeypatch) + monkeypatch.setattr(cs, "deliver_boot", lambda *a, **k: None) + assert cs.spawn_session(paths, Path("/boot.md"), agent=ag.CLAUDE) is None + assert closed == ["workspace:310"] + assert cs.read_session(paths) is None + + +def test_spawn_session_binds_the_transcript_that_recorded_the_boot(tmp_path, monkeypatch): + """The session is bound to the transcript deliver_boot confirmed, never to + whichever file is newest (which was often another session's).""" + paths, closed = _spawn_env(tmp_path, monkeypatch) + booted: list = [] + + def fake_boot(p, surface_ref, cwd, boot_prompt, agent): + booted.append((surface_ref, boot_prompt, agent)) + return "/confirmed.jsonl" + monkeypatch.setattr(cs, "deliver_boot", fake_boot) + sess = cs.spawn_session(paths, Path("/boot.md"), agent=ag.CLAUDE) + assert booted == [("surface:310", Path("/boot.md"), ag.CLAUDE)] + assert sess["transcript_path"] == "/confirmed.jsonl" + assert sess["ws_ref"] == "workspace:310" + assert closed == [] + + def test_spawn_session_closes_workspace_when_no_surface(tmp_path, monkeypatch): """A workspace with no pane surface is unusable but still alive. spawn_session must close it rather than leak it. Mutation probe: drop the cleanup call on diff --git a/tests/test_comms_submission.py b/tests/test_comms_submission.py new file mode 100644 index 0000000..38283f8 --- /dev/null +++ b/tests/test_comms_submission.py @@ -0,0 +1,282 @@ +"""Tests for the warm session's submission check and workspace liveness. + +2026-09-27/28: warm sessions came up with the boot prompt typed but never +submitted, Slack messages sat in the prompt box for hours, and every refused +cmux connection was read as "workspace gone" and closed a healthy session. +These pin the pure pieces that now decide both: what's in the prompt box, +whether a transcript recorded a prompt, when to press Enter again, and whether +a cmux failure means the workspace is gone. +""" +from __future__ import annotations + +import json +import os +from pathlib import Path + +import comms_session as cs + +RULE = "─" * 60 + + +def _screen(box_lines: list[str], history: list[str] | None = None) -> str: + return "\n".join([*(history or []), RULE, *box_lines, RULE, + " main │ ●1 │ context 11% │ $2.13 │ #c2f4fe01", + " ⏵⏵ bypass permissions on (shift+tab to cycle)"]) + + +# ─── input_box_text ───────────────────────────────────────────────────────── + + +def test_input_box_text_reads_a_wrapped_prompt(): + """The live stuck boot prompt from 2026-09-28 wrapped across two rows.""" + screen = _screen(["❯ Read /x/prompt.md in full and execute every instruction in it. [boot", + " 20260928T153605Z-7988]"]) + box = cs.input_box_text(screen) + assert box == "Read /x/prompt.md in full and execute every instruction in it. [boot 20260928T153605Z-7988]" + + +def test_input_box_text_empty_box_is_empty_string_not_none(): + assert cs.input_box_text(_screen(["❯ "])) == "" + + +def test_input_box_text_ignores_earlier_prompts_in_the_scrollback(): + """A past prompt starts with ❯ too, but only the fenced region is the box.""" + screen = _screen(["❯ "], history=["❯ Okay, how many active workspaces do you see?", + "⏺ Twelve."]) + assert cs.input_box_text(screen) == "" + + +def test_input_box_text_none_without_a_box(): + assert cs.input_box_text("just some output\nno rules here") is None + assert cs.input_box_text(f"{RULE}\nDo you trust this folder?\n{RULE}") is None + assert cs.input_box_text(f"{RULE}\n{RULE}") is None + + +# ─── box_holds ────────────────────────────────────────────────────────────── + + +def test_box_holds_matches_a_marker_split_by_a_line_wrap(): + box = "[slack channel=C1 msg_ts=17905639 89.005039 send_cli=x] hi" + assert cs.box_holds(box, "msg_ts=1790563989.005039") + + +def test_box_holds_collapsed_paste(): + assert cs.box_holds("[Pasted text #1 +12 lines]", "msg_ts=1") + + +def test_box_holds_false_for_other_text_or_empty(): + assert not cs.box_holds("something the user is typing", "msg_ts=1790563989.005039") + assert not cs.box_holds("", "msg_ts=1") + assert not cs.box_holds(None, "msg_ts=1") + + +# ─── transcript_has_submission / find_submission ───────────────────────────── + + +def _jsonl(path: Path, recs: list[dict]) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("".join(json.dumps(r) + "\n" for r in recs)) + return path + + +MARK = "msg_ts=1790563989.005039" + + +def test_submission_counts_a_user_prompt(tmp_path): + t = _jsonl(tmp_path / "t.jsonl", [ + {"type": "user", "entrypoint": "cli", + "message": {"role": "user", "content": f"[slack channel=C1 {MARK}] Are you alive?"}}]) + assert cs.transcript_has_submission(t, MARK) + + +def test_submission_counts_text_blocks_and_queued_prompts(tmp_path): + blocks = _jsonl(tmp_path / "a.jsonl", [ + {"type": "user", "message": {"role": "user", + "content": [{"type": "text", "text": f"hi {MARK}"}]}}]) + queued = _jsonl(tmp_path / "b.jsonl", [ + {"type": "queue-operation", "operation": "enqueue", "content": f"hi {MARK}"}]) + assert cs.transcript_has_submission(blocks, MARK) + assert cs.transcript_has_submission(queued, MARK) + + +def test_submission_ignores_tool_results_headless_runs_and_assistant_text(tmp_path): + """The warm session greps logs that contain msg_ts markers, the proofgate + Stop hook's headless run quotes its prompts, and the assistant may repeat + one — none of those mean the prompt was submitted.""" + t = _jsonl(tmp_path / "t.jsonl", [ + {"type": "user", "message": {"role": "user", "content": [ + {"type": "tool_result", "content": f"log line {MARK}"}]}}, + {"type": "user", "entrypoint": "sdk-cli", + "message": {"role": "user", "content": f"verify this trace: {MARK}"}}, + {"type": "assistant", "message": {"role": "assistant", + "content": [{"type": "text", "text": MARK}]}}, + {"type": "queue-operation", "operation": "dequeue"}, + {"type": "user", "message": "not-a-dict " + MARK}, + ]) + assert not cs.transcript_has_submission(t, MARK) + + +def test_submission_skips_corrupt_lines_and_missing_files(tmp_path): + t = tmp_path / "t.jsonl" + t.write_text(f"{{broken {MARK}\n\"a string {MARK}\"\n") + assert not cs.transcript_has_submission(t, MARK) + assert not cs.transcript_has_submission(tmp_path / "missing.jsonl", MARK) + + +def test_submission_reads_only_the_tail(tmp_path, monkeypatch): + monkeypatch.setattr(cs, "TRANSCRIPT_TAIL_BYTES", 200) + rec = {"type": "user", "message": {"role": "user", "content": f"old {MARK}"}} + t = tmp_path / "t.jsonl" + t.write_text(json.dumps(rec) + "\n" + ("x" * 400) + "\n") + assert not cs.transcript_has_submission(t, MARK) + + +def test_submission_droid_schema(tmp_path): + t = _jsonl(tmp_path / "t.jsonl", [ + {"type": "message", "message": {"role": "user", "content": f"go {MARK}"}}]) + assert cs.transcript_has_submission(t, MARK) + + +def _prompt(text: str) -> dict: + return {"type": "user", "entrypoint": "cli", "message": {"role": "user", "content": text}} + + +def test_find_submission_returns_the_transcript_that_recorded_it(tmp_path): + """Binding follows the marker, not the newest file: the newest file here is + a different session's.""" + hit = _jsonl(tmp_path / "hit.jsonl", [_prompt(f"x {MARK}")]) + other = _jsonl(tmp_path / "other.jsonl", [_prompt("unrelated")]) + os.utime(hit, (1000, 1000)) + os.utime(other, (2000, 2000)) + assert cs.find_submission(tmp_path, MARK, since=0) == str(hit) + + +def test_find_submission_skips_old_files_subagents_and_missing_dirs(tmp_path): + old = _jsonl(tmp_path / "old.jsonl", [_prompt(f"x {MARK}")]) + os.utime(old, (1000, 1000)) + _jsonl(tmp_path / "sess" / "subagents" / "a.jsonl", [_prompt(f"x {MARK}")]) + assert cs.find_submission(tmp_path, MARK, since=5000) is None + assert cs.find_submission(tmp_path / "nope", MARK, since=0) is None + + +def test_find_submission_finds_nested_session_transcripts(tmp_path): + nested = _jsonl(tmp_path / "sess" / "main.jsonl", [_prompt(f"x {MARK}")]) + assert cs.find_submission(tmp_path, MARK, since=0) == str(nested) + + +def test_boot_instruction_is_unique_per_nonce(): + a = cs.boot_instruction(Path("/p.md"), "n1") + assert a.startswith("Read /p.md in full and execute every instruction in it.") + assert "[boot n1]" in a + assert a != cs.boot_instruction(Path("/p.md"), "n2") + + +# ─── submit_until_confirmed ───────────────────────────────────────────────── + + +class FakeTerminal: + """Records keystrokes; confirms after a given number of Enter presses.""" + + def __init__(self, confirm_after_enters: int | None, box: str | None = "stuck msg_ts=1"): + self.confirm_after = confirm_after_enters + self.box = box + self.typed = 0 + self.enters = 0 + self.now = 0.0 + + def send_text(self): + self.typed += 1 + + def press_enter(self): + self.enters += 1 + + def read_box(self): + return self.box + + def confirmed(self): + return self.confirm_after is not None and self.enters >= self.confirm_after + + def sleep(self, sec): + self.now += sec + + def clock(self): + return self.now + + +def _submit(term: FakeTerminal, attempts=3, wait_sec=5): + return cs.submit_until_confirmed( + term.send_text, term.press_enter, term.read_box, term.confirmed, "msg_ts=1", + attempts=attempts, wait_sec=wait_sec, sleep=term.sleep, clock=term.clock) + + +def test_submit_confirms_on_the_first_enter(): + term = FakeTerminal(confirm_after_enters=1) + assert _submit(term) + assert (term.typed, term.enters) == (1, 1) + + +def test_submit_presses_enter_again_when_the_box_still_holds_the_text(): + """The 2026-09-27 failure: Enter was lost and the text sat in the box.""" + term = FakeTerminal(confirm_after_enters=2) + assert _submit(term) + assert (term.typed, term.enters) == (1, 2), "text typed once, Enter pressed twice" + + +def test_submit_gives_up_after_the_attempt_budget(): + term = FakeTerminal(confirm_after_enters=None) + assert not _submit(term, attempts=3) + assert (term.typed, term.enters) == (1, 3) + + +def test_submit_never_presses_enter_on_a_box_without_the_marker(): + """If the box holds someone else's text (or nothing), a retry could submit + the wrong thing — so no extra Enter, and no retyping.""" + for box in ("the user is typing here", "", None): + term = FakeTerminal(confirm_after_enters=None, box=box) + assert not _submit(term) + assert (term.typed, term.enters) == (1, 1) + + +def test_submit_waits_the_full_window_before_retrying(): + term = FakeTerminal(confirm_after_enters=None) + _submit(term, attempts=2, wait_sec=5) + assert term.now >= 10 + + +# ─── liveness ─────────────────────────────────────────────────────────────── + + +def test_classify_tree_result(): + assert cs.classify_tree_result(0, "") == cs.ALIVE + assert cs.classify_tree_result(1, "Error: invalid_params: Missing or invalid workspace_id") == cs.GONE + # The 2026-09-27 stall signature: exit 1, but cmux never answered. + refused = ("Error: Failed to connect to socket at /x/cmux.sock " + "(Connection refused, errno 61)") + assert cs.classify_tree_result(1, refused) == cs.UNKNOWN + assert cs.classify_tree_result(-1, "timeout after 10s") == cs.UNKNOWN + assert cs.classify_tree_result(1, None) == cs.UNKNOWN + + +def test_ref_listed_matches_whole_refs_only(): + listing = " workspace:258 assistant-comms (warm) #ea290b [258]\n workspace:1 Build [1]" + assert cs.ref_listed(listing, "workspace:258") + assert cs.ref_listed(listing, "workspace:1") + assert not cs.ref_listed(listing, "workspace:25") + assert not cs.ref_listed(listing, "workspace:2") + + +def test_resolve_workspace_state_retries_through_a_blip(): + answers = iter([cs.UNKNOWN, cs.ALIVE]) + slept: list = [] + assert cs.resolve_workspace_state(lambda: next(answers), attempts=3, retry_sec=2, + sleep=slept.append) == cs.ALIVE + assert slept == [2] + + +def test_resolve_workspace_state_returns_gone_at_once_and_unknown_when_silent(): + slept: list = [] + assert cs.resolve_workspace_state(lambda: cs.GONE, sleep=slept.append) == cs.GONE + assert slept == [] + assert cs.resolve_workspace_state(lambda: cs.UNKNOWN, attempts=3, retry_sec=1, + sleep=slept.append) == cs.UNKNOWN + assert slept == [1, 1], "no sleep after the last try" diff --git a/tests/test_comms_subsystem.py b/tests/test_comms_subsystem.py index 6e59523..e834230 100644 --- a/tests/test_comms_subsystem.py +++ b/tests/test_comms_subsystem.py @@ -109,9 +109,13 @@ def http(token, method, payload): def test_slack_fmt_action_line_matches_comms_lib_shape(): - line = slack.fmt_action_line({"kind": "cleanup", "key": "k", "outcome": "verified", + # Full output parity with comms_lib is pinned in test_comms_lib.py. + line = slack.fmt_action_line({"kind": "goal-edit", "key": "k", "outcome": "verified", "verified_via": "screen_read"}) - assert "*[cleanup]* ok" in line and "(!)screen_read" in line + assert line == ("I updated your goals.\n" + "Heads up: I only confirmed this by reading the screen, " + "which isn't reliable proof.\n" + "_goal-edit · k_") # ─── conversation (src-side) ──────────────────────────────────────────────── @@ -285,10 +289,41 @@ def test_heartbeat_dedup(cfg: Config, monkeypatch): stale = int(time.time()) - 99999 cfg.heartbeat_path.write_text(json.dumps({"last_pulse_ts": stale, "status": "ok"})) sub._check_heartbeat() - sub._check_heartbeat() # within dedup window + sub._check_heartbeat() # still the same outage assert len(fake.sends) == 1 +def test_heartbeat_pages_once_per_outage_then_announces_recovery(cfg: Config, monkeypatch): + """One page per outage and one recovery message, not a page every half + hour (621 of them during the 2026-09-14→27 pulse outage).""" + sub, fake = _make_subsystem(cfg, monkeypatch) + stale = int(time.time()) - 99999 + cfg.heartbeat_path.write_text(json.dumps({"last_pulse_ts": stale, "status": "ok"})) + sub._check_heartbeat() + sub._check_heartbeat() + fresh = int(time.time()) + cfg.heartbeat_path.write_text(json.dumps({"last_pulse_ts": fresh, "status": "ok"})) + sub._check_heartbeat() + sub._check_heartbeat() + assert [s["kind"] for s in fake.sends] == ["urgent", "action"] + assert fake.sends[1]["text"] == slack.fmt_heartbeat_recovered( + {"last_pulse_ts": fresh, "status": "ok"}, fresh - stale) + + +def test_heartbeat_send_disabled_and_failures_are_logged(cfg: Config, monkeypatch, caplog): + sub, fake = _make_subsystem(cfg, monkeypatch, send_enabled=False) + sub._send_heartbeat("body", "urgent") + assert fake.sends == [] + sub, _fake = _make_subsystem(cfg, monkeypatch) + + def boom(*a, **k): + raise RuntimeError("slack down") + monkeypatch.setattr("assistant.subsystems.comms.slack.send", boom) + with caplog.at_level(logging.WARNING, logger="test.comms"): + sub._send_heartbeat("body", "action") + assert "heartbeat action failed" in caplog.text + + def test_status_snapshot(cfg: Config, monkeypatch): sub, _fake = _make_subsystem(cfg, monkeypatch) st = sub.status() From 6720a686fed6fbe6972360b97e9407da1adafdf4 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Mon, 28 Sep 2026 09:36:15 -0700 Subject: [PATCH 2/7] Cover the last changed comms lines: unstat-able transcripts, unconfirmed replies, and housekeeping in the subsystem Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/comms-listen.py | 5 +---- tests/test_comms_listen_inbox.py | 7 +++++++ tests/test_comms_submission.py | 6 ++++++ tests/test_comms_subsystem.py | 7 +++++++ 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/bin/comms-listen.py b/bin/comms-listen.py index 4e1fa00..dc762dc 100755 --- a/bin/comms-listen.py +++ b/bin/comms-listen.py @@ -916,10 +916,7 @@ def _drain_inbox_once(env: dict) -> int: continue ws_key = str(item.get("ws_ref") or "ws") if not inbox_should_ping(item, last_ping.get(ws_key), now): - try: - p.unlink() - except OSError: - pass + p.unlink(missing_ok=True) held += 1 continue body = comms_lib.fmt_workspace_signal(item) diff --git a/tests/test_comms_listen_inbox.py b/tests/test_comms_listen_inbox.py index aacd3f3..90ac4de 100644 --- a/tests/test_comms_listen_inbox.py +++ b/tests/test_comms_listen_inbox.py @@ -359,6 +359,13 @@ def test_reply_not_delivered_returns_false_and_leaves_the_session(env_reply, env assert "should_clear" not in rec and "wrote" not in rec +def test_reply_unconfirmed_when_no_transcript_recorded_it(env_reply, env_inbox): + rec, _ = env_reply + rec["found"] = None + sess = _droid_sess("/bound.jsonl") + assert listen.reply_to_message(listen.comms_lib.Paths.from_env(), sess, INBOUND) == (False, sess) + + def test_feed_text_combines_messages_that_piled_up(): recs = [{"text": "Are you alive?", "msg_ts": "1.0"}, {"text": "Pulse active now?", "msg_ts": "2.0"}] diff --git a/tests/test_comms_submission.py b/tests/test_comms_submission.py index 38283f8..1d7d17a 100644 --- a/tests/test_comms_submission.py +++ b/tests/test_comms_submission.py @@ -159,6 +159,12 @@ def test_find_submission_skips_old_files_subagents_and_missing_dirs(tmp_path): assert cs.find_submission(tmp_path / "nope", MARK, since=0) is None +def test_find_submission_skips_files_it_cannot_stat(tmp_path): + (tmp_path / "dangling.jsonl").symlink_to(tmp_path / "gone.jsonl") + hit = _jsonl(tmp_path / "hit.jsonl", [_prompt(f"x {MARK}")]) + assert cs.find_submission(tmp_path, MARK, since=0) == str(hit) + + def test_find_submission_finds_nested_session_transcripts(tmp_path): nested = _jsonl(tmp_path / "sess" / "main.jsonl", [_prompt(f"x {MARK}")]) assert cs.find_submission(tmp_path, MARK, since=0) == str(nested) diff --git a/tests/test_comms_subsystem.py b/tests/test_comms_subsystem.py index e834230..432ac87 100644 --- a/tests/test_comms_subsystem.py +++ b/tests/test_comms_subsystem.py @@ -259,6 +259,13 @@ def test_both_suppressors_cover_every_receipt_kind(): f"— it would firehose to Slack") +def test_broadcast_skips_housekeeping_kinds(cfg: Config, monkeypatch): + sub, fake = _make_subsystem(cfg, monkeypatch) + for kind in ("decision-transition", "strategist-autopause", "stranded", "skipped"): + sub._broadcast_entry({"kind": kind, "key": f"{kind}:1", "outcome": "failed"}) + assert fake.sends == [] + + def test_heartbeat_pages_when_stale(cfg: Config, monkeypatch): sub, fake = _make_subsystem(cfg, monkeypatch) stale = int(time.time()) - 99999 From 14b44e14302985f09e507b35b39b6d085830a39c Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Mon, 28 Sep 2026 09:55:06 -0700 Subject: [PATCH 3/7] Back off inbound retries, replace only the session that failed, queue messages on arrival, and clear sessions before feeding them Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/agent_session.py | 10 +- bin/cmux-watcher.py | 10 +- bin/comms-listen.py | 184 +++++++++++++++++++--------- bin/comms_session.py | 41 ++++--- docs/assistant-comms-onboarding.md | 10 +- src/assistant/subsystems/comms.py | 14 ++- tests/test_agent_session.py | 14 +++ tests/test_cmux_watcher.py | 8 +- tests/test_comms_listen_delivery.py | 125 ++++++++++++++++--- tests/test_comms_listen_inbox.py | 65 ++++++++-- tests/test_comms_submission.py | 86 ++++++++++++- tests/test_comms_subsystem.py | 24 +++- 12 files changed, 463 insertions(+), 128 deletions(-) diff --git a/bin/agent_session.py b/bin/agent_session.py index ec1dca2..d913215 100644 --- a/bin/agent_session.py +++ b/bin/agent_session.py @@ -60,10 +60,18 @@ def project_slug(cwd: str | Path) -> str: return os.path.realpath(str(cwd)).replace("/", "-") +def claude_project_slug(cwd: str | Path) -> str: + """The project dir name Claude Code uses for `cwd`: every non-alphanumeric + character of the real path becomes `-`, so `/x/.worktrees/a_b` is + `-x--worktrees-a-b`. Same as project_slug for plain paths.""" + return re.sub(r"[^A-Za-z0-9-]", "-", project_slug(cwd)) + + def confirm_dir(agent: str, cwd: str | Path, home: str | Path | None = None) -> Path: """Per-cwd transcript dir a freshly-spawned `agent` session writes into.""" - return transcript_root(agent, home=home) / project_slug(cwd) + slug = project_slug(cwd) if agent == DROID else claude_project_slug(cwd) + return transcript_root(agent, home=home) / slug def record_role(obj: object) -> str | None: diff --git a/bin/cmux-watcher.py b/bin/cmux-watcher.py index 90be66e..e04eb3c 100644 --- a/bin/cmux-watcher.py +++ b/bin/cmux-watcher.py @@ -389,14 +389,6 @@ def last_lines(text: str, n: int = 3) -> str: _SESSION_ID_RE = re.compile(r"[A-Za-z0-9-]+") -def claude_project_slug(cwd: str) -> str: - """The project dir name Claude Code uses for `cwd`: every non-alphanumeric - character of the real path becomes `-`. agent_session.project_slug maps - only `/`, so dotted or underscored paths (`.worktrees`, macOS temp dirs) - are finished here.""" - return re.sub(r"[^A-Za-z0-9-]", "-", agent_session.project_slug(cwd)) - - def transcript_path(cwd: str | None, session_id: str | None, projects_dir: Path = CLAUDE_PROJECTS) -> Path | None: """Locate the Claude Code transcript for a hook payload's session. @@ -409,7 +401,7 @@ def transcript_path(cwd: str | None, session_id: str | None, return None name = f"{session_id.removeprefix('claude-')}.jsonl" if cwd: - direct = projects_dir / claude_project_slug(cwd) / name + direct = projects_dir / agent_session.claude_project_slug(cwd) / name if direct.is_file(): return direct return next((d / name for d in projects_dir.iterdir() if (d / name).is_file()), None) diff --git a/bin/comms-listen.py b/bin/comms-listen.py index dc762dc..9cdcf29 100755 --- a/bin/comms-listen.py +++ b/bin/comms-listen.py @@ -95,14 +95,19 @@ def _load_doctor(): LEDGER_POLL_SEC = float(os.environ.get("COMMS_LEDGER_POLL_SEC", "2")) HEARTBEAT_CHECK_SEC = int(os.environ.get("COMMS_HEARTBEAT_CHECK_SEC", "60")) HEARTBEAT_CONFIRM_CHECKS = 2 +# A page that failed to send is tried again after this long, not every check. +HEARTBEAT_PAGE_RETRY_SEC = 600 # Inbound messages wait on disk until the warm session confirms it received # them. The slack cursor moves past a message as soon as it's polled, so before # this queue a message that arrived while no session was up was lost for good # (2026-09-27: two "are you alive?" messages). Undelivered messages are retried -# every PENDING_RETRY_SEC and given up after PENDING_MAX_AGE_SEC, when an -# answer would no longer help. +# after PENDING_RETRY_SEC, doubling after each failed try up to +# PENDING_RETRY_MAX_SEC (a retry can spawn a session, so an unbacked-off loop +# could repeat the 2026-09-14 respawn storm), and given up after +# PENDING_MAX_AGE_SEC, when an answer would no longer help. PENDING_RETRY_SEC = float(os.environ.get("COMMS_PENDING_RETRY_SEC", "30")) +PENDING_RETRY_MAX_SEC = 1800.0 PENDING_MAX_AGE_SEC = float(os.environ.get("COMMS_PENDING_MAX_AGE_SEC", str(3 * 3600))) RESTART_NOTICE = ("My chat session isn't responding right now. I'll answer as soon " "as it's back.") @@ -220,18 +225,35 @@ def _note_cmux_answer(answered: bool) -> None: _cmux_silent_since = None +def spawn_backoff_remaining(failures: int, failed_at: float, now: float) -> float: + """Seconds until the next spawn may be tried after `failures` failed spawns + in a row, the last at `failed_at`. Shared by every caller of _warm_session, + so the watchdog and inbound retries together can't spawn faster than the + watchdog's own backoff curve (2026-09-14: ~200 leaked spawns killed cmux).""" + if failures <= 0: + return 0.0 + return max(0.0, failed_at + watchdog_delay(failures) - now) + + +# Consecutive failed spawns and when the last one failed (see spawn_backoff_remaining). +_spawn_failures = 0 +_spawn_failed_at = 0.0 + + def _warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False, - force_respawn: bool = False) -> tuple[dict | None, str]: + replace_ws: str | None = None) -> tuple[dict | None, str]: """The live warm-session record and how it was obtained, spawning one if none is alive. A session is replaced only when cmux says its workspace is gone, or when it's alive and the caller asks: respawn_on_stale (inbound path only) for a session whose model id no longer matches the current backend, or - force_respawn for one that didn't accept a typed prompt. When cmux doesn't - answer at all, the session is left alone — a refused or timed-out check - says nothing about the workspace (2026-09-27: every such check used to - close a healthy session and spawn another onto a stalled cmux). + replace_ws for a session that didn't accept a typed prompt — honored only + while the registry still names that workspace, so a newer healthy session + is never closed in its place. When cmux doesn't answer at all, the session + is left alone — a refused or timed-out check says nothing about the + workspace (2026-09-27: every such check used to close a healthy session and + spawn another onto a stalled cmux). Spawns back off after failures. On respawn, close the prior warm workspace first so we never leak Claude processes. close_own_workspace is title-guarded — it only ever closes an @@ -241,6 +263,7 @@ def _warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False, Serialized by _warm_session_lock: the watchdog tick and an inbound message can both call this concurrently, and without a guard both would see the session dead and double-spawn. The lock scopes only the respawn decision.""" + global _spawn_failures, _spawn_failed_at with _warm_session_lock: sess = comms_session.read_session(paths) if sess: @@ -253,7 +276,7 @@ def _warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False, # only the INBOUND path respawns it, BEFORE it feeds its own # message. The watchdog leaves a live session alone: closing it # there could race an active reply. - if force_respawn: + if replace_ws and replace_ws == sess["ws_ref"]: why = "didn't accept the typed message" elif respawn_on_stale and not comms_session.warm_session_model_is_current(paths, sess): why = f"model stale ({sess.get('model')!r} — backend changed since spawn)" @@ -264,8 +287,17 @@ def _warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False, log(f"warm session {sess['ws_ref']} {why} — closing it and respawning") comms_session.close_own_workspace(paths, sess["ws_ref"], log=log) comms_session.clear_session_registry(paths) + wait = spawn_backoff_remaining(_spawn_failures, _spawn_failed_at, time.time()) + if wait > 0: + log(f"warm session spawn backing off {int(wait)}s after {_spawn_failures} failure(s)") + return None, SESSION_NONE spawned = comms_session.spawn_session(paths, WARM_PROMPT, log=log) - return spawned, (SESSION_SPAWNED if spawned else SESSION_NONE) + if spawned: + _spawn_failures = 0 + return spawned, SESSION_SPAWNED + _spawn_failures += 1 + _spawn_failed_at = time.time() + return None, SESSION_NONE def ensure_warm_session(paths: comms_lib.Paths, *, respawn_on_stale: bool = False) -> dict | None: @@ -289,20 +321,37 @@ def feed_text(recs: list[dict], channel: str) -> str: f"Answer them together in one reply. {parts}") +def _clear_if_full(paths: comms_lib.Paths, sess: dict) -> dict: + """Clear-and-resume the warm session when its context is past the + threshold: >= 50% (claude) or the size proxy (droid). Runs before a message + is fed rather than right after one, so it never cuts into the reply it + triggered, and under _warm_session_lock, since clear_session can fall back + to a respawn that must not race the watchdog's. Returns the session to + feed.""" + # Thread the session's provider through every transcript-root / context call: + # a Droid session writes under ~/.factory/sessions with no usage block, so a + # claude default here would read the wrong root and never clear (G3). + agent = sess.get("agent") or agent_session.CLAUDE + bound = sess.get("transcript_path") + if not bound or not comms_session.should_clear(bound, agent=agent): + return sess + log(f"context threshold reached ({agent}) — clear-and-resume") + with _warm_session_lock: + return comms_session.clear_session(paths, sess, WARM_PROMPT, agent=agent, log=log) + + def reply_to_message(paths: comms_lib.Paths, sess: dict, recs: list[dict]) -> tuple[bool, dict]: """Hand inbound message(s) to the warm session and confirm its transcript - recorded them, then /clear if context >= 50%. Returns (delivered, the - possibly refreshed session record). + recorded them. Returns (delivered, the session that was fed, possibly + refreshed). Confirmation looks for the message's marker in the bound transcript first, then in any transcript in the session's project folder, and rebinds the session to wherever it landed.""" + sess = _clear_if_full(paths, sess) channel = str(recs[-1].get("channel")) marker = f"msg_ts={recs[-1].get('msg_ts')}" - # Thread the session's provider through every transcript-root / context call: - # a Droid session writes under ~/.factory/sessions with no usage block, so a - # claude default here would read the wrong root and never clear (G3). agent = sess.get("agent") or agent_session.CLAUDE project_dir = comms_session.project_dir_for_cwd(sess["cwd"], agent) bound = sess.get("transcript_path") @@ -326,15 +375,6 @@ def confirmed() -> bool: if not delivered: return False, sess transcript = found[-1] - - # Context management: clear-and-resume at >= 50% (claude) or the size proxy - # (droid). should_clear + clear_session are provider-aware; for droid a - # "clear" is a lossless respawn since durable memory lives in conversation.jsonl. - # clear_session owns the registry update and returns the refreshed record. - if comms_session.should_clear(transcript, agent=agent): - log(f"context threshold reached ({agent}) — clear-and-resume") - return True, comms_session.clear_session(paths, sess, WARM_PROMPT, agent=agent, log=log) - if transcript != bound: log(f"warm session transcript rebound to {transcript}") comms_session.write_session(paths, sess["ws_ref"], sess["surface_ref"], @@ -415,17 +455,20 @@ def _record_inbound(rec: dict) -> None: cli(args, timeout=10) -def _restart_notice_path(paths: comms_lib.Paths) -> Path: - return paths.comms_dir / "restart-notice.json" +def _restart_notice_path(paths: comms_lib.Paths, channel: str) -> Path: + safe = "".join(c if c.isalnum() else "-" for c in channel) + return paths.comms_dir / f"restart-notice-{safe}.json" def _notify_restart_once(paths: comms_lib.Paths, channel: str, env: dict | None) -> None: - """Tell the user, once per outage, that their message is waiting. The marker - is written before sending, so a failing send can't repeat every retry.""" - marker = _restart_notice_path(paths) + """Tell the user, once per outage per channel, that their message is + waiting. The marker is written before sending, so a failing send can't + repeat every retry; it's cleared once nothing is left waiting.""" + marker = _restart_notice_path(paths, channel) if marker.exists(): return - marker.write_text(json.dumps({"ts": time.time(), "channel": channel})) + marker.parent.mkdir(parents=True, exist_ok=True) + marker.write_text(json.dumps({"ts": time.time()})) rc, _out, err = cli(_send_args(RESTART_NOTICE, "reply", channel, None), timeout=30, env=env) log(f"restart notice sent to {channel}" if rc == 0 else f"restart notice rc={rc} err={err.strip()[:160]}") @@ -442,24 +485,34 @@ def _deliver_pending(paths: comms_lib.Paths, channel: str, env: dict | None) -> f"{comms_lib.fmt_age(int(message_age_sec(r, now)))} — giving up on it") remove_pending(paths, [r.get("msg_ts") for r in expired]) if not fresh: + _restart_notice_path(paths, channel).unlink(missing_ok=True) return True sess, how = _warm_session(paths, respawn_on_stale=True) delivered = False if sess and how != SESSION_UNREACHABLE: - delivered, _sess = reply_to_message(paths, sess, fresh) - if not delivered: - _warm_session(paths, force_respawn=True) + delivered, fed = reply_to_message(paths, sess, fresh) + # Replace an existing session that didn't take the message. One that was + # just spawned (here or by a clear) is left for the next, backed-off + # try, so a Claude that can't accept input costs one spawn per retry. + if not delivered and how == SESSION_ALIVE and fed["ws_ref"] == sess["ws_ref"]: + _warm_session(paths, replace_ws=sess["ws_ref"]) if delivered: remove_pending(paths, [r.get("msg_ts") for r in fresh]) - _restart_notice_path(paths).unlink(missing_ok=True) + _restart_notice_path(paths, channel).unlink(missing_ok=True) return True - log(f"inbound: {len(fresh)} message(s) waiting for the warm session " - f"(session={how}); retrying in {int(PENDING_RETRY_SEC)}s") + log(f"inbound: {len(fresh)} message(s) waiting for the warm session (session={how})") if message_age_sec(fresh[0], now) >= RESTART_NOTICE_AFTER_SEC: _notify_restart_once(paths, channel, env) return False +def pending_retry_delay(failures: int) -> float: + """Seconds before retrying undelivered messages after `failures` failed + tries in a row: PENDING_RETRY_SEC, doubling, capped at + PENDING_RETRY_MAX_SEC.""" + return min(PENDING_RETRY_SEC * (2 ** max(0, failures - 1)), PENDING_RETRY_MAX_SEC) + + def _poll_thread(stop: threading.Event, env: dict, msg_queue: queue.Queue) -> None: """Continuously poll Slack for inbound messages and enqueue them. @@ -487,33 +540,38 @@ def _poll_thread(stop: threading.Event, env: dict, msg_queue: queue.Queue) -> No stop.wait(SLACK_POLL_INTERVAL_SEC) -def _channel_worker(channel_id: str, ch_queue: queue.Queue, stop: threading.Event, +def _channel_worker(channel_id: str, wake: queue.Queue, stop: threading.Event, env: dict | None = None) -> None: - """Per-channel worker: queues each inbound message on disk, delivers the - queue, and retries every PENDING_RETRY_SEC until the warm session confirms + """Per-channel worker: delivers the channel's on-disk queue as soon as a + message arrives (inbound_loop has already recorded and queued it), then + retries with backoff (pending_retry_delay) until the warm session confirms it. Serializes replies for one channel while other channels run concurrently.""" paths = comms_lib.Paths.from_env() waiting = bool(split_pending(read_pending(paths), channel_id, time.time(), PENDING_MAX_AGE_SEC)[0]) last_try = 0.0 + failures = 0 while not stop.is_set(): try: - rec = ch_queue.get(timeout=1) - except queue.Empty: - rec = None - if rec is not None: - log(f"inbound channel={channel_id} msg={rec.get('msg_ts')} " - f"text={rec.get('text', '')[:80]!r}") - _record_inbound(rec) - add_pending(paths, rec) + wake.get(timeout=1) + woken = True waiting = True - if waiting and (rec is not None or time.time() - last_try >= PENDING_RETRY_SEC): - last_try = time.time() - try: - waiting = not _deliver_pending(paths, channel_id, env) - except Exception as e: # noqa: BLE001 — one bad pass must never kill the channel - log(f"inbound delivery error (will retry): {type(e).__name__}: {e}") + except queue.Empty: + woken = False + due = failures == 0 or time.time() - last_try >= pending_retry_delay(failures) + if not (waiting and (woken or due)): + continue + try: + waiting = not _deliver_pending(paths, channel_id, env) + except Exception as e: # noqa: BLE001 — one bad pass must never kill the channel + log(f"inbound delivery error (will retry): {type(e).__name__}: {e}") + # Timed from the end of the try: a try that waits on a spawn can take + # minutes, and timing from its start would retry the moment it ends. + last_try = time.time() + failures = failures + 1 if waiting else 0 + if waiting: + log(f"inbound: retrying in {int(pending_retry_delay(failures))}s") def inbound_loop(stop: threading.Event, env: dict) -> None: @@ -527,15 +585,15 @@ def inbound_loop(stop: threading.Event, env: dict) -> None: def worker_for(channel_id: str) -> queue.Queue: if channel_id not in channel_workers: - ch_q: queue.Queue = queue.Queue() + wake: queue.Queue = queue.Queue() t = threading.Thread( target=_channel_worker, - args=(channel_id, ch_q, stop, env), + args=(channel_id, wake, stop, env), name=f"inbound-{channel_id}", daemon=True, ) t.start() - channel_workers[channel_id] = (ch_q, t) + channel_workers[channel_id] = (wake, t) return channel_workers[channel_id][0] # Messages still queued from before a restart get their workers right away. @@ -552,7 +610,14 @@ def worker_for(channel_id: str) -> queue.Queue: rec = msg_queue.get(timeout=1) except queue.Empty: continue - worker_for(str(rec.get("channel") or "default")).put(rec) + channel_id = str(rec.get("channel") or "default") + log(f"inbound channel={channel_id} msg={rec.get('msg_ts')} " + f"text={rec.get('text', '')[:80]!r}") + # On disk before the worker sees it: the slack cursor has already moved + # past this message, and the worker may be busy for minutes. + _record_inbound(rec) + add_pending(paths, rec) + worker_for(channel_id).put(True) # --------------------------------------------------------------------------- warm-session liveness watchdog @@ -1008,6 +1073,7 @@ def heartbeat_loop(stop: threading.Event, env: dict) -> None: paths = comms_lib.Paths.from_env() paged_last_ts: int | None = None # the stale heartbeat's last pulse when we paged unhealthy_checks = 0 + last_page_try = 0.0 log("heartbeat loop started (slack)") while not stop.is_set(): try: @@ -1028,10 +1094,12 @@ def heartbeat_loop(stop: threading.Event, env: dict) -> None: or hb.get("status") in {"frozen", "stale_world", "respawn-requested"}) unhealthy_checks = unhealthy_checks + 1 if unhealthy else 0 action = heartbeat_action(unhealthy_checks, paged_last_ts is not None) - if action == "page": + if action == "page" and time.time() - last_page_try >= HEARTBEAT_PAGE_RETRY_SEC: + last_page_try = time.time() body = comms_lib.fmt_heartbeat_alert(hb, age) rc, _, _err = cli(_send_args(body, "urgent", target, None), timeout=30, env=env) - paged_last_ts = last_ts + if rc == 0: + paged_last_ts = last_ts log(f"heartbeat-stale page age={age}s rc={rc}") elif action == "recover": body = comms_lib.fmt_heartbeat_recovered(hb, max(0, last_ts - paged_last_ts)) diff --git a/bin/comms_session.py b/bin/comms_session.py index c9913a3..ded92ea 100644 --- a/bin/comms_session.py +++ b/bin/comms_session.py @@ -233,10 +233,10 @@ def clear_session_registry(paths: comms_lib.Paths) -> None: def project_dir_for_cwd(cwd: str, agent: str = agent_session.CLAUDE) -> Path: """Per-cwd transcript dir a warm `agent` session writes into. Claude: - ~/.claude/projects/; Droid: ~/.factory/sessions/. slug = the - realpath with '/' → '-'. Delegates to agent_session.confirm_dir (the single - source of truth for both roots), rooted at this module's HOME so a tmp-home - test resolves against its own tree.""" + ~/.claude/projects/; Droid: ~/.factory/sessions/. Delegates to + agent_session.confirm_dir (the single source of truth for both roots and + slugs), rooted at this module's HOME so a tmp-home test resolves against + its own tree.""" return agent_session.confirm_dir(agent, cwd, home=HOME) @@ -417,9 +417,14 @@ def submit_until_confirmed(send_text, press_enter, read_box, confirmed, marker: Enter was lost or became a newline — press Enter again, up to `attempts` presses in all. The text is never retyped and Enter is never pressed on a box that doesn't hold the marker, so a retry can't double-send a prompt or - submit someone else's half-typed input. All I/O is injected.""" - send_text() - sleep(0.5) + submit someone else's half-typed input. A prompt an earlier try already + got recorded isn't typed again, and one an earlier try left sitting in the + box only gets its Enter. All I/O is injected.""" + if confirmed(): + return True + if not box_holds(read_box(), marker): + send_text() + sleep(0.5) press_enter() for attempt in range(attempts): deadline = clock() + wait_sec @@ -504,15 +509,16 @@ def _surface_read_text(paths: comms_lib.Paths, surface_ref: str, lines: int = 20 return d.get("text", "") or "" -def _probe_workspace(paths: comms_lib.Paths, ws_ref: str) -> str: # pragma: no cover - live cmux I/O +def probe_workspace(paths: comms_lib.Paths, ws_ref: str, run=None) -> str: """One look at a workspace. When `tree` fails without saying the ref is - unknown, a successful `list-workspaces` still settles it either way.""" - rc, _, err = comms_lib.run_cmd( - [str(paths.cmux_bin), "tree", "--workspace", ws_ref, "--json"], timeout=10) + unknown, a successful `list-workspaces` still settles it either way; if + that fails too, the answer is UNKNOWN. `run` defaults to comms_lib.run_cmd.""" + run = run or comms_lib.run_cmd + rc, _, err = run([str(paths.cmux_bin), "tree", "--workspace", ws_ref, "--json"], timeout=10) state = classify_tree_result(rc, err) if state != UNKNOWN: return state - rc, out, _ = comms_lib.run_cmd([str(paths.cmux_bin), "list-workspaces"], timeout=10) + rc, out, _ = run([str(paths.cmux_bin), "list-workspaces"], timeout=10) if rc != 0: return UNKNOWN return ALIVE if ref_listed(out, ws_ref) else GONE @@ -520,7 +526,7 @@ def _probe_workspace(paths: comms_lib.Paths, ws_ref: str) -> str: # pragma: no def workspace_state(paths: comms_lib.Paths, ws_ref: str) -> str: # pragma: no cover - live cmux I/O """ALIVE, GONE, or UNKNOWN (cmux didn't answer) for a warm workspace.""" - return resolve_workspace_state(lambda: _probe_workspace(paths, ws_ref)) + return resolve_workspace_state(lambda: probe_workspace(paths, ws_ref)) def close_own_workspace(paths: comms_lib.Paths, ws_ref: str, log=lambda m: None) -> None: # pragma: no cover - live cmux I/O @@ -577,11 +583,12 @@ def submit(paths: comms_lib.Paths, surface_ref: str, text: str, marker: str, def deliver_boot(paths: comms_lib.Paths, surface_ref: str, cwd: str, boot_prompt: Path, - agent: str) -> str | None: # pragma: no cover - live cmux I/O + agent: str, submit_fn=None) -> str | None: """Type the boot prompt and return the transcript that recorded it, or None if it was never submitted. The session is bound to that exact file — never to whichever transcript happens to be newest, which on 2026-09-27/28 was - often another session's.""" + often another session's. `submit_fn` defaults to submit.""" + submit_fn = submit_fn or submit nonce = f"{time.strftime('%Y%m%dT%H%M%SZ', time.gmtime())}-{os.getpid()}" marker = f"[boot {nonce}]" project_dir = project_dir_for_cwd(cwd, agent) @@ -594,9 +601,9 @@ def confirmed() -> bool: found.append(hit) return hit is not None - if not submit(paths, surface_ref, boot_instruction(boot_prompt, nonce), marker, confirmed): + if not submit_fn(paths, surface_ref, boot_instruction(boot_prompt, nonce), marker, confirmed): return None - return found[-1] if found else None + return found[-1] def clear_session(paths: comms_lib.Paths, sess: dict, boot_prompt: Path, diff --git a/docs/assistant-comms-onboarding.md b/docs/assistant-comms-onboarding.md index 8474611..71c3b65 100644 --- a/docs/assistant-comms-onboarding.md +++ b/docs/assistant-comms-onboarding.md @@ -80,7 +80,12 @@ To stop: `launchctl bootout gui/$UID/com.assistant.assistant-comms`. | `SLACK_PING_TARGET` | optional | `C…` private channel (recommended) or `U…` user (DMed); overrides `config.slack.target`. | | `COMMS_MODEL` | optional | warm-session model (default Sonnet 4.6 1M). | | `COMMS_SLACK_POLL_SEC` | optional | inbound poll interval (default 3s). | -| `COMMS_REPLY_WAIT_SEC` | optional | max wait for a warm reply (default 120s). | +| `COMMS_SUBMIT_ATTEMPTS` / `COMMS_SUBMIT_WAIT_SEC` | optional | Enter presses to try when a typed prompt doesn't reach the transcript, and seconds to wait after each (default 3 and 15s). | +| `COMMS_LIVENESS_ATTEMPTS` / `COMMS_LIVENESS_RETRY_SEC` | optional | looks at a workspace cmux didn't answer about before calling it unknown, and seconds between them (default 3 and 3s). | +| `COMMS_PENDING_RETRY_SEC` / `COMMS_PENDING_MAX_AGE_SEC` | optional | first retry delay for an undelivered inbound message (doubles on each failure, up to 30 min), and how long to keep trying (default 30s and 3h). | +| `COMMS_RESTART_NOTICE_AFTER_SEC` | optional | how long a message waits before you get the "I'll answer as soon as it's back" note (default 60s). | +| `COMMS_INBOX_COOLDOWN_SEC` | optional | minimum gap between pings about one workspace; real questions always go through (default 900s). | +| `COMMS_LEDGER_MAX_PER_PASS` | optional | action updates per ledger pass before the rest collapse into one summary line (default 5). | ## Files it owns @@ -89,6 +94,9 @@ To stop: `launchctl bootout gui/$UID/com.assistant.assistant-comms`. - `~/.assistant/comms/threads.jsonl` — sent-message-ts ↔ ledger-key links. - `~/.assistant/comms/slack.cursor` / `ledger.cursor` — poll offsets. - `~/.assistant/comms/session.json` — the warm workspace registry. +- `~/.assistant/comms/pending-inbound.json` — inbound messages the warm session hasn't confirmed yet. +- `~/.assistant/comms/restart-notice.json` — marks that this outage's "I'll answer as soon as it's back" note went out. +- `~/.assistant/comms/inbox-cooldown.json` — when each workspace was last pinged. - `~/.assistant/comms/comms-listen.log` — the daemon's own log. ## Relationship to slack-reactor diff --git a/src/assistant/subsystems/comms.py b/src/assistant/subsystems/comms.py index f215054..3e5ccf0 100644 --- a/src/assistant/subsystems/comms.py +++ b/src/assistant/subsystems/comms.py @@ -155,9 +155,9 @@ def _check_heartbeat(self) -> None: stale = age > self.config.stale_heartbeat_sec bad = hb.get("status") in {"frozen", "stale_world", "respawn-requested"} if (stale or bad) and self._paged_last_ts is None: - self._send_heartbeat(slack.fmt_heartbeat_alert(hb, age), "urgent") - self._paged_last_ts = last_ts - self._pages += 1 + if self._send_heartbeat(slack.fmt_heartbeat_alert(hb, age), "urgent"): + self._paged_last_ts = last_ts + self._pages += 1 self.log.warning("heartbeat-stale page age=%ss", age) elif not (stale or bad) and self._paged_last_ts is not None: down = max(0, last_ts - self._paged_last_ts) @@ -165,16 +165,20 @@ def _check_heartbeat(self) -> None: self._paged_last_ts = None self.log.info("heartbeat recovered after %ss", down) - def _send_heartbeat(self, body: str, kind: str) -> None: + def _send_heartbeat(self, body: str, kind: str) -> bool: + """Send (or, with sending disabled, log) a heartbeat message. False only + when a real send failed, so the caller can try the page again.""" if not self._send_enabled: self.log.info("would send heartbeat %s (send disabled)", kind) - return + return True try: slack.send(body, self.config.target, token=self.config.bot_token, allowed=self.config.allowed_targets, kind=kind) except RuntimeError as e: self.log.warning("heartbeat %s failed target=%s: %s", kind, self.config.target, str(e)[:160]) + return False + return True def _read_heartbeat(self) -> dict: p = self.config.heartbeat_path diff --git a/tests/test_agent_session.py b/tests/test_agent_session.py index 4d24c09..32fad56 100644 --- a/tests/test_agent_session.py +++ b/tests/test_agent_session.py @@ -8,6 +8,7 @@ from __future__ import annotations import importlib.util +import os import sys from pathlib import Path @@ -71,6 +72,19 @@ def test_project_slug_and_confirm_dir_share_convention(): Path("/sandbox/.factory/sessions")) +def test_claude_confirm_dir_maps_every_non_alphanumeric(tmp_path): + """Claude names the folder for /x/.worktrees/a_b as -x--worktrees-a-b. The + old '/'-only slug pointed the warm-session boot check at an empty folder + whenever the checkout sat under .worktrees.""" + cwd = tmp_path / ".worktrees" / "comms_fix" + cwd.mkdir(parents=True) + real = os.path.realpath(cwd) + want = "".join(c if c.isalnum() or c == "-" else "-" for c in real) + assert ag.claude_project_slug(cwd) == want + assert ag.confirm_dir(ag.CLAUDE, cwd).name == want + assert ag.confirm_dir(ag.DROID, cwd).name == ag.project_slug(cwd) + + # ── spawn policy ────────────────────────────────────────────────────────────── def test_launch_command(tmp_path): diff --git a/tests/test_cmux_watcher.py b/tests/test_cmux_watcher.py index 9cbe89f..99c0a68 100644 --- a/tests/test_cmux_watcher.py +++ b/tests/test_cmux_watcher.py @@ -514,12 +514,12 @@ def test_project_slug_maps_every_non_alphanumeric(self): cwd = self.home / "dev" / "assistant" / ".worktrees" / "comms_fix" cwd.mkdir(parents=True) expected = os.path.realpath(str(cwd)).replace("/", "-").replace(".", "-").replace("_", "-") - self.assertEqual(self.mod.claude_project_slug(str(cwd)), expected) + self.assertEqual(self.mod.agent_session.claude_project_slug(str(cwd)), expected) self.assertIn("--worktrees-comms-fix", expected) def test_transcript_path_direct_hit_skips_the_scan(self): cwd = "/Users/me/dev/assistant/.worktrees/x" - want = self._transcript(self.mod.claude_project_slug(cwd), "sess-1", []) + want = self._transcript(self.mod.agent_session.claude_project_slug(cwd), "sess-1", []) with mock.patch.object(Path, "iterdir", side_effect=AssertionError("scanned all dirs")): self.assertEqual(self.mod.transcript_path(cwd, "sess-1", self.projects), want) @@ -588,7 +588,7 @@ def test_read_last_message_end_to_end(self): _assistant({"type": "text", "text": "I found two ways.\n\nPick one."}), _assistant(_ask("t9", "Should I rebase or merge main?")), ] - self._transcript(self.mod.claude_project_slug(cwd), "sess-9", records) + self._transcript(self.mod.agent_session.claude_project_slug(cwd), "sess-9", records) self.assertEqual(self.mod.read_last_message(cwd, "sess-9", question=True), "Should I rebase or merge main?") self.assertEqual(self.mod.read_last_message(cwd, "sess-9", question=False), @@ -612,7 +612,7 @@ def test_live_payload_shape_finds_the_question(self): def test_read_last_message_none_when_nothing_to_say(self): cwd = "/Users/me/dev/quiet" - self._transcript(self.mod.claude_project_slug(cwd), "sess-q", + self._transcript(self.mod.agent_session.claude_project_slug(cwd), "sess-q", [_user({"type": "text", "text": "hello?"})]) self.assertIsNone(self.mod.read_last_message(cwd, "sess-q", question=False)) diff --git a/tests/test_comms_listen_delivery.py b/tests/test_comms_listen_delivery.py index bd3d21e..b73c2cf 100644 --- a/tests/test_comms_listen_delivery.py +++ b/tests/test_comms_listen_delivery.py @@ -38,6 +38,7 @@ def _load(): @pytest.fixture def env(tmp_path: Path, monkeypatch): """Isolated comms state plus a recording fake for every CLI call.""" + monkeypatch.setattr(listen, "_spawn_failures", 0) home = tmp_path / "home" (home / ".assistant" / "comms").mkdir(parents=True) (home / ".assistant" / "inbox").mkdir(parents=True) @@ -164,7 +165,7 @@ def test_undelivered_message_stays_queued_until_the_session_takes_it(env, monkey listen.SESSION_ALIVE], [False, True]) assert listen._deliver_pending(paths, "C0", None) is False assert [r["text"] for r in listen.read_pending(paths)] == ["Are you alive?"] - assert {"force_respawn": True} in seen["warm"], "a session that didn't take input is replaced" + assert {"replace_ws": "workspace:5"} in seen["warm"], "a session that didn't take input is replaced" assert listen._deliver_pending(paths, "C0", None) is True assert listen.read_pending(paths) == [] assert seen["replied"] == [["Are you alive?"], ["Are you alive?"]] @@ -179,11 +180,37 @@ def test_no_session_sends_one_notice_per_outage_and_clears_it_on_delivery(env, m assert listen._deliver_pending(paths, "C0", None) is False assert listen._deliver_pending(paths, "C0", None) is False assert _sends(calls) == [listen.RESTART_NOTICE], "one notice, not one per retry" + assert listen._restart_notice_path(paths, "C0").exists() assert listen._deliver_pending(paths, "C0", None) is True - assert not (paths.comms_dir / "restart-notice.json").exists() + assert not listen._restart_notice_path(paths, "C0").exists() assert listen.read_pending(paths) == [] +def test_a_just_spawned_session_is_not_replaced_right_away(env, monkeypatch): + """A fresh session that refuses input is left for the next backed-off try + instead of being swapped for another spawn in the same pass.""" + paths, _ = env + listen.add_pending(paths, _msg("hi")) + seen = _stub_session(monkeypatch, [listen.SESSION_SPAWNED], [False]) + assert listen._deliver_pending(paths, "C0", None) is False + assert seen["warm"] == [{"respawn_on_stale": True}] + + +def test_notice_marker_cleared_when_everything_expired(env, monkeypatch): + paths, _ = env + listen._restart_notice_path(paths, "C0").write_text("{}") + listen.add_pending(paths, _msg("ancient", age_sec=listen.PENDING_MAX_AGE_SEC + 60)) + _stub_session(monkeypatch, [], []) + assert listen._deliver_pending(paths, "C0", None) is True + assert not listen._restart_notice_path(paths, "C0").exists(), "the next outage gets a notice" + + +def test_restart_notice_is_per_channel(): + paths = cl.Paths.from_env({"HOME": "/h", "COMMS_HOME": "/h"}) + assert listen._restart_notice_path(paths, "C0") != listen._restart_notice_path(paths, "C1") + assert listen._restart_notice_path(paths, "a/b").name == "restart-notice-a-b.json" + + def test_no_notice_for_a_brief_blip(env, monkeypatch): paths, calls = env listen.add_pending(paths, _msg("hi", age_sec=5)) @@ -237,21 +264,39 @@ def get(self, timeout=None): return item -def test_channel_worker_records_queues_and_retries(env, monkeypatch): - paths, calls = env - attempts: list[int] = [] +def test_channel_worker_delivers_on_wake_and_backs_off_retries(env, monkeypatch): + """A wake-up delivers at once; after a failure the next try waits + pending_retry_delay, timed from the END of the failed try.""" + paths, _ = env + clock = {"now": 1000.0} + monkeypatch.setattr(listen.time, "time", lambda: clock["now"]) + attempts: list[float] = [] results = iter([False, True]) def fake_deliver(p, channel, e): - attempts.append(len(listen.read_pending(p))) + attempts.append(clock["now"]) + clock["now"] += 120 # a try that waited on a spawn return next(results) monkeypatch.setattr(listen, "_deliver_pending", fake_deliver) - monkeypatch.setattr(listen, "PENDING_RETRY_SEC", 0) + monkeypatch.setattr(listen, "PENDING_RETRY_SEC", 30) stop = threading.Event() - listen._channel_worker("C0", ScriptedQueue([_msg("hello"), None, None], stop), stop) - assert attempts == [1, 1], "delivered on arrival, retried once, then idle" - assert any("conversation.py" in a[0] for a in calls), "inbound turn recorded on arrival" + + class Ticks(ScriptedQueue): + def get(self, timeout=None): + clock["now"] += 10 + return super().get(timeout) + + listen._channel_worker("C0", Ticks([True, None, None, None, None], stop), stop) + # The failed try ran 1010→1130; the retry waits 30s from 1130, not from 1010. + assert attempts == [1010.0, 1160.0], "retry 30s after the failed try ended, not at once" + assert "inbound: retrying in 30s" in _log(paths) + + +def test_pending_retry_delay_doubles_and_caps(): + assert listen.pending_retry_delay(1) == listen.PENDING_RETRY_SEC + assert listen.pending_retry_delay(2) == listen.PENDING_RETRY_SEC * 2 + assert listen.pending_retry_delay(50) == listen.PENDING_RETRY_MAX_SEC def test_channel_worker_picks_up_messages_queued_before_a_restart(env, monkeypatch): @@ -273,22 +318,25 @@ def boom(p, channel, e): monkeypatch.setattr(listen, "_deliver_pending", boom) stop = threading.Event() - listen._channel_worker("C0", ScriptedQueue([_msg("x")], stop), stop) + listen._channel_worker("C0", ScriptedQueue([True], stop), stop) assert "inbound delivery error (will retry): OSError: disk full" in _log(paths) -def test_inbound_loop_starts_workers_for_queued_and_new_messages(env, monkeypatch): - paths, _ = env +def test_inbound_loop_records_and_queues_before_the_worker_sees_it(env, monkeypatch): + """The slack cursor has moved past a message by the time it's polled, so it + must be on disk before a (possibly busy) worker gets it.""" + paths, calls = env listen.add_pending(paths, _msg("queued", channel="C1")) monkeypatch.setattr(listen, "ensure_warm_session", lambda p, **kw: None) started: list[str] = [] - received: list[str] = [] + seen_on_disk: list[list[str]] = [] stop = threading.Event() - def fake_worker(channel_id, ch_q, stop_, env_): + def fake_worker(channel_id, wake, stop_, env_): started.append(channel_id) if channel_id == "C0": - received.append(ch_q.get(timeout=5)["text"]) + wake.get(timeout=5) + seen_on_disk.append(sorted(r["text"] for r in listen.read_pending(paths))) stop_.set() def fake_poll(stop_, env_, msg_queue): @@ -300,7 +348,9 @@ def fake_poll(stop_, env_, msg_queue): t.start() t.join(timeout=10) assert not t.is_alive() - assert sorted(started) == ["C0", "C1"] and received == ["new"] + assert sorted(started) == ["C0", "C1"] + assert seen_on_disk == [["new", "queued"]] + assert any("conversation.py" in a[0] and "append" in a for a in calls) # ─── heartbeat ────────────────────────────────────────────────────────────── @@ -334,15 +384,54 @@ def fake_wait(timeout=None): paths.heartbeat.write_text(json.dumps({"last_pulse_ts": script[ticks["n"]]})) return False - monkeypatch.setattr(stop, "wait", fake_wait) + sent_after_tick: list[int] = [] + + def fake_wait_recording(timeout=None): + sent_after_tick.append(len(_sends(calls))) + return fake_wait(timeout) + + monkeypatch.setattr(stop, "wait", fake_wait_recording) listen.heartbeat_loop(stop, {}) sent = _sends(calls) + assert sent_after_tick[0] == 0, "no page after a single stale check" + assert sent_after_tick[1] == 1, "the page goes out on the second stale check" assert sent[0].startswith("PAGE") and len(sent) == 2 assert sent[1] == f"BACK {script[-1] - stale}" kinds = [a[a.index("--kind") + 1] for a in calls if "slack-send.py" in a[0]] assert kinds == ["urgent", "action"] +def test_heartbeat_page_that_failed_is_retried_later_not_every_check(env, monkeypatch): + paths, _ = env + paths.heartbeat.write_text(json.dumps({"last_pulse_ts": int(time.time()) - 5000})) + monkeypatch.setattr(cl, "fmt_heartbeat_alert", lambda hb, age: "PAGE") + tries: list[int] = [] + + def flaky_cli(argv, timeout=30, env=None): + if "slack-send.py" in argv[0]: + tries.append(1) + return (1, "", "slack down") if len(tries) == 1 else (0, "{}", "") + return 0, "", "" + + monkeypatch.setattr(listen, "cli", flaky_cli) + clock = {"now": time.time()} + monkeypatch.setattr(listen.time, "time", lambda: clock["now"]) + stop = threading.Event() + ticks = {"n": 0} + + def fake_wait(timeout=None): + ticks["n"] += 1 + clock["now"] += 60 if ticks["n"] < 4 else listen.HEARTBEAT_PAGE_RETRY_SEC + if ticks["n"] >= 6: + stop.set() + return True + return False + + monkeypatch.setattr(stop, "wait", fake_wait) + listen.heartbeat_loop(stop, {}) + assert len(tries) == 2, "one failed page, one retry after the wait, then silence" + + def test_heartbeat_loop_bad_status_pages_and_missing_config_is_quiet(env, monkeypatch): paths, calls = env paths.heartbeat.write_text(json.dumps({"last_pulse_ts": int(time.time()), "status": "frozen"})) diff --git a/tests/test_comms_listen_inbox.py b/tests/test_comms_listen_inbox.py index 90ac4de..3de0a14 100644 --- a/tests/test_comms_listen_inbox.py +++ b/tests/test_comms_listen_inbox.py @@ -305,9 +305,10 @@ def _droid_sess(transcript=None): def test_reply_threads_droid_agent_into_context_calls(env_reply, env_inbox): rec, _ = env_reply - listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), INBOUND) + rec["_clear"] = False + listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess("/t.jsonl"), INBOUND) assert rec["project_dir_agent"] == "droid" - assert rec["should_clear"] == ("/warm-t.jsonl", "droid") + assert rec["should_clear"] == ("/t.jsonl", "droid") def test_reply_types_the_header_and_waits_for_its_marker(env_reply, env_inbox): @@ -319,11 +320,24 @@ def test_reply_types_the_header_and_waits_for_its_marker(env_reply, env_inbox): assert marker == "msg_ts=1.1" -def test_reply_delegates_to_clear_session_and_returns_refreshed(env_reply, env_inbox): +def test_reply_clears_a_full_session_before_feeding_it(env_reply, env_inbox): + """A full session is cleared BEFORE the next message is typed, not right + after one lands, so the clear never cuts into the reply it triggered; the + message then goes to the refreshed session.""" rec, refreshed = env_reply - out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess(), INBOUND) + delivered, out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), + _droid_sess("/full.jsonl"), INBOUND) + assert rec["should_clear"] == ("/full.jsonl", "droid") assert rec["clear_agent"] == "droid" - assert out == (True, refreshed) + assert [t[0] for t in rec["typed"]] == ["surface:new"], "typed into the refreshed session" + assert delivered is True + + +def test_reply_skips_the_clear_under_the_threshold(env_reply, env_inbox): + rec, _ = env_reply + rec["_clear"] = False + listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess("/ok.jsonl"), INBOUND) + assert "clear_agent" not in rec and [t[0] for t in rec["typed"]] == ["surface:3"] def test_reply_rebinds_to_the_transcript_that_recorded_the_message(env_reply, env_inbox): @@ -335,7 +349,6 @@ def test_reply_rebinds_to_the_transcript_that_recorded_the_message(env_reply, en delivered, out = listen.reply_to_message(listen.comms_lib.Paths.from_env(), _droid_sess("/wrong.jsonl"), INBOUND) assert delivered is True - assert "clear_agent" not in rec, "should_clear False must not delegate to clear_session" assert rec["wrote"] == "/warm-t.jsonl" assert out["transcript_path"] == "/warm-t.jsonl" @@ -356,12 +369,13 @@ def test_reply_not_delivered_returns_false_and_leaves_the_session(env_reply, env rec["submitted"] = False sess = _droid_sess() assert listen.reply_to_message(listen.comms_lib.Paths.from_env(), sess, INBOUND) == (False, sess) - assert "should_clear" not in rec and "wrote" not in rec + assert "wrote" not in rec def test_reply_unconfirmed_when_no_transcript_recorded_it(env_reply, env_inbox): rec, _ = env_reply rec["found"] = None + rec["_clear"] = False sess = _droid_sess("/bound.jsonl") assert listen.reply_to_message(listen.comms_lib.Paths.from_env(), sess, INBOUND) == (False, sess) @@ -404,6 +418,7 @@ def _stub_warm(monkeypatch, *, alive: bool | None, model_current: bool): """Stub the comms_session machinery ensure_warm_session drives; return a dict recording which lifecycle calls fired. alive=None means cmux didn't answer.""" rec = {"closed": [], "cleared": False, "spawned": False} + monkeypatch.setattr(listen, "_spawn_failures", 0) sess = {"ws_ref": "workspace:18", "agent": "claude", "model": "us.anthropic.claude-sonnet-4-6[1m]"} state = {True: listen.comms_session.ALIVE, False: listen.comms_session.GONE, @@ -494,16 +509,44 @@ def test_warm_session_logs_when_cmux_answers_again(env_inbox, monkeypatch): cl.Paths.from_env().comms_dir / "comms-listen.log").read_text() -def test_warm_session_force_respawn_replaces_a_live_session(env_inbox, monkeypatch): +def test_warm_session_replace_ws_replaces_the_session_that_failed(env_inbox, monkeypatch): """A live session that didn't accept a typed message is replaced, so the queued message lands in a fresh one.""" rec, _ = _stub_warm(monkeypatch, alive=True, model_current=True) - out, how = listen._warm_session(cl.Paths.from_env(), force_respawn=True) + out, how = listen._warm_session(cl.Paths.from_env(), replace_ws="workspace:18") assert rec["closed"] == ["workspace:18"] and rec["spawned"] is True assert how == listen.SESSION_SPAWNED and out["ws_ref"] == "workspace:19" -def test_warm_session_reports_a_failed_spawn(env_inbox, monkeypatch): +def test_warm_session_replace_ws_never_closes_a_newer_session(env_inbox, monkeypatch): + """By the time the replace runs, another path may already have put a + healthy session in the registry; that one must survive.""" + rec, sess = _stub_warm(monkeypatch, alive=True, model_current=True) + out, how = listen._warm_session(cl.Paths.from_env(), replace_ws="workspace:17") + assert out is sess and how == listen.SESSION_ALIVE + assert rec["closed"] == [] and rec["spawned"] is False + + +def test_warm_session_spawn_backs_off_after_a_failure(env_inbox, monkeypatch): + """Every caller shares one spawn backoff, so inbound retries can't bring + back the 2026-09-14 respawn storm. Mutation probe: drop the backoff check + and the second call spawns again.""" rec, _ = _stub_warm(monkeypatch, alive=False, model_current=True) - monkeypatch.setattr(listen.comms_session, "spawn_session", lambda p, prompt, log=None: None) + spawns = [] + monkeypatch.setattr(listen.comms_session, "spawn_session", + lambda p, prompt, log=None: spawns.append(1)) assert listen._warm_session(cl.Paths.from_env()) == (None, listen.SESSION_NONE) + assert listen._warm_session(cl.Paths.from_env()) == (None, listen.SESSION_NONE) + assert spawns == [1], "the second call inside the backoff window doesn't spawn" + assert "spawn backing off" in (cl.Paths.from_env().comms_dir / "comms-listen.log").read_text() + monkeypatch.setattr(listen, "_spawn_failed_at", 0.0) + monkeypatch.setattr(listen.comms_session, "spawn_session", + lambda p, prompt, log=None: {"ws_ref": "workspace:30"}) + out, how = listen._warm_session(cl.Paths.from_env()) + assert how == listen.SESSION_SPAWNED and listen._spawn_failures == 0 + + +def test_spawn_backoff_remaining_follows_the_watchdog_curve(): + assert listen.spawn_backoff_remaining(0, 100.0, 100.0) == 0.0 + assert listen.spawn_backoff_remaining(1, 100.0, 130.0) == listen.watchdog_delay(1) - 30 + assert listen.spawn_backoff_remaining(3, 100.0, 100.0 + 10_000) == 0.0 diff --git a/tests/test_comms_submission.py b/tests/test_comms_submission.py index 1d7d17a..8163585 100644 --- a/tests/test_comms_submission.py +++ b/tests/test_comms_submission.py @@ -13,6 +13,7 @@ import os from pathlib import Path +import comms_lib as cl import comms_session as cs RULE = "─" * 60 @@ -183,15 +184,20 @@ def test_boot_instruction_is_unique_per_nonce(): class FakeTerminal: """Records keystrokes; confirms after a given number of Enter presses.""" - def __init__(self, confirm_after_enters: int | None, box: str | None = "stuck msg_ts=1"): + def __init__(self, confirm_after_enters: int | None, box: str | None = None, + typed_box: str | None = "stuck msg_ts=1"): + """`box` is what the prompt box shows before anything is typed; + `typed_box` is what it shows once the daemon has typed its text.""" self.confirm_after = confirm_after_enters self.box = box + self.typed_box = typed_box self.typed = 0 self.enters = 0 self.now = 0.0 def send_text(self): self.typed += 1 + self.box = self.typed_box def press_enter(self): self.enters += 1 @@ -238,7 +244,7 @@ def test_submit_never_presses_enter_on_a_box_without_the_marker(): """If the box holds someone else's text (or nothing), a retry could submit the wrong thing — so no extra Enter, and no retyping.""" for box in ("the user is typing here", "", None): - term = FakeTerminal(confirm_after_enters=None, box=box) + term = FakeTerminal(confirm_after_enters=None, typed_box=box) assert not _submit(term) assert (term.typed, term.enters) == (1, 1) @@ -286,3 +292,79 @@ def test_resolve_workspace_state_returns_gone_at_once_and_unknown_when_silent(): assert cs.resolve_workspace_state(lambda: cs.UNKNOWN, attempts=3, retry_sec=1, sleep=slept.append) == cs.UNKNOWN assert slept == [1, 1], "no sleep after the last try" + + +def test_submit_only_presses_enter_when_an_earlier_try_left_the_text_in_the_box(): + """Retyping would put two copies in the box and send the message twice in + one turn.""" + term = FakeTerminal(confirm_after_enters=1, box="[slack channel=C msg_ts=1] hi") + assert _submit(term) + assert (term.typed, term.enters) == (0, 1) + + +def test_submit_does_not_retype_a_prompt_that_already_landed(): + """A retry after an earlier try's prompt was recorded late must not send + it again.""" + term = FakeTerminal(confirm_after_enters=0) + assert _submit(term) + assert (term.typed, term.enters) == (0, 0) + + +# ─── probe_workspace ──────────────────────────────────────────────────────── + + +class FakeCmux: + def __init__(self, tree, listing): + self.tree, self.listing, self.calls = tree, listing, [] + + def __call__(self, argv, timeout=30): + self.calls.append(argv[1]) + return self.tree if argv[1] == "tree" else self.listing + + +PATHS = cl.Paths.from_env({"HOME": "/h", "COMMS_HOME": "/h"}) +LISTING = (0, " workspace:258 assistant-comms (warm) #ea290b [258]\n", "") +REFUSED = (1, "", "Error: Failed to connect to socket (Connection refused, errno 61)") + + +def test_probe_workspace_trusts_a_definite_tree_answer(): + alive = FakeCmux((0, "{}", ""), LISTING) + assert cs.probe_workspace(PATHS, "workspace:258", run=alive) == cs.ALIVE + gone = FakeCmux((1, "", "Error: invalid_params: Missing or invalid workspace_id"), LISTING) + assert cs.probe_workspace(PATHS, "workspace:258", run=gone) == cs.GONE + assert alive.calls == ["tree"] and gone.calls == ["tree"] + + +def test_probe_workspace_settles_an_unclear_tree_with_the_workspace_list(): + assert cs.probe_workspace(PATHS, "workspace:258", run=FakeCmux(REFUSED, LISTING)) == cs.ALIVE + assert cs.probe_workspace(PATHS, "workspace:25", run=FakeCmux(REFUSED, LISTING)) == cs.GONE + assert cs.probe_workspace(PATHS, "workspace:258", + run=FakeCmux(REFUSED, REFUSED)) == cs.UNKNOWN + + +# ─── deliver_boot ─────────────────────────────────────────────────────────── + + +def test_deliver_boot_binds_the_transcript_holding_this_boots_nonce(tmp_path, monkeypatch): + """An older transcript with an earlier boot prompt must not be picked, even + though it's newer on disk than nothing and carries the same instruction.""" + monkeypatch.setattr(cs, "project_dir_for_cwd", lambda cwd, agent="claude": tmp_path) + old = _jsonl(tmp_path / "old.jsonl", [_prompt(cs.boot_instruction(Path("/p.md"), "earlier"))]) + typed: list[str] = [] + + def fake_submit(paths, surface_ref, text, marker, confirmed): + typed.append(text) + assert not confirmed(), "nothing has recorded this boot yet" + _jsonl(tmp_path / "new.jsonl", [_prompt(text)]) + return confirmed() + + got = cs.deliver_boot(PATHS, "surface:1", "/cwd", Path("/p.md"), "claude", submit_fn=fake_submit) + assert got == str(tmp_path / "new.jsonl") and got != str(old) + assert typed[0].startswith("Read /p.md in full") and "[boot " in typed[0] + + +def test_deliver_boot_none_when_never_submitted(tmp_path, monkeypatch): + monkeypatch.setattr(cs, "project_dir_for_cwd", lambda cwd, agent="claude": tmp_path) + got = cs.deliver_boot(PATHS, "surface:1", "/cwd", Path("/p.md"), "claude", + submit_fn=lambda *a: False) + assert got is None diff --git a/tests/test_comms_subsystem.py b/tests/test_comms_subsystem.py index 432ac87..969599f 100644 --- a/tests/test_comms_subsystem.py +++ b/tests/test_comms_subsystem.py @@ -317,9 +317,29 @@ def test_heartbeat_pages_once_per_outage_then_announces_recovery(cfg: Config, mo {"last_pulse_ts": fresh, "status": "ok"}, fresh - stale) +def test_heartbeat_page_that_failed_is_tried_again(cfg: Config, monkeypatch): + sub, fake = _make_subsystem(cfg, monkeypatch) + stale = int(time.time()) - 99999 + cfg.heartbeat_path.write_text(json.dumps({"last_pulse_ts": stale, "status": "ok"})) + real_send = fake.send + attempts = {"n": 0} + + def flaky(*a, **k): + attempts["n"] += 1 + if attempts["n"] == 1: + raise RuntimeError("slack down") + return real_send(*a, **k) + + monkeypatch.setattr("assistant.subsystems.comms.slack.send", flaky) + sub._check_heartbeat() + sub._check_heartbeat() + sub._check_heartbeat() + assert attempts["n"] == 2 and len(fake.sends) == 1 + + def test_heartbeat_send_disabled_and_failures_are_logged(cfg: Config, monkeypatch, caplog): sub, fake = _make_subsystem(cfg, monkeypatch, send_enabled=False) - sub._send_heartbeat("body", "urgent") + assert sub._send_heartbeat("body", "urgent") is True assert fake.sends == [] sub, _fake = _make_subsystem(cfg, monkeypatch) @@ -327,7 +347,7 @@ def boom(*a, **k): raise RuntimeError("slack down") monkeypatch.setattr("assistant.subsystems.comms.slack.send", boom) with caplog.at_level(logging.WARNING, logger="test.comms"): - sub._send_heartbeat("body", "action") + assert sub._send_heartbeat("body", "action") is False assert "heartbeat action failed" in caplog.text From d55aa6cf6e33cfc1423c84050dc71ad373931bd2 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Mon, 28 Sep 2026 09:59:31 -0700 Subject: [PATCH 4/7] Cover worker reuse for a channel's later messages Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_comms_listen_delivery.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_comms_listen_delivery.py b/tests/test_comms_listen_delivery.py index b73c2cf..880ff96 100644 --- a/tests/test_comms_listen_delivery.py +++ b/tests/test_comms_listen_delivery.py @@ -335,12 +335,14 @@ def test_inbound_loop_records_and_queues_before_the_worker_sees_it(env, monkeypa def fake_worker(channel_id, wake, stop_, env_): started.append(channel_id) if channel_id == "C0": + wake.get(timeout=5) wake.get(timeout=5) seen_on_disk.append(sorted(r["text"] for r in listen.read_pending(paths))) stop_.set() def fake_poll(stop_, env_, msg_queue): msg_queue.put(_msg("new")) + msg_queue.put(_msg("newer")) monkeypatch.setattr(listen, "_channel_worker", fake_worker) monkeypatch.setattr(listen, "_poll_thread", fake_poll) @@ -348,8 +350,8 @@ def fake_poll(stop_, env_, msg_queue): t.start() t.join(timeout=10) assert not t.is_alive() - assert sorted(started) == ["C0", "C1"] - assert seen_on_disk == [["new", "queued"]] + assert sorted(started) == ["C0", "C1"], "one worker per channel, reused for later messages" + assert seen_on_disk == [["new", "newer", "queued"]] assert any("conversation.py" in a[0] and "append" in a for a in calls) From 439c31cb713ea09285bbf34f52502218bbdca8e1 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Mon, 28 Sep 2026 10:05:31 -0700 Subject: [PATCH 5/7] Test the live comms wrappers this change touches instead of excluding them from coverage Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/comms_session.py | 12 ++++---- tests/test_comms_session.py | 26 ++++++++++++++++ tests/test_comms_submission.py | 54 ++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/bin/comms_session.py b/bin/comms_session.py index ded92ea..d040ed9 100644 --- a/bin/comms_session.py +++ b/bin/comms_session.py @@ -524,12 +524,12 @@ def probe_workspace(paths: comms_lib.Paths, ws_ref: str, run=None) -> str: return ALIVE if ref_listed(out, ws_ref) else GONE -def workspace_state(paths: comms_lib.Paths, ws_ref: str) -> str: # pragma: no cover - live cmux I/O +def workspace_state(paths: comms_lib.Paths, ws_ref: str) -> str: """ALIVE, GONE, or UNKNOWN (cmux didn't answer) for a warm workspace.""" return resolve_workspace_state(lambda: probe_workspace(paths, ws_ref)) -def close_own_workspace(paths: comms_lib.Paths, ws_ref: str, log=lambda m: None) -> None: # pragma: no cover - live cmux I/O +def close_own_workspace(paths: comms_lib.Paths, ws_ref: str, log=lambda m: None) -> None: """Close a warm workspace THIS daemon spawned (tracked in session.json). Scope of the 2026-05-26 close-workspace ban: automation must never close a @@ -557,7 +557,7 @@ def close_own_workspace(paths: comms_lib.Paths, ws_ref: str, log=lambda m: None) else f"close {ws_ref} rc={rc}: {err.strip()[:120]}") -def send_enter(paths: comms_lib.Paths, surface_ref: str) -> None: # pragma: no cover - live cmux I/O +def send_enter(paths: comms_lib.Paths, surface_ref: str) -> None: """Submit whatever is in the prompt box by writing a carriage return to the terminal. `surface.send_key enter` reports success on a warm workspace that was never shown on screen, yet the prompt stays unsent (reproduced @@ -567,7 +567,7 @@ def send_enter(paths: comms_lib.Paths, surface_ref: str) -> None: # pragma: no def submit(paths: comms_lib.Paths, surface_ref: str, text: str, marker: str, - confirmed) -> bool: # pragma: no cover - live cmux I/O + confirmed) -> bool: """Type text into the warm session and press Enter until `confirmed()` sees it in the transcript (see submit_until_confirmed). The trailing newline is stripped: send_text streams keystrokes, so a trailing \\n would submit @@ -608,7 +608,7 @@ def confirmed() -> bool: def clear_session(paths: comms_lib.Paths, sess: dict, boot_prompt: Path, agent: str = agent_session.CLAUDE, - log=lambda m: None) -> dict: # pragma: no cover - live cmux I/O + log=lambda m: None) -> dict: """Clear-AND-resume: reset the context window losslessly, then return the refreshed session record. Per-message thread continuity comes from conversation.jsonl (the boot prompt tells the session to reconstruct it), so @@ -878,7 +878,7 @@ def _abandon_failed_spawn(paths: comms_lib.Paths, ws_ref: str | None, def spawn_session(paths: comms_lib.Paths, boot_prompt: Path, log=lambda m: None, - agent: str | None = None) -> dict | None: # pragma: no cover - live cmux I/O + agent: str | None = None) -> dict | None: """Spawn a fresh warm cmux session and deliver the responder boot prompt. Returns the session record on success, None on failure. Mirrors pulse.py's proven dispatch sequence. diff --git a/tests/test_comms_session.py b/tests/test_comms_session.py index b30da82..823d67f 100644 --- a/tests/test_comms_session.py +++ b/tests/test_comms_session.py @@ -631,6 +631,32 @@ def fake_run_cmd(cmd, timeout=30): return paths, closed +def test_spawn_session_logs_why_cmux_didnt_answer(tmp_path, monkeypatch): + home = tmp_path / "home" + (home / ".assistant").mkdir(parents=True) + paths = cl.Paths.from_env({"HOME": str(home), "COMMS_HOME": str(home)}) + monkeypatch.setattr(cl, "run_cmd", lambda cmd, timeout=30: (1, "", "Connection refused, errno 61")) + logs: list = [] + assert cs.spawn_session(paths, Path("/boot.md"), log=logs.append, agent=ag.CLAUDE) is None + assert logs == ["cmux isn't answering (Connection refused, errno 61) — cannot spawn warm session"] + + +def test_spawn_session_answers_the_trust_prompt_with_a_carriage_return(tmp_path, monkeypatch): + paths, closed = _spawn_env(tmp_path, monkeypatch) + rpc: list = [] + monkeypatch.setattr(cs, "_cmux_rpc", lambda p, m, params, timeout=15: rpc.append(params["text"])) + monkeypatch.setattr(cs.time, "sleep", lambda s: None) + + def ready_after_trust(**kw): + kw["answer_trust"]() + return True, True + + monkeypatch.setattr(cs, "await_ready", ready_after_trust) + monkeypatch.setattr(cs, "deliver_boot", lambda *a, **k: "/t.jsonl") + assert cs.spawn_session(paths, Path("/boot.md"), agent=ag.CLAUDE)["transcript_path"] == "/t.jsonl" + assert rpc[:2] == ["1", "\r"], "trust answered with 1, then a carriage return" + + def test_spawn_session_closes_workspace_when_boot_never_submitted(tmp_path, monkeypatch): """2026-09-27/28: 16 of 30 warm sessions came up with the boot prompt typed but never submitted, and were still declared ready. A spawn whose boot prompt diff --git a/tests/test_comms_submission.py b/tests/test_comms_submission.py index 8163585..ea93691 100644 --- a/tests/test_comms_submission.py +++ b/tests/test_comms_submission.py @@ -368,3 +368,57 @@ def test_deliver_boot_none_when_never_submitted(tmp_path, monkeypatch): got = cs.deliver_boot(PATHS, "surface:1", "/cwd", Path("/p.md"), "claude", submit_fn=lambda *a: False) assert got is None + + +# ─── live wrappers, driven through stubs ──────────────────────────────────── + + +def _record_rpc(monkeypatch): + calls: list = [] + monkeypatch.setattr(cs, "_cmux_rpc", + lambda p, method, params, timeout=15: calls.append((method, params))) + return calls + + +def test_send_enter_writes_a_carriage_return(monkeypatch): + """send_key enter left prompts unsent on never-shown workspaces; a "\\r" + through send_text submits them.""" + calls = _record_rpc(monkeypatch) + cs.send_enter(PATHS, "surface:9") + assert calls == [("surface.send_text", {"surface_id": "surface:9", "text": "\r"})] + + +def test_submit_types_once_then_presses_enter(monkeypatch): + calls = _record_rpc(monkeypatch) + monkeypatch.setattr(cs, "_surface_read_text", lambda p, s, lines=200: _screen(["❯ "])) + monkeypatch.setattr(cs.time, "sleep", lambda s: None) + seen = iter([False, True]) + assert cs.submit(PATHS, "surface:9", "hello msg_ts=5\n", "msg_ts=5", lambda: next(seen)) + assert calls == [("surface.send_text", {"surface_id": "surface:9", "text": "hello msg_ts=5"}), + ("surface.send_text", {"surface_id": "surface:9", "text": "\r"})] + + +def test_workspace_state_retries_the_probe(monkeypatch): + answers = iter([cs.UNKNOWN, cs.GONE]) + monkeypatch.setattr(cs, "probe_workspace", lambda p, ref: next(answers)) + monkeypatch.setattr(cs.time, "sleep", lambda s: None) + assert cs.workspace_state(PATHS, "workspace:1") == cs.GONE + + +def test_close_own_workspace_matches_whole_refs_only(monkeypatch): + """workspace:25 is a user's workspace; the listing only has the warm + workspace:258. The old substring check would have closed workspace:25.""" + closed: list = [] + + def fake_run(argv, timeout=30): + if argv[1] == "list-workspaces": + return 0, " workspace:258 assistant-comms (warm) #ea290b [258]\n workspace:25 Build [25]\n", "" + closed.append(argv[-1]) + return 0, "", "" + + monkeypatch.setattr(cl, "run_cmd", fake_run) + logs: list = [] + cs.close_own_workspace(PATHS, "workspace:25", log=logs.append) + assert closed == [] and "skip close workspace:25" in logs[0] + cs.close_own_workspace(PATHS, "workspace:258", log=logs.append) + assert closed == ["workspace:258"] From ff2a15702adc798b0c876a76ca4c193de2f9ed87 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Mon, 28 Sep 2026 10:10:47 -0700 Subject: [PATCH 6/7] Cover the clear-and-resume wait loop and the droid spawn path Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_comms_session.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_comms_session.py b/tests/test_comms_session.py index 823d67f..38a0524 100644 --- a/tests/test_comms_session.py +++ b/tests/test_comms_session.py @@ -383,6 +383,20 @@ def test_clear_session_claude_clears_in_place_and_returns_refreshed(paths: cl.Pa assert out["agent"] == ag.CLAUDE +def test_clear_session_waits_for_the_welcome_and_an_empty_box(paths: cl.Paths, monkeypatch): + """The reset takes a moment; the boot prompt waits until the welcome shows + and the box is empty, instead of matching the old banner at once.""" + rpc_calls, feeds, calls, _ = _stub_cmux(monkeypatch) + screens = iter(["still working", f"Welcome back!\n{RULE}\n❯ /clear\n{RULE}", WELCOME_SCREEN]) + reads: list = [] + monkeypatch.setattr(cs, "_surface_read_text", + lambda *a, **k: reads.append(1) or next(screens, WELCOME_SCREEN)) + cs.clear_session(paths, {"ws_ref": "workspace:5", "surface_ref": "surface:3", "cwd": "/cwd"}, + Path("/boot.md"), agent=ag.CLAUDE) + assert len(reads) == 3, "kept polling until the welcome showed with an empty box" + assert len(feeds) == 1 + + def test_clear_session_claude_respawns_when_boot_never_lands(paths: cl.Paths, monkeypatch): """If the boot prompt after /clear is never submitted, the session would sit with no instructions; clear_session falls back to a lossless respawn instead @@ -657,6 +671,16 @@ def ready_after_trust(**kw): assert rpc[:2] == ["1", "\r"], "trust answered with 1, then a carriage return" +def test_spawn_session_droid_skips_the_claude_input_wait(tmp_path, monkeypatch): + paths, _ = _spawn_env(tmp_path, monkeypatch) + waits: list = [] + monkeypatch.setattr(cs, "await_ready", lambda **kw: waits.append(kw["ready_re"]) or (True, False)) + monkeypatch.setattr(cs, "deliver_boot", lambda *a, **k: "/droid.jsonl") + sess = cs.spawn_session(paths, Path("/boot.md"), agent=ag.DROID) + assert sess["transcript_path"] == "/droid.jsonl" and sess["agent"] == ag.DROID + assert waits == [ag.ready_re(ag.DROID)], "droid has no status-bar wait" + + def test_spawn_session_closes_workspace_when_boot_never_submitted(tmp_path, monkeypatch): """2026-09-27/28: 16 of 30 warm sessions came up with the boot prompt typed but never submitted, and were still declared ready. A spawn whose boot prompt From 7cc2aebe834e72206b3754b5e4a9eb61f0fdaf45 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Mon, 28 Sep 2026 10:26:13 -0700 Subject: [PATCH 7/7] Type new text past a stale paste, page a new outage without the retry wait, and pin the page-retry window Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/comms-listen.py | 1 + bin/comms_session.py | 20 ++++++++----- docs/assistant-comms-onboarding.md | 2 +- tests/test_comms_listen_delivery.py | 46 +++++++++++++++++++++++------ tests/test_comms_submission.py | 8 +++++ 5 files changed, 59 insertions(+), 18 deletions(-) diff --git a/bin/comms-listen.py b/bin/comms-listen.py index 9cdcf29..c0943a7 100755 --- a/bin/comms-listen.py +++ b/bin/comms-listen.py @@ -1105,6 +1105,7 @@ def heartbeat_loop(stop: threading.Event, env: dict) -> None: body = comms_lib.fmt_heartbeat_recovered(hb, max(0, last_ts - paged_last_ts)) rc, _, _err = cli(_send_args(body, "action", target, None), timeout=30, env=env) paged_last_ts = None + last_page_try = 0.0 log(f"heartbeat recovered rc={rc}") comms_lib.write_comms_heartbeat(paths, status="active", pulse_idx=0, note="listen-daemon") diff --git a/bin/comms_session.py b/bin/comms_session.py index d040ed9..9ec13c3 100644 --- a/bin/comms_session.py +++ b/bin/comms_session.py @@ -325,14 +325,16 @@ def input_box_text(screen: str) -> str | None: return " ".join(part.strip() for part in [first, *body[1:]]).strip() +def box_has_marker(box: str | None, marker: str) -> bool: + """True if the prompt box shows the daemon's marker, compared without + whitespace so a line wrap inside it still matches.""" + return bool(box) and _WS_RE.sub("", marker) in _WS_RE.sub("", box) + + def box_holds(box: str | None, marker: str) -> bool: - """True if the prompt box still holds text the daemon typed: its marker - (compared without whitespace, so a line wrap inside it still matches) or a - collapsed paste.""" - if not box: - return False - return (_WS_RE.sub("", marker) in _WS_RE.sub("", box) - or "[Pasted text" in box) + """True if the prompt box still holds text the daemon just typed: its + marker, or a collapsed paste (Claude folds long typed text into one).""" + return box_has_marker(box, marker) or bool(box) and "[Pasted text" in box def _prompt_text(rec: dict) -> str | None: @@ -422,7 +424,9 @@ def submit_until_confirmed(send_text, press_enter, read_box, confirmed, marker: box only gets its Enter. All I/O is injected.""" if confirmed(): return True - if not box_holds(read_box(), marker): + # Only this prompt's own marker counts here: an old collapsed paste in the + # box says nothing about whether this text was typed. + if not box_has_marker(read_box(), marker): send_text() sleep(0.5) press_enter() diff --git a/docs/assistant-comms-onboarding.md b/docs/assistant-comms-onboarding.md index 71c3b65..bc96b95 100644 --- a/docs/assistant-comms-onboarding.md +++ b/docs/assistant-comms-onboarding.md @@ -95,7 +95,7 @@ To stop: `launchctl bootout gui/$UID/com.assistant.assistant-comms`. - `~/.assistant/comms/slack.cursor` / `ledger.cursor` — poll offsets. - `~/.assistant/comms/session.json` — the warm workspace registry. - `~/.assistant/comms/pending-inbound.json` — inbound messages the warm session hasn't confirmed yet. -- `~/.assistant/comms/restart-notice.json` — marks that this outage's "I'll answer as soon as it's back" note went out. +- `~/.assistant/comms/restart-notice-.json` — marks that this outage's "I'll answer as soon as it's back" note went out to that channel. - `~/.assistant/comms/inbox-cooldown.json` — when each workspace was last pinged. - `~/.assistant/comms/comms-listen.log` — the daemon's own log. diff --git a/tests/test_comms_listen_delivery.py b/tests/test_comms_listen_delivery.py index 880ff96..1d01738 100644 --- a/tests/test_comms_listen_delivery.py +++ b/tests/test_comms_listen_delivery.py @@ -404,34 +404,62 @@ def fake_wait_recording(timeout=None): def test_heartbeat_page_that_failed_is_retried_later_not_every_check(env, monkeypatch): + """A page that fails stays unsent, so it's tried again — but only once per + HEARTBEAT_PAGE_RETRY_SEC, not on every 60s check.""" paths, _ = env paths.heartbeat.write_text(json.dumps({"last_pulse_ts": int(time.time()) - 5000})) monkeypatch.setattr(cl, "fmt_heartbeat_alert", lambda hb, age: "PAGE") - tries: list[int] = [] + tries: list[float] = [] + clock = {"now": time.time()} - def flaky_cli(argv, timeout=30, env=None): + def failing_cli(argv, timeout=30, env=None): if "slack-send.py" in argv[0]: - tries.append(1) - return (1, "", "slack down") if len(tries) == 1 else (0, "{}", "") + tries.append(clock["now"]) + return 1, "", "slack down" return 0, "", "" - monkeypatch.setattr(listen, "cli", flaky_cli) - clock = {"now": time.time()} + monkeypatch.setattr(listen, "cli", failing_cli) monkeypatch.setattr(listen.time, "time", lambda: clock["now"]) stop = threading.Event() + steps = [60] * 8 + [listen.HEARTBEAT_PAGE_RETRY_SEC] + ticks = {"n": 0} + + def fake_wait(timeout=None): + if ticks["n"] >= len(steps): + stop.set() + return True + clock["now"] += steps[ticks["n"]] + ticks["n"] += 1 + return False + + monkeypatch.setattr(stop, "wait", fake_wait) + listen.heartbeat_loop(stop, {}) + assert len(tries) == 2, "one try, then silence for the retry window, then one more" + assert tries[1] - tries[0] >= listen.HEARTBEAT_PAGE_RETRY_SEC + + +def test_a_new_outage_right_after_recovery_pages_at_once(env, monkeypatch): + """The failed-page retry wait must not delay the page for the next outage.""" + paths, calls = env + now = time.time() + script = [now - 5000, now - 5000, now, now - 5000, now - 5000] + paths.heartbeat.write_text(json.dumps({"last_pulse_ts": int(script[0])})) + monkeypatch.setattr(cl, "fmt_heartbeat_alert", lambda hb, age: "PAGE") + monkeypatch.setattr(cl, "fmt_heartbeat_recovered", lambda hb, down: "BACK") + stop = threading.Event() ticks = {"n": 0} def fake_wait(timeout=None): ticks["n"] += 1 - clock["now"] += 60 if ticks["n"] < 4 else listen.HEARTBEAT_PAGE_RETRY_SEC - if ticks["n"] >= 6: + if ticks["n"] >= len(script): stop.set() return True + paths.heartbeat.write_text(json.dumps({"last_pulse_ts": int(script[ticks["n"]])})) return False monkeypatch.setattr(stop, "wait", fake_wait) listen.heartbeat_loop(stop, {}) - assert len(tries) == 2, "one failed page, one retry after the wait, then silence" + assert _sends(calls) == ["PAGE", "BACK", "PAGE"] def test_heartbeat_loop_bad_status_pages_and_missing_config_is_quiet(env, monkeypatch): diff --git a/tests/test_comms_submission.py b/tests/test_comms_submission.py index ea93691..1012470 100644 --- a/tests/test_comms_submission.py +++ b/tests/test_comms_submission.py @@ -302,6 +302,14 @@ def test_submit_only_presses_enter_when_an_earlier_try_left_the_text_in_the_box( assert (term.typed, term.enters) == (0, 1) +def test_submit_types_its_text_even_when_an_old_paste_sits_in_the_box(): + """A stale "[Pasted text" says nothing about this prompt, so the text is + still typed; the paste only matters for Enter retries.""" + term = FakeTerminal(confirm_after_enters=1, box="[Pasted text #1 +40 lines]") + assert _submit(term) + assert (term.typed, term.enters) == (1, 1) + + def test_submit_does_not_retype_a_prompt_that_already_landed(): """A retry after an earlier try's prompt was recorded late must not send it again."""