From 747afbaf9d7e852c77f2c7ef060e31daa55158ce Mon Sep 17 00:00:00 2001 From: Marius Mutu Date: Wed, 2 Sep 2026 11:05:58 +0000 Subject: [PATCH] =?UTF-8?q?feat(steering):=20mesaje=20mid-tur=20+=20/stop?= =?UTF-8?q?=20pe=20turul=20=C3=AEn=20zbor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Un al doilea mesaj trimis cât Claude încă lucra aștepta până se termina turul 1 — corecția „stai, nu în master" ajungea după ce greșeala era gata. Verificat în producție înainte de commit: mesajul 2 stătea 25s blocat în lock, apoi pornea ca tur separat. Acum canalele de chat pot ține un proces `claude` viu per canal, cu stdin deschis, și al doilea mesaj intră în ACELAȘI tur. - `src/claude_runner.py` — ClaudeProcess (steering, respawn cu --resume, drenare stderr, respawn la comutarea OpenRouter) + RunnerRegistry (max_live, reaper pe inactivitate, stop_all la shutdown) - `src/stream_json.py` — parser stream-json partajat cu `_run_claude`; pur, nu aruncă niciodată pe is_error (PlanningSession retrimite pe error_max_turns și depinde de asta) - `src/sentinels.py` — un singur loc pentru __AUDIO__/__STEERED__, în loc de 4 verificări copiate; repară și bug-ul preexistent prin care WhatsApp posta literal `__AUDIO__:/cale` - dispecer în `send_message`: lock.acquire(blocking=False) — eșecul de a lua lock-ul ESTE „rulează un tur", ceea ce elimină flagul inflight din decizie și cursa TOCTOU odată cu el - `/stop` oprește turul, nu sesiunea — active.json rămâne valid - rate limit prin proces persistent vine ca result.is_error, nu ca exit code; convertit înapoi în același RuntimeError, altfel fallback-ul local nu s-ar mai declanșa niciodată, în tăcere Steering-ul nu face niciodată cross-adapter (un mesaj text nu intră într-un tur voice: împart același channel_id). Mesajele steered dintr-un tur care pică sunt re-livrate, nu pierdute. Testat live cu CLI-ul real: corecție la secunda 10 dintr-un tur de 24s, un singur result, num_turns=2. Notă: mesajele steered sunt împachetate în [EXTERNAL CONTENT], deci o corecție formulată ca override agresiv poate fi refuzată ca prompt injection — pentru oprire folosește /stop. Suită: 1199 passed, 12 failed (toate pre-existente pe HEAD curat). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SiJGsZVSEGjRHZEJiXaxCC --- CLAUDE.md | 10 + cli.py | 70 ++ config.json | 5 + src/adapters/discord_bot.py | 22 +- src/adapters/telegram_bot.py | 25 +- src/adapters/whatsapp.py | 52 +- src/claude_runner.py | 548 +++++++++++ src/claude_session.py | 429 +++++++-- src/main.py | 16 + src/router.py | 52 +- src/sentinels.py | 40 + src/stream_json.py | 153 ++++ src/voice/pipeline.py | 3 +- tasks/steering-plan.md | 1375 ++++++++++++++++++++++++++++ tests/fake_claude.py | 124 +++ tests/test_claude_runner.py | 351 +++++++ tests/test_claude_session_mutex.py | 114 ++- tests/test_cli.py | 64 ++ tests/test_local_fallback.py | 219 ++++- tests/test_router.py | 14 +- tests/test_sentinels.py | 101 ++ tests/test_steering_dispatch.py | 433 +++++++++ tests/test_stop_command.py | 58 ++ 23 files changed, 4183 insertions(+), 95 deletions(-) create mode 100644 src/claude_runner.py create mode 100644 src/sentinels.py create mode 100644 src/stream_json.py create mode 100644 tasks/steering-plan.md create mode 100755 tests/fake_claude.py create mode 100644 tests/test_claude_runner.py create mode 100644 tests/test_sentinels.py create mode 100644 tests/test_steering_dispatch.py create mode 100644 tests/test_stop_command.py diff --git a/CLAUDE.md b/CLAUDE.md index b56712e..7abe6c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -90,6 +90,13 @@ source .venv/bin/activate && pip install -r requirements.txt **Sesiuni** (`src/claude_session.py`): o sesiune persistentă per canal, `claude --resume `. Mesajele externe împachetate în markeri `[EXTERNAL CONTENT]`. +**Steering — turnuri persistente** (`src/claude_runner.py`): pe lângă calea one-shot de mai sus (`_run_claude` — un `claude -p` per tur, procesul iese la final), canalele de chat interactive (Discord/Telegram/WhatsApp) pot ține un proces `claude` **viu** per canal, cu stdin deschis, ca un al doilea mesaj trimis cât primul încă rulează să intre în ACELAȘI tur (`ClaudeProcess.steer()`) în loc să aștepte după el. +- **Config** (`config.json → steering`): `{"enabled": false, "idle_minutes": 20, "max_live": 2}`. **Off implicit** — rollback e o linie (`enabled: false` + restart). Kill switch fără să atingi JSON versionat: variabila de mediu `ECHO_STEERING=off`. +- `heartbeat.py`, `planning_session.py` și `scheduler.py` rămân **deliberat one-shot** — folosesc `_run_claude`/`_run_claude_extra` direct, nu importă router-ul: n-are cine corecta un job cron sau o conversație de planning la mijlocul turului, deci un proces viu acolo ar adăuga doar RAM (292-541 MB per proces) pentru o capabilitate nefolosită. +- **`/stop`** oprește doar turul curent în zbor (`ClaudeProcess.stop()` / `stop_turn()` în `claude_session.py`), nu sesiunea — `sessions/active.json` rămâne valid, canalul răspunde normal la mesajul următor. +- **Diagnostic:** `eco status` arată `steering: on/off · N procese vii` (flag citit din config la fiecare apel; numărătoarea e prin `pgrep -f "--input-format stream-json"`, nu prin registry-ul din proces — `eco` rulează separat de serviciu). `eco doctor` verifică suportul binarului pentru `--input-format stream-json` doar cât timp `steering.enabled` e pornit. +- **Rețetă de reproducere manuală** (T15): cere-i lui Echo ceva cu un `sleep` de 30s+ în Bash pe canalul de test (ex. „rulează `sleep 40 && echo gata`, apoi zi-mi vremea"), apoi trimite al doilea mesaj pe același canal cât primul încă rulează — urmărește linia de log „steered N chars". **Contează:** primul spike de testare n-a dovedit nimic, pentru că Claude a mutat `sleep`-ul în `run_in_background`, iar turul s-a terminat în 7.8s înainte ca steering-ul să apuce să conteze — dacă turul se termină prea repede, cere explicit ca task-ul să blocheze în prim-plan, nu în fundal. + **State:** `sessions/active.json` — channel ID → `{session_id, model, message_count, ...}` **Credențiale** (`src/credential_store.py`): keyring de sistem, serviciu `"echo-core"`. Niciodată secrete ca argumente CLI. @@ -255,6 +262,9 @@ Fișierele Ralph (planning_session, planning_orchestrator, ralph.sh, ralph_dag, | `src/main.py` | Entry point — adaptoare + scheduler + heartbeat | | `src/router.py` | Comenzi vs mesaje Claude | | `src/claude_session.py` | Wrapper Claude CLI cu `--resume` | +| `src/claude_runner.py` | Procese Claude persistente per canal ("steering") — vezi § Arhitectură | +| `src/stream_json.py` | Parser stream-json partajat între `claude_session.py` și `claude_runner.py` | +| `src/sentinels.py` | Markeri de protocol partajați (ex. `__AUDIO__:`, `__STEERED__`) pe cele 4 căi (Discord/Telegram/WhatsApp/voice) | | `src/local_fallback_tools.py` | Registry allowlist de unelte doar-citire pentru modelul local (vezi § Fallback local) | | `src/fallback_history.py` | Istoric conversație per canal pentru fallback (6 schimburi, TTL 30 min) | | `src/net_status.py` | Status read-only mașini Proxmox/LXC prin SSH paralel | diff --git a/cli.py b/cli.py index 348e7c3..db5ab63 100755 --- a/cli.py +++ b/cli.py @@ -94,6 +94,7 @@ def cmd_status(args): print(f"WA Bridge: OFFLINE ({bridge_active})") _print_session_count() + print(_steering_status_line()) def _print_session_count(): @@ -103,6 +104,56 @@ def _print_session_count(): print(f"Sessions: {count} active") +def _count_steering_processes() -> int | None: + """Count live steering `claude` processes via the process table, not + the in-process registry — `eco` is its own short-lived process, so + `get_registry().live_count()` here would always be 0 regardless of what + the running echo-core.service actually has live (a diagnostic that + structurally cannot be non-zero is worse than none). + + Works because `ClaudeProcess._build_cmd()` (claude_runner.py) always + spawns with `--input-format stream-json`, and the one-shot path + (`_run_claude` in claude_session.py) never passes that flag — so + counting processes with it in their command line counts exactly the + live steering procs, from any process, no shared state needed. + + Returns None (unknown) only on a real failure to count. `pgrep`'s exit + code 1 means zero matches, not an error — must not be treated as one. + """ + import subprocess + try: + result = subprocess.run( + ["pgrep", "-fc", "--", "--input-format stream-json"], + capture_output=True, text=True, timeout=5, + ) + except (OSError, subprocess.TimeoutExpired): + return None + if result.returncode not in (0, 1): + return None + try: + return int(result.stdout.strip()) + except ValueError: + return None + + +def _steering_status_line() -> str: + """A3: 'steering: on/off · N procese vii'. + + Flag read through `Config()` at call time (never cached at module load — + the flag is meant to be flippable without a restart). Guarded (D1/D3): + a counting failure must not break `eco status` — a diagnostics command + that dies because the thing it diagnoses is broken is worthless. + """ + try: + from src.config import Config + enabled = bool(Config().get("steering.enabled", False)) + except Exception: + enabled = False + count = _count_steering_processes() + count_str = "necunoscut" if count is None else str(count) + return f"steering: {'on' if enabled else 'off'} · {count_str} procese vii" + + def _load_sessions_file() -> dict: """Load sessions/active.json, return {} on any error.""" try: @@ -325,6 +376,25 @@ def cmd_doctor(args): else: checks.append(("WhatsApp bridge (optional)", True)) + # 13. Steering: verify the CLI binary supports stream-json input (D1) — + # only meaningful if the feature is actually turned on. + try: + from src.config import Config + steering_enabled = bool(Config().get("steering.enabled", False)) + except Exception: + steering_enabled = False + if steering_enabled and claude_found: + try: + help_out = subprocess.run( + ["claude", "--help"], capture_output=True, text=True, timeout=10, + ).stdout + checks.append(("Claude CLI supports --input-format stream-json", + "--input-format" in help_out)) + except Exception: + checks.append(("Claude CLI supports --input-format stream-json", False)) + else: + checks.append(("Steering (optional, currently off)", True)) + # ---- Voice stack checks (Pas 10) ---- checks.extend(_voice_doctor_checks()) diff --git a/config.json b/config.json index c6fd62f..44b8604 100644 --- a/config.json +++ b/config.json @@ -127,5 +127,10 @@ "tts": { "default_engine": "pockettts", "pockettts_url": "http://127.0.0.1:7789" + }, + "steering": { + "enabled": true, + "idle_minutes": 20, + "max_live": 2 } } diff --git a/src/adapters/discord_bot.py b/src/adapters/discord_bot.py index b909f10..2a6ac25 100644 --- a/src/adapters/discord_bot.py +++ b/src/adapters/discord_bot.py @@ -29,6 +29,7 @@ from src.router import ( start_planning_session, ) from src.adapters._text_chunks import split_message +from src.sentinels import audio_path as _sentinel_audio_path, is_steered as _sentinel_is_steered from src.adapters.discord_views import ( RalphRootView, PlanningActiveView, @@ -1036,8 +1037,8 @@ def create_bot(config: Config) -> discord.Client: total = len(chunks) for i, chunk in enumerate(chunks, 1): result = await asyncio.to_thread(fast_dispatch, "audio", [voice, chunk]) - if result and result.startswith("__AUDIO__:"): - wav_path = result[len("__AUDIO__:"):] + wav_path = _sentinel_audio_path(result) + if wav_path: ogg_path = await asyncio.to_thread(_wav_to_ogg, wav_path) try: ext = "ogg" if ogg_path.endswith(".ogg") else "wav" @@ -1066,8 +1067,8 @@ def create_bot(config: Config) -> discord.Client: if rezumat: args.append("rezumat") result = await asyncio.to_thread(fast_dispatch, "audio", args) - if result and result.startswith("__AUDIO__:"): - wav_path = result[len("__AUDIO__:"):] + wav_path = _sentinel_audio_path(result) + if wav_path: ogg_path = await asyncio.to_thread(_wav_to_ogg, wav_path) try: ext = "ogg" if ogg_path.endswith(".ogg") else "wav" @@ -1298,11 +1299,18 @@ def create_bot(config: Config) -> discord.Client: adapter_name="discord", ) + # Steered: message was injected into an in-flight turn — react, + # post nothing. + if _sentinel_is_steered(response): + try: + await message.add_reaction("➡️") + except Exception: + logger.warning("steered reaction failed", exc_info=True) # Only send the final combined response if no intermediates # were delivered (avoids duplicating content). - if sent_count == 0: - if response.startswith("__AUDIO__:"): - wav_path = response[len("__AUDIO__:"):] + elif sent_count == 0: + wav_path = _sentinel_audio_path(response) + if wav_path: await message.channel.send( file=discord.File(wav_path, filename="echo-audio.wav") ) diff --git a/src/adapters/telegram_bot.py b/src/adapters/telegram_bot.py index 39d8909..8bf60bc 100644 --- a/src/adapters/telegram_bot.py +++ b/src/adapters/telegram_bot.py @@ -47,6 +47,7 @@ from src.router import ( start_planning_session, ) from src.planning_session import is_in_planning +from src.sentinels import audio_path as _sentinel_audio_path, is_steered as _sentinel_is_steered WORKSPACE_DIR = Path("/home/moltbot/workspace") ADAPTER_NAME = "telegram" @@ -747,8 +748,6 @@ async def callback_ralph(update: Update, context: ContextTypes.DEFAULT_TYPE) -> # --- Audio helpers --- -_AUDIO_PREFIX = "__AUDIO__:" - async def _send_voice_telegram(update: Update, wav_path: str) -> None: """Convertește WAV→OGG (ffmpeg) și trimite ca voice note Telegram.""" @@ -787,8 +786,8 @@ async def _fast_cmd(update: Update, name: str, args: list[str]) -> None: await update.message.chat.send_action(ChatAction.TYPING) result = await asyncio.to_thread(fast_dispatch, name, args) if result: - if result.startswith(_AUDIO_PREFIX): - wav_path = result[len(_AUDIO_PREFIX):] + wav_path = _sentinel_audio_path(result) + if wav_path: await _send_voice_telegram(update, wav_path) else: for chunk in split_message(result): @@ -1034,18 +1033,22 @@ async def handle_message(update: Update, context: ContextTypes.DEFAULT_TYPE) -> adapter_name=ADAPTER_NAME, ) - # Only send combined response if no intermediates were delivered - if sent_count == 0: - chunks = split_message(response) - for chunk in chunks: - await message.reply_text(chunk) + if _sentinel_is_steered(response): + # Injected into an in-flight turn — react, post nothing. + reaction = "➡️" + else: + # Only send combined response if no intermediates were delivered + if sent_count == 0: + chunks = split_message(response) + for chunk in chunks: + await message.reply_text(chunk) + reaction = "✅" - # Emoji reaction: ✅ = răspuns trimis try: await context.bot.set_message_reaction( chat_id=chat_id, message_id=message.message_id, - reaction=[ReactionTypeEmoji(emoji="✅")], + reaction=[ReactionTypeEmoji(emoji=reaction)], ) except Exception: pass diff --git a/src/adapters/whatsapp.py b/src/adapters/whatsapp.py index 67a5e65..d008abf 100644 --- a/src/adapters/whatsapp.py +++ b/src/adapters/whatsapp.py @@ -1,13 +1,16 @@ """WhatsApp adapter for Echo Core — connects to Node.js bridge.""" import asyncio +import base64 import logging +import os import httpx from src.config import Config from src.router import route_message from src.claude_session import clear_session, get_active_session +from src.sentinels import audio_path as _sentinel_audio_path, is_steered as _sentinel_is_steered log = logging.getLogger("echo-core.whatsapp") _security_log = logging.getLogger("echo-core.security") @@ -104,6 +107,32 @@ async def send_whatsapp(client: httpx.AsyncClient, to: str, text: str) -> bool: return False +async def send_whatsapp_audio(client: httpx.AsyncClient, to: str, wav_path: str) -> bool: + """Send a WAV file via the bridge's /send-document endpoint. + + The Baileys bridge has no dedicated voice-note (PTT) route, only + generic document upload — so this arrives as a playable attachment, + not a mic-bubble voice note. Good enough to not lose the audio. + """ + try: + with open(wav_path, "rb") as f: + data_b64 = base64.b64encode(f.read()).decode() + resp = await client.post( + f"{_bridge_url}/send-document", + json={ + "to": to, + "filename": "echo-audio.wav", + "mimetype": "audio/wav", + "data_base64": data_b64, + }, + timeout=30, + ) + return resp.status_code == 200 and resp.json().get("ok", False) + except Exception as e: + log.error("Send audio error: %s", e) + return False + + async def react_whatsapp( client: httpx.AsyncClient, to: str, message_id: str, emoji: str, *, from_me: bool = False, participant: str | None = None, @@ -222,23 +251,36 @@ async def handle_incoming(msg: dict, client: httpx.AsyncClient) -> None: ) sent_count += 1 + steered = False try: response, _is_cmd = await asyncio.to_thread( route_message, channel_id, user_id, text, on_text=on_text, adapter_name="whatsapp", ) - # Only send combined response if no intermediates were delivered - if sent_count == 0: - await send_whatsapp(client, sender, response) + if _sentinel_is_steered(response): + # Injected into an in-flight turn — react, post nothing. + steered = True + elif sent_count == 0: + # Only send combined response if no intermediates were delivered + wav_path = _sentinel_audio_path(response) + if wav_path: + if not await send_whatsapp_audio(client, sender, wav_path): + await send_whatsapp(client, sender, "Nu am putut trimite audio.") + try: + os.unlink(wav_path) + except OSError: + pass + else: + await send_whatsapp(client, sender, response) except Exception as e: log.error("Error handling message from %s: %s", user_id, e) await send_whatsapp(client, sender, "Sorry, an error occurred.") finally: - # Remove eyes reaction after responding + # Swap eyes for an arrow when steered, otherwise just clear it. if message_id: await react_whatsapp( - client, sender, message_id, "", + client, sender, message_id, "➡️" if steered else "", from_me=from_me, participant=msg.get("participant"), ) diff --git a/src/claude_runner.py b/src/claude_runner.py new file mode 100644 index 0000000..7c77001 --- /dev/null +++ b/src/claude_runner.py @@ -0,0 +1,548 @@ +"""Persistent Claude CLI processes ("steering") for Echo-Core. + +Why two paths exist +-------------------- +Echo-Core has ALWAYS run the Claude CLI one-shot: `_run_claude` in +`claude_session.py` spawns `claude -p ""`, reads the response, and +the process exits. That is simple and fine for a request/response turn, but +it means a second message sent while turn 1 is still running can't reach +Claude until turn 1 finishes — by then it's too late to say "wait, don't do +that". + +`ClaudeProcess` below keeps one `claude` subprocess ALIVE per channel, with +stdin held open, so a second message can be written into the SAME turn +(`steer()`) instead of queueing behind it. `RunnerRegistry` owns the set of +live processes: a cap (`max_live`), an idle reaper, and shutdown. + +`heartbeat.py` and `planning_session.py` deliberately stay on the one-shot +path (`_run_claude` / `start_session` / `resume_session`, unchanged) — there +is no human waiting to correct a 3am cron job or a planning conversation +mid-turn, so a live process there would only add RAM (measured 292-541 MB +per process) for a capability nobody uses. Only interactive chat channels +(Discord/Telegram/WhatsApp) route through this module, and only when +`steering.enabled` is on (that flag and the dispatch decision live in +`claude_session.py`/`router.py` — this module doesn't read config itself). + +Manual repro recipe (T15): to exercise steering by hand, ask Echo for +something with a 30s+ `sleep` in Bash ("run `sleep 40 && echo done` then +tell me the weather"), then send a second message on the same channel +while it's still running. Watch for the "steered N chars" log line below. + +Threading model +---------------- +Thread-based, NOT asyncio: `send_message` (in `claude_session.py`) is sync +code called via `asyncio.to_thread` from the async adapters, so a second +event loop here would be unused complexity. Three kinds of threads touch a +single `ClaudeProcess`: the caller's own thread running `run_turn()` +(blocking read of stdout), a caller's thread calling `steer()` concurrently, +and the shared reaper thread. `_stdin_lock` is the single lock serializing +all of them around `self.inflight` and stdin writes — see `steer()`'s +docstring for the race it closes and the one it deliberately does not (and +cannot: it is an inter-process race, not a Python one). +""" + +import atexit +import dataclasses +import enum +import json +import logging +import shutil +import subprocess +import threading +import time +from collections import deque +from pathlib import Path +from typing import Callable + +from src.claude_session import ( + CLAUDE_BIN, + DEFAULT_MODEL, + DEFAULT_TIMEOUT, + PROJECT_ROOT, + _safe_env, + build_system_prompt, + is_rate_limit_error, +) +from src.stream_json import consume_stream + +logger = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +MAX_LIVE_DEFAULT = 2 # measured 292-541 MB RSS per live `claude` process (8 GB host) +IDLE_MINUTES_DEFAULT = 20 + +# A2: `_safe_env()` checks this at spawn to decide whether to route through +# OpenRouter. On a one-shot process that's re-evaluated every turn; on a +# persistent one it would freeze at spawn time. Compared against its state +# each turn (see `_respawn_if_needed`) — the ONLY justified respawn-on-change +# (respawning on personality/ edits was explicitly rejected, see plan +# "Riscul #2"). +OPENROUTER_SEMAPHORE = PROJECT_ROOT / ".use_openrouter" + + +def _wrap_external_content(text: str) -> str: + """Same injection-protection wrapping `start_session`/`resume_session` + use — T1: a steered message MUST NOT bypass it just because it goes in + over stdin instead of argv (Section 3 finding S1, a security regression + otherwise).""" + return f"[EXTERNAL CONTENT]\n{text}\n[END EXTERNAL CONTENT]" + + +def _user_turn_line(text: str) -> str: + wrapped = _wrap_external_content(text) + return json.dumps( + {"type": "user", "message": {"role": "user", "content": wrapped}}, + ensure_ascii=False, + ) + + +# --------------------------------------------------------------------------- +# steer() outcome — X2: enum, not bool, so the fallback logic can't be +# skipped by accident. +# --------------------------------------------------------------------------- + + +class SteerStatus(enum.Enum): + STEERED = "steered" + # DANGER, not the safe case (C1): no turn was in flight when the write + # landed, so the CLI treated it as a brand-new turn N+1 whose stdout + # nobody else is reading. `SteerResult.turn` already holds that turn's + # full result — decision D4 ("consume the stream you just started"). + RAN_AS_TURN = "ran_as_turn" + # T7: the stdin write itself raised (process was dead/dying). The text + # was re-dispatched as a fresh `run_turn()` (respawning if needed) — + # `SteerResult.turn` holds ITS result. Never lost, per CLAUDE.md's + # "turnul nu se pierde niciodată". + PROCESS_DEAD = "process_dead" + + +@dataclasses.dataclass +class SteerResult: + """Return value of `ClaudeProcess.steer()`. + + `turn` is populated for every status except STEERED, and IS the + response for that text — bundling it here (instead of a bare enum) + makes it structurally impossible for a caller to see a non-STEERED + status and call `run_turn(text)` again "just in case": that would + double-send the message and bill it twice. + """ + + status: SteerStatus + turn: dict | None = None + + +# --------------------------------------------------------------------------- +# ClaudeProcess +# --------------------------------------------------------------------------- + + +class ClaudeProcess: + """One live `claude` subprocess for a single channel.""" + + def __init__(self, channel_id: str, model: str = DEFAULT_MODEL, + session_id: str | None = None, cwd: Path | str | None = None): + self.channel_id = channel_id + self.model = model + self.session_id = session_id + self.cwd = cwd or PROJECT_ROOT + self.proc: subprocess.Popen | None = None + self.inflight = False + self.last_active = time.monotonic() + + self._stdin_lock = threading.Lock() + self._lifecycle_lock = threading.Lock() + self._pending_steers: list[str] = [] + self._stderr_buf: deque[str] = deque(maxlen=50) + self._openrouter_at_spawn = False + + # -- lifecycle --------------------------------------------------------- + + def alive(self) -> bool: + return self.proc is not None and self.proc.poll() is None + + def _build_cmd(self) -> list[str]: + cmd = [ + CLAUDE_BIN, "-p", + "--input-format", "stream-json", + "--output-format", "stream-json", "--verbose", + "--model", self.model, + "--system-prompt", build_system_prompt(), + "--dangerously-skip-permissions", + "--autocompact", "auto", + ] + if self.session_id: + cmd += ["--resume", self.session_id] + return cmd + + def _spawn(self) -> None: + if not shutil.which(CLAUDE_BIN): + raise FileNotFoundError( + "Claude CLI not found. " + "Install: https://docs.anthropic.com/en/docs/claude-code" + ) + self._openrouter_at_spawn = OPENROUTER_SEMAPHORE.exists() + self.proc = subprocess.Popen( + self._build_cmd(), + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + bufsize=1, # line-buffered: a steer() write must reach the CLI promptly + env=_safe_env(), + cwd=str(self.cwd), + ) + self.inflight = False + self.last_active = time.monotonic() + self._stderr_buf.clear() + threading.Thread(target=self._drain_stderr, args=(self.proc,), daemon=True).start() + logger.info("channel=%s: spawned claude pid=%s resume=%s", + self.channel_id, self.proc.pid, bool(self.session_id)) + + def _drain_stderr(self, proc: subprocess.Popen) -> None: + """T3: MANDATORY. An undrained stderr pipe fills (~64KB) and + deadlocks the child mid-turn with no exception on our side — + this thread is the only thing preventing that.""" + try: + for line in proc.stderr: + self._stderr_buf.append(line.rstrip("\n")) + except (ValueError, OSError): + pass # pipe torn down under us — process is exiting, nothing left to drain + + def _respawn_if_needed(self) -> None: + """Call with `_lifecycle_lock` held.""" + if self.alive() and OPENROUTER_SEMAPHORE.exists() != self._openrouter_at_spawn: + logger.info("channel=%s: .use_openrouter changed since spawn — respawning (A2)", + self.channel_id) + self.stop() + if not self.alive(): + self._spawn() + + def stop(self) -> None: + """Terminate the process. Safe to call repeatedly / when already dead. + + Takes `_stdin_lock` (E-C2): a concurrent `steer()` must never write + into a pipe we're closing underneath it. + """ + with self._stdin_lock: + proc = self.proc + if proc is None: + return + try: + proc.stdin.close() + except (BrokenPipeError, OSError, ValueError): + pass + try: + proc.terminate() + proc.wait(timeout=3) + except subprocess.TimeoutExpired: + try: + proc.kill() + proc.wait(timeout=3) + except OSError: + pass + except OSError: + pass + logger.info("channel=%s: process stopped", self.channel_id) + + # -- turns --------------------------------------------------------- + + def run_turn(self, text: str, on_text: Callable[[str], None] | None = None, + timeout: int = DEFAULT_TIMEOUT) -> dict: + """Run one turn. Returns exactly the dict `_run_claude` returns, and + raises the exact same exceptions for the same reasons + (FileNotFoundError, TimeoutError, `RuntimeError("Claude CLI error + (exit 1): ...")` on a rate limit) so `router.py`'s existing handling + — including the local-fallback trigger — keeps working unchanged. + """ + with self._lifecycle_lock: + self._respawn_if_needed() + + line = _user_turn_line(text) + with self._stdin_lock: + self.inflight = True + self._pending_steers = [] + try: + self.proc.stdin.write(line + "\n") + self.proc.stdin.flush() + except (BrokenPipeError, ValueError, OSError) as exc: + self.inflight = False + raise RuntimeError(f"Claude CLI error: failed to start turn: {exc}") from exc + + return self._read_until_result(on_text=on_text, timeout=timeout) + + def steer(self, text: str) -> SteerResult: + """Write *text* into the current in-flight turn's stdin, without + opening a new turn — the common case. + + C1/E-C1: the only race a lock can close is between OUR threads — + `steer()` checks `self.inflight` under the same `_stdin_lock` that + `run_turn()`/`_read_until_result()` flips it under, so nothing here + can observe a stale `True`. What no lock can close is the CLI's own + internal turn boundary: it may have already finished turn N and + gone idle before our write physically reaches its stdin pipe, in + which case the CLI treats the write as opening turn N+1 regardless + of what we believed. That is `RAN_AS_TURN` — see decision D4 on + `SteerStatus` above. (A permanent stdout-owning reader thread would + close this residual race too; deferred as a documented upgrade, not + required for the accepted fix.) + """ + line = _user_turn_line(text) + wrote = False + became_new_turn = False + with self._stdin_lock: + was_inflight = self.inflight + if self.proc is not None: + try: + self.proc.stdin.write(line + "\n") + self.proc.stdin.flush() + wrote = True + except (BrokenPipeError, ValueError, OSError): + wrote = False + if wrote and was_inflight: + self._pending_steers.append(text) + elif wrote: + became_new_turn = True + self.inflight = True # we now own reading this orphaned turn + self._pending_steers = [] + + if wrote and was_inflight: + logger.info("channel=%s: steered %d chars into in-flight turn", + self.channel_id, len(text)) + return SteerResult(SteerStatus.STEERED) + + if became_new_turn: + logger.info( + "channel=%s: steer() found no in-flight turn — the write became " + "turn N+1, consuming its stream now (D4)", self.channel_id, + ) + turn = self._read_until_result(on_text=None, timeout=DEFAULT_TIMEOUT) + return SteerResult(SteerStatus.RAN_AS_TURN, turn=turn) + + # T7: the write itself failed — process dead/dying. Never lose the + # message: re-dispatch as a fresh (respawning) turn. + logger.info("channel=%s: steer() hit a dead process — falling back to run_turn (T7)", + self.channel_id) + turn = self.run_turn(text) + return SteerResult(SteerStatus.PROCESS_DEAD, turn=turn) + + def pop_pending_steers(self) -> list[str]: + """C3: texts steered into the turn that just failed (rate limit, + timeout, crash) — the caller MUST re-dispatch them ("turnul nu se + pierde niciodată"). Empty once the turn they belonged to succeeds.""" + pending, self._pending_steers = self._pending_steers, [] + return pending + + def _read_until_result(self, on_text: Callable[[str], None] | None, + timeout: int) -> dict: + proc = self.proc + timed_out = threading.Event() + + def _watchdog(): + try: + proc.wait(timeout=timeout) + except subprocess.TimeoutExpired: + timed_out.set() + try: + proc.kill() # M2: next run_turn() respawns with --resume + except OSError: + pass + + watchdog = threading.Thread(target=_watchdog, daemon=True) + watchdog.start() + + def _on_init(sid: str) -> None: + self.session_id = sid + + try: + result = consume_stream(proc.stdout, on_text=on_text, on_init=_on_init) + finally: + with self._stdin_lock: + self.inflight = False + self.last_active = time.monotonic() + + if timed_out.is_set(): + raise TimeoutError(f"Claude CLI timed out after {timeout}s") + + if result is None: + stderr_tail = "\n".join(self._stderr_buf)[-500:] + raise RuntimeError( + f"Claude CLI error: no result line in stream. stderr: {stderr_tail}" + ) + + if result.get("session_id"): + # E-T4: refresh every turn, not just the first — a respawn after + # turn 5 must --resume turn 5's session, not turn 1's. + self.session_id = result["session_id"] + + if result.get("is_error"): + detail = result.get("result", "") + if is_rate_limit_error(detail): + # T2/C4: persistent process surfaces a rate limit as + # `result.is_error`, never a nonzero exit code. Must raise + # this EXACT phrasing or `is_rate_limit_error()` in + # router.py/scheduler.py won't recognize it and the local + # fallback never fires. + raise RuntimeError(f"Claude CLI error (exit 1): {detail}") + # any other is_error: return it, don't raise (C4 — parser and + # this wrapper both stay non-throwing for non-rate-limit errors) + + self._pending_steers = [] + return result + + +# --------------------------------------------------------------------------- +# RunnerRegistry +# --------------------------------------------------------------------------- + + +class RunnerRegistry: + """Owns the set of live `ClaudeProcess` instances, one per channel. + + Module-level global state (like `claude_session._session_locks`) — see + `get_registry()` below. X6: no LRU eviction; at `max_live` capacity a + new channel simply degrades to the one-shot path (T10/M1) rather than + evicting someone else's live process. + """ + + def __init__(self, max_live: int = MAX_LIVE_DEFAULT, + idle_minutes: int = IDLE_MINUTES_DEFAULT, + reap_interval_s: float = 60.0): + if max_live <= 0: + # X7: don't fail silently — a mis-set config value should be + # loud in the logs, even though the *behavior* (always degrade) + # is already correct without special-casing it below. + logger.warning( + "steering.max_live=%d <= 0 — steering effectively disabled, " + "every channel degrades to one-shot", max_live, + ) + self.max_live = max_live + self.idle_minutes = max(1, idle_minutes) # X7: clamp, never "reap instantly" + self._procs: dict[str, ClaudeProcess] = {} + self._lock = threading.Lock() + self._reap_interval_s = reap_interval_s + self._stopping = threading.Event() + self._reaper = threading.Thread(target=self._reap_loop, daemon=True) + self._reaper.start() + atexit.register(self.stop_all) + + def get(self, channel_id: str, model: str = DEFAULT_MODEL, + session_id: str | None = None, cwd: Path | str | None = None) -> ClaudeProcess | None: + """Return the channel's live process, creating one if under + `max_live`. Returns None (T10) when at capacity and this channel + doesn't already have one — the caller must degrade to one-shot. + + An existing entry for *channel_id* is always returned as-is, + regardless of capacity — which is also what rules out the M1 + "two writers on one session_id" scenario: a channel already + holding a live process can never be told to fall back to one-shot + by this method. + """ + with self._lock: + proc = self._procs.get(channel_id) + if proc is not None: + return proc + if len(self._procs) >= self.max_live: + logger.info( + "channel=%s: max_live=%d reached, degrading to one-shot", + channel_id, self.max_live, + ) + return None + proc = ClaudeProcess(channel_id, model=model, session_id=session_id, cwd=cwd) + self._procs[channel_id] = proc + return proc + + def stop(self, channel_id: str) -> bool: + with self._lock: + proc = self._procs.pop(channel_id, None) + if proc is None: + return False + proc.stop() + return True + + def stop_all(self) -> None: + """Shutdown path (T4/A1) — zero orphaned `claude` processes after a + `systemctl restart`. Registered with `atexit` in `__init__`; a + caller in `main.py` should also invoke this explicitly on SIGTERM + for a synchronous shutdown (atexit is the fallback, not the primary + path).""" + self._stopping.set() + with self._lock: + procs = list(self._procs.values()) + self._procs.clear() + for proc in procs: + try: + proc.stop() + except Exception: + logger.exception("channel=%s: error stopping during stop_all", proc.channel_id) + + def live_count(self) -> int: + with self._lock: + return len(self._procs) + + # -- reaper --------------------------------------------------------- + + def _reap_loop(self) -> None: + while not self._stopping.wait(self._reap_interval_s): + self._reap_once() + + def _reap_once(self) -> None: + """T11: resilient to any single process's `stop()` raising — the + loop (and the rest of the batch) must keep going, and every reap + must be logged (a silently-dead reaper is an invisible RAM leak).""" + cutoff = time.monotonic() - self.idle_minutes * 60 + with self._lock: + candidates = [ + cid for cid, p in self._procs.items() + if not p.inflight and p.last_active < cutoff + ] + for cid in candidates: + try: + with self._lock: + proc = self._procs.get(cid) + # Re-check under lock: it may have gone inflight, been + # stopped, or been replaced since the snapshot above. + if proc is None or proc.inflight or proc.last_active >= cutoff: + continue + del self._procs[cid] + proc.stop() + logger.info("reaper: stopped idle channel=%s (idle >= %d min)", + cid, self.idle_minutes) + except Exception: + logger.exception("reaper: error stopping channel=%s — continuing", cid) + + +# --------------------------------------------------------------------------- +# Module-level singleton — mirrors `claude_session._session_locks`. +# --------------------------------------------------------------------------- + +_registry: RunnerRegistry | None = None +_registry_lock = threading.Lock() + + +def get_registry(max_live: int = MAX_LIVE_DEFAULT, + idle_minutes: int = IDLE_MINUTES_DEFAULT) -> RunnerRegistry: + """Lazy singleton. *max_live*/*idle_minutes* only apply on first call — + later calls just return the existing registry (config is read by + `router.py` per call and passed in here; this function doesn't re-read + config.json itself).""" + global _registry + if _registry is not None: + return _registry + with _registry_lock: + if _registry is None: + _registry = RunnerRegistry(max_live=max_live, idle_minutes=idle_minutes) + return _registry + + +def reset_registry_for_tests() -> None: + """H2: tests must not leak live (fake) processes into each other. Stops + and drops the current singleton so the next `get_registry()` call + builds a fresh one.""" + global _registry + with _registry_lock: + if _registry is not None: + _registry.stop_all() + _registry = None diff --git a/src/claude_session.py b/src/claude_session.py index 5c0c22d..a36f8c3 100644 --- a/src/claude_session.py +++ b/src/claude_session.py @@ -19,6 +19,8 @@ from datetime import datetime, timezone from pathlib import Path from typing import Callable +from src.stream_json import consume_stream + logger = logging.getLogger(__name__) _invoke_log = logging.getLogger("echo-core.invoke") _security_log = logging.getLogger("echo-core.security") @@ -95,6 +97,53 @@ def _get_session_lock(channel_id: str) -> threading.Lock: return _session_locks.setdefault(channel_id, threading.Lock()) +# --------------------------------------------------------------------------- +# In-flight turn registry — for /stop +# --------------------------------------------------------------------------- +# +# Etapa 0 of the steering plan: `/stop` kills only the current turn's +# subprocess, not the session (sessions/active.json is untouched). Maps +# channel_id -> the live Popen for that channel's in-flight `_run_claude` +# call. Registered at process start, removed in `_run_claude`'s `finally` +# so it's cleaned up on every exit path (success, error, timeout). +_live_procs: dict[str, subprocess.Popen] = {} +_live_procs_lock = threading.Lock() + + +def stop_turn(channel_id: str) -> bool: + """Kill the in-flight Claude CLI turn for *channel_id*, if any. + + Returns True if a process was signalled, False if nothing was running. + Does NOT touch sessions/active.json — the session survives, only the + turn dies (killed proc makes _run_claude raise; route_message's + existing error handling reports that back to the user). + + Must NOT acquire `_get_session_lock(channel_id)` — the turn thread + holds that lock for the whole call, so taking it here would deadlock. + """ + with _live_procs_lock: + proc = _live_procs.get(channel_id) + if proc is None or proc.poll() is not None: + return False + try: + proc.terminate() + proc.wait(timeout=3) + except subprocess.TimeoutExpired: + try: + proc.kill() + except OSError: + pass + except OSError: + pass + # Also remove here (not just in _run_claude's finally): the turn thread + # may take a moment to notice the process died, and a repeated /stop + # in that window should see "nothing running", not re-signal a dead proc. + with _live_procs_lock: + if _live_procs.get(channel_id) is proc: + del _live_procs[channel_id] + return True + + PERSONALITY_FILES = [ "IDENTITY.md", "SOUL.md", @@ -286,6 +335,7 @@ def _run_claude( timeout: int, on_text: Callable[[str], None] | None = None, cwd: Path | str | None = None, + channel_id: str | None = None, ) -> dict: """Run a Claude CLI command and return parsed output. @@ -300,6 +350,10 @@ def _run_claude( *cwd* — optional working directory override (default: PROJECT_ROOT). Used by PlanningSession to scope the subprocess to ``~/workspace//`` so artifacts land in the target repo. + + *channel_id* — if given, registers the subprocess in `_live_procs` so + `/stop` can kill this turn. Callers that omit it (heartbeat, planning + sessions) simply aren't stoppable — unchanged behavior. """ if not shutil.which(CLAUDE_BIN): raise FileNotFoundError( @@ -316,6 +370,10 @@ def _run_claude( cwd=str(cwd) if cwd else PROJECT_ROOT, ) + if channel_id is not None: + with _live_procs_lock: + _live_procs[channel_id] = proc + # Watchdog thread: kill the process if it exceeds the timeout timed_out = threading.Event() @@ -332,40 +390,18 @@ def _run_claude( watchdog = threading.Thread(target=_watchdog, daemon=True) watchdog.start() - # --- Parse stream-json output line by line --- - text_blocks: list[str] = [] - result_obj: dict | None = None - intermediate_count = 0 - + # --- Parse stream-json output (T5: shared with ClaudeProcess via + # src.stream_json.consume_stream; see that module's docstring for its + # non-throwing contract — blocker C4. The raising below stays here. --- try: - for line in proc.stdout: - line = line.strip() - if not line: - continue - try: - obj = json.loads(line) - except json.JSONDecodeError: - continue - - msg_type = obj.get("type") - - if msg_type == "assistant": - message = obj.get("message", {}) - for block in message.get("content", []): - if block.get("type") == "text": - text = block.get("text", "").strip() - if text: - text_blocks.append(text) - if on_text: - try: - on_text(text) - intermediate_count += 1 - except Exception: - logger.exception("on_text callback error") - - elif msg_type == "result": - result_obj = obj + result = consume_stream(proc.stdout, on_text=on_text) finally: + # Deregister on every exit path (success, error, timeout) so a + # stale entry never outlives the process it points to. + if channel_id is not None: + with _live_procs_lock: + if _live_procs.get(channel_id) is proc: + del _live_procs[channel_id] # Ensure process resources are cleaned up proc.stdout.close() try: @@ -381,39 +417,24 @@ def _run_claude( raise TimeoutError(f"Claude CLI timed out after {timeout}s") if proc.returncode != 0: - stdout_tail = "\n".join(text_blocks[-3:]) if text_blocks else "" - # Check if result_obj has an error - result_error = "" - if result_obj and result_obj.get("is_error"): - result_error = result_obj.get("result", "") or result_obj.get("error", "") + stdout_tail = (result or {}).get("result", "") or "" + result_error = stdout_tail if (result and result.get("is_error")) else "" detail = stderr_output[:500] or result_error[:500] or stdout_tail[:500] logger.error("Claude CLI stderr: %s", stderr_output[:1000]) - logger.error("Claude CLI result_obj: %s", result_obj) + logger.error("Claude CLI result_obj: %s", result) raise RuntimeError( f"Claude CLI error (exit {proc.returncode}): {detail}" ) - if result_obj is None: + if result is None: raise RuntimeError( "Failed to parse Claude CLI output: no result line in stream" ) - combined_text = "\n\n".join(text_blocks) if text_blocks else result_obj.get("result", "") - - return { - "result": combined_text, - "session_id": result_obj.get("session_id", ""), - "usage": result_obj.get("usage", {}), - "total_cost_usd": result_obj.get("total_cost_usd", 0), - "cost_usd": result_obj.get("cost_usd", 0), - "duration_ms": result_obj.get("duration_ms", 0), - "num_turns": result_obj.get("num_turns", 0), - "intermediate_count": intermediate_count, - # Surface subtype/is_error for callers that retry on `error_max_turns` - # (PlanningSession does this — spike findings recommended retry strategy). - "subtype": result_obj.get("subtype", ""), - "is_error": bool(result_obj.get("is_error", False)), - } + # `consume_stream` already returns exactly the dict shape below + # (result/session_id/usage/total_cost_usd/cost_usd/duration_ms/num_turns/ + # intermediate_count/subtype/is_error) — nothing left to rebuild. + return result # --------------------------------------------------------------------------- @@ -497,7 +518,7 @@ def start_session( ] _t0 = time.monotonic() - data = _run_claude(cmd, timeout, on_text=on_text) + data = _run_claude(cmd, timeout, on_text=on_text, channel_id=channel_id) _elapsed_ms = int((time.monotonic() - _t0) * 1000) for field in ("result", "session_id"): @@ -543,11 +564,15 @@ def resume_session( message: str, timeout: int = DEFAULT_TIMEOUT, on_text: Callable[[str], None] | None = None, + channel_id: str | None = None, ) -> str: """Resume an existing Claude session by ID. Returns response text. If *on_text* is provided, each intermediate Claude text block is passed to the callback as soon as it arrives. + + *channel_id* — passed through to `_run_claude` so `/stop` can find this + turn's subprocess; optional (defaults to not stoppable). """ # Find channel/model for logging and model selection sessions = _load_sessions() @@ -571,7 +596,7 @@ def resume_session( ] _t0 = time.monotonic() - data = _run_claude(cmd, timeout, on_text=on_text) + data = _run_claude(cmd, timeout, on_text=on_text, channel_id=channel_id) _elapsed_ms = int((time.monotonic() - _t0) * 1000) if not data.get("result"): @@ -608,6 +633,268 @@ def resume_session( return response_text +# --------------------------------------------------------------------------- +# Steering dispatcher (H1/C2/C3/M1) — see tasks/steering-plan.md. +# +# `claude_runner` imports FROM this module (parser, build_system_prompt, +# _safe_env) — importing it back at module level here would be a cycle, so +# every reference below is a lazy `from src import claude_runner` inside a +# function body, as the plan requires. +# --------------------------------------------------------------------------- + +# C2: which adapter owns the turn currently dispatched on a channel. Voice +# and text share `channel_id` as session_key (router.py's `session_key = +# channel_id`) — without this a text message could steer into a live VOICE +# turn and its reply would be spoken aloud, never rendered as text. Plain +# dict; a stale/racy read only affects the rare tie-break in the tiny +# window between acquiring the lock and recording ownership below, which is +# dwarfed by real subprocess/IO latency. +_channel_adapter: dict[str, str] = {} + + +# Same three namespaces router.py's channel config lives in (Discord, +# Telegram, WhatsApp) — checked in order so a per-channel `steering` key +# works regardless of which adapter the channel_id came from. +_CHANNEL_NAMESPACES = ("channels", "telegram_channels", "whatsapp_channels") + + +def _channel_steering_override(config, channel_id: str) -> bool | None: + """Etapa 6 (X16): `channels..steering` (or its telegram/whatsapp + twins), matched by the channel's `id` field — same lookup router.py's + `_get_channel_config` uses. None if unset, so the caller falls through + to the global `steering.enabled` default.""" + for namespace in _CHANNEL_NAMESPACES: + for ch in (config.get(namespace, {}) or {}).values(): + if ch.get("id") == channel_id and "steering" in ch: + return bool(ch["steering"]) + return None + + +def _steering_config(channel_id: str | None = None) -> tuple[bool, int, int]: + """Read `steering.*` through `Config()` on EVERY call — never at import + time. A module-level read (like `ALLOWED_TOOLS` above) would mean the + flag can't be flipped without a restart, which defeats the point of a + rollback flag. `ECHO_STEERING=off` (kill switch, precedent: `CLAUDE_BIN`) + always wins over config.json. Returns (enabled, max_live, idle_minutes) + with the clamps already applied: `max_live <= 0` forces disabled; + `idle_minutes < 1` clamps to 1. + + *channel_id* — if given, a per-channel `steering` override (see + `_channel_steering_override`) wins over the global `enabled` flag for + that one channel — the staged rollout the plan's Etapa 6 needs ("flip + on one channel, then global"). Unset on the channel -> global default. + """ + if os.environ.get("ECHO_STEERING", "").strip().lower() == "off": + logger.debug("steering: disabled by ECHO_STEERING=off kill switch") + return False, 0, 20 + try: + from src.config import Config + full_cfg = Config() + cfg = full_cfg.get("steering", {}) or {} + except (OSError, ValueError): + logger.exception("steering: failed to read config.json — treating as disabled") + return False, 0, 20 + enabled = bool(cfg.get("enabled", False)) + if channel_id is not None: + override = _channel_steering_override(full_cfg, channel_id) + if override is not None: + logger.debug("channel=%s: per-channel steering override=%s", channel_id, override) + enabled = override + max_live = cfg.get("max_live", 2) + if not isinstance(max_live, int): + max_live = 0 + idle_minutes = cfg.get("idle_minutes", 20) + if not isinstance(idle_minutes, int) or idle_minutes < 1: + idle_minutes = 1 + if max_live <= 0 and enabled: + logger.warning("steering: max_live=%r <= 0 — disabling steering", max_live) + enabled = False + if not enabled: + logger.debug("steering: disabled by config") + return enabled, max_live, idle_minutes + + +def _persist_steering_session(channel_id: str, model: str, data: dict) -> None: + """Persist sessions/active.json after a steering-dispatched turn — same + fields/update semantics `start_session`/`resume_session` already use, so + `/status`, `/model`, and a later one-shot fallback all keep working.""" + usage = data.get("usage", {}) + now = datetime.now(timezone.utc).isoformat() + sessions = _load_sessions() + existing = sessions.get(channel_id) + if existing is None: + sessions[channel_id] = { + "session_id": data.get("session_id", ""), + "model": model, + "created_at": now, + "last_message_at": now, + "message_count": 1, + "total_input_tokens": usage.get("input_tokens", 0), + "total_output_tokens": usage.get("output_tokens", 0), + "total_cost_usd": data.get("total_cost_usd", 0), + "duration_ms": data.get("duration_ms", 0), + "context_tokens": usage.get("input_tokens", 0) + usage.get("output_tokens", 0), + } + else: + existing["session_id"] = data.get("session_id") or existing.get("session_id", "") + existing["model"] = model + existing["last_message_at"] = now + existing["message_count"] = existing.get("message_count", 0) + 1 + existing["total_input_tokens"] = existing.get("total_input_tokens", 0) + usage.get("input_tokens", 0) + existing["total_output_tokens"] = existing.get("total_output_tokens", 0) + usage.get("output_tokens", 0) + existing["total_cost_usd"] = existing.get("total_cost_usd", 0) + data.get("total_cost_usd", 0) + existing["duration_ms"] = existing.get("duration_ms", 0) + data.get("duration_ms", 0) + existing["context_tokens"] = usage.get("input_tokens", 0) + usage.get("output_tokens", 0) + _save_sessions(sessions) + _invoke_log.info( + "channel=%s model=%s steered=1 session=%s", + channel_id, model, sessions[channel_id]["session_id"][:8], + ) + + +def _run_steering_turn( + proc, lock: threading.Lock, channel_id: str, message: str, model: str, + timeout: int, on_text: Callable[[str], None] | None, adapter_name: str, +) -> str: + """Run *message* as a fresh turn on the already-acquired *lock*/*proc*. + + Releases the lock on every exit path. On failure, any messages that got + steered into this same turn before it failed are left in + `proc._pending_steers` for `pop_pending_steers()` (C3) — router.py picks + them up from its exception handler and re-dispatches them, since their + own request threads already returned `__STEERED__` and are gone. + """ + _channel_adapter[channel_id] = adapter_name + try: + data = proc.run_turn(message, on_text=on_text, timeout=timeout) + except Exception: + lock.release() + raise + lock.release() + _persist_steering_session(channel_id, model, data) + return data["result"] + + +def _dispatch_steering( + channel_id: str, + message: str, + model: str, + timeout: int, + on_text: Callable[[str], None] | None, + adapter_name: str | None, + max_live: int, + idle_minutes: int, +) -> str | None: + """Try to steer *message* into a live process for *channel_id*, or run + it as a fresh turn. Returns the response text, the `__STEERED__` + sentinel, or None to tell `send_message` to fall back to the unchanged + one-shot path (T10: registry is at capacity and this channel has no + live process of its own — the only case where that path is safe, see + M1 below). + + T12: every branch logs why a message did or did not become a steer. + """ + from src import claude_runner + + registry = claude_runner.get_registry(max_live=max_live, idle_minutes=idle_minutes) + session = get_active_session(channel_id) + session_id = session.get("session_id") if session else None + effective_model = model + if session is not None and session.get("model"): + effective_model = session["model"] + + proc = registry.get(channel_id, model=effective_model, session_id=session_id) + if proc is None: + # M1: safe only because a channel that ALREADY has a live process + # is always returned as-is by `registry.get()`, regardless of + # capacity — so we only ever land here for a channel with no live + # process, meaning the one-shot fallback below is the sole writer. + logger.info( + "channel=%s: steering registry at capacity — degrading to one-shot (T10)", + channel_id, + ) + return None + + lock = _get_session_lock(channel_id) + caller_adapter = adapter_name or "echo" + + if lock.acquire(blocking=False): + # H1: acquiring the lock IS "no turn is running" — no separate + # `inflight` flag/check, no TOCTOU between them. + logger.debug("channel=%s: lock free — running as a full turn (not a steer)", channel_id) + return _run_steering_turn( + proc, lock, channel_id, message, effective_model, timeout, on_text, caller_adapter, + ) + + owner = _channel_adapter.get(channel_id) + if owner is not None and owner != caller_adapter: + # C2: never steer a message from a different adapter into someone + # else's in-flight turn (a text message hijacking a live voice + # turn, or vice versa — they share `channel_id`). M1 rules out + # falling back to the one-shot `resume_session` path here too: this + # channel already has a live process holding the session, and a + # concurrent one-shot `--resume` would be a second writer on the + # same session_id. So: wait for the current turn's lock like today, + # then take our turn through the SAME live process. + logger.info( + "channel=%s: turn owned by adapter=%s, message from adapter=%s — " + "waiting instead of steering (C2), staying on steering process (M1)", + channel_id, owner, caller_adapter, + ) + lock.acquire(blocking=True) + return _run_steering_turn( + proc, lock, channel_id, message, effective_model, timeout, on_text, caller_adapter, + ) + + result = proc.steer(message) + if result.status is claude_runner.SteerStatus.STEERED: + logger.info("channel=%s: steered %d chars (adapter=%s)", + channel_id, len(message), caller_adapter) + from src.sentinels import STEERED + return STEERED + + # RAN_AS_TURN / PROCESS_DEAD: `result.turn` IS the response for this + # text (D4/T7) — never re-run it, that would double-send and double-bill. + logger.info("channel=%s: steer() resolved as %s — using its turn result directly", + channel_id, result.status.value) + _persist_steering_session(channel_id, effective_model, result.turn) + return result.turn["result"] + + +def pop_pending_steers(channel_id: str) -> list[str]: + """C3 hook for router.py: texts steered into a turn on *channel_id* + that then failed (rate limit, timeout, crash) — the caller MUST + re-dispatch them, never drop them (CLAUDE.md: "turnul nu se pierde + niciodată"). Empty list if steering was never used on this process + (never spins up the registry as a side effect — that would be wrong + for a channel that never used steering) or the channel has no live + process, or nothing was pending. + """ + from src import claude_runner + + registry = claude_runner._registry + if registry is None: + return [] + proc = registry._procs.get(channel_id) # peek only — .get() would spawn a new one + if proc is None: + return [] + return proc.pop_pending_steers() + + +def _stop_steering_process(channel_id: str) -> None: + """T9: `/clear` and `/model` must not leave a live steering process + running with stale session/model config — it respawns on the next + turn (via `_respawn_if_needed`/`--resume`), picking up the new state. + No-op if steering was never used (doesn't spin up the registry).""" + from src import claude_runner + + registry = claude_runner._registry + if registry is None: + return + if registry.stop(channel_id): + logger.info("channel=%s: stopped live steering process (T9)", channel_id) + + def send_message( channel_id: str, message: str, @@ -615,6 +902,7 @@ def send_message( timeout: int = DEFAULT_TIMEOUT, on_text: Callable[[str], None] | None = None, voice_mode: bool = False, + adapter_name: str | None = None, ) -> str: """High-level convenience: auto start or resume based on channel state. @@ -624,12 +912,35 @@ def send_message( parallel — each holds its own lock. Lock is acquired blocking; we rely on `timeout` (default 5 minutes) to bound the worst case rather than a non-blocking acquire (loss of fairness vs adapter-side queueing). + + *adapter_name* — steering dispatcher only (C2). Identifies which + adapter's turn is in flight so a message from a DIFFERENT adapter on + the same channel_id never gets steered into it. Ignored entirely when + steering is off — this whole path is then bit-for-bit what it always + was. + + When `steering.enabled` is on (config.json, per-call — see + `_steering_config`), this dispatches through `_dispatch_steering` + first: it returns a response, the `__STEERED__` sentinel, or None to + fall through to the unchanged path below. """ + enabled, max_live, idle_minutes = _steering_config(channel_id) + if enabled: + dispatched = _dispatch_steering( + channel_id, message, model, timeout, on_text, adapter_name, + max_live, idle_minutes, + ) + if dispatched is not None: + return dispatched + with _get_session_lock(channel_id): session = get_active_session(channel_id) # Only resume if session has a valid session_id (not a pre-set model placeholder) if session is not None and session.get("session_id"): - return resume_session(session["session_id"], message, timeout, on_text=on_text) + return resume_session( + session["session_id"], message, timeout, + on_text=on_text, channel_id=channel_id, + ) # Use model from pre-set session if available, otherwise use provided model effective_model = model if session is not None and session.get("model"): @@ -643,6 +954,7 @@ def send_message( def clear_session(channel_id: str) -> bool: """Remove a channel's session entry. Returns True if removed.""" + _stop_steering_process(channel_id) # T9 sessions = _load_sessions() if channel_id not in sessions: return False @@ -663,6 +975,7 @@ def set_session_model(channel_id: str, model: str) -> bool: raise ValueError( f"Invalid model '{model}'. Must be one of: {', '.join(sorted(VALID_MODELS))}" ) + _stop_steering_process(channel_id) # T9 sessions = _load_sessions() if channel_id not in sessions: return False diff --git a/src/main.py b/src/main.py index 6af7fa1..ecdd43e 100644 --- a/src/main.py +++ b/src/main.py @@ -21,6 +21,21 @@ PID_FILE = PROJECT_ROOT / "echo-core.pid" LOG_DIR = PROJECT_ROOT / "logs" +def _stop_steering_registry(logger: logging.Logger) -> None: + """T4: kill any live persistent `claude` processes before the process + exits. `RunnerRegistry.stop_all()` is already `atexit`-registered, but + `atexit` does NOT fire on SIGTERM — exactly how `systemctl --user + restart echo-core` stops this process — so it must also be called + explicitly from the shutdown path. A leaked live process is 292-541 MB + RSS on an 8 GB host. Imported lazily so a system without steering wired + up yet (or any import error in claude_runner) can't break shutdown.""" + try: + from src.claude_runner import get_registry + get_registry().stop_all() + except Exception: + logger.exception("Error stopping claude_runner registry during shutdown") + + def setup_logging(): LOG_DIR.mkdir(parents=True, exist_ok=True) fmt = "%(asctime)s [%(levelname)s] %(name)s: %(message)s" @@ -148,6 +163,7 @@ def main(): loop.run_until_complete(scheduler.stop()) loop.run_until_complete(client.close()) finally: + _stop_steering_registry(logger) PID_FILE.unlink(missing_ok=True) logger.info("Echo Core shut down.") diff --git a/src/router.py b/src/router.py index 88eeb1d..47c27da 100644 --- a/src/router.py +++ b/src/router.py @@ -24,7 +24,10 @@ from src.claude_session import ( rate_limit_detail as _rate_limit_detail, RATE_LIMIT_RE as _RATE_LIMIT_RE, VALID_MODELS, + stop_turn, + pop_pending_steers as _pop_pending_steers, ) +from src.sentinels import is_steered as _is_steered from src.jsonlock import read_locked, write_locked from src.planning_orchestrator import PlanningOrchestrator from src.planning_session import ( @@ -566,6 +569,11 @@ def route_message( if text.lower() == "/status": return _status(channel_id), True + if text.lower() == "/stop": + if stop_turn(channel_id): + return "⏹ Oprit.", True + return "Nu rulează nimic pe canalul ăsta.", True + if text.lower().startswith("/model"): return _model_command(channel_id, text), True @@ -602,15 +610,29 @@ def route_message( try: response = send_message( session_key, claude_text, model=model, on_text=on_text, - voice_mode=voice_mode, + voice_mode=voice_mode, adapter_name=adapter_name, ) + if _is_steered(response): + # Same pattern as the existing __AUDIO__: sentinel — no + # _set_last_response, the adapter reacts instead of replying. + return response, False _set_last_response(channel_id, response) return response, False except Exception as e: log.error("Claude error for channel %s: %s", channel_id, e) + # C3: texts steered into this same turn before it failed — their own + # request threads already returned __STEERED__ and are gone, so the + # only way left to answer them is `on_text`, the same real-time + # channel already used for intermediate assistant text. + pending_steers = _pop_pending_steers(channel_id) if _is_rate_limit_error(e): log.warning("Rate limit detected for channel %s — trying local fallback", channel_id) fallback = _local_fallback_reply(text, channel_id=channel_id) + for steered_text in pending_steers: + _redeliver_steered_reply( + steered_text, channel_id, on_text, + _local_fallback_reply(steered_text, channel_id=channel_id), + ) if fallback is not None: _set_last_response(channel_id, fallback) return fallback, False @@ -622,9 +644,37 @@ def route_message( "⚠️ Claude e la limită, iar modelul local nu a răspuns.\n" f"{_rate_limit_detail(e)}" ), False + for steered_text in pending_steers: + _redeliver_steered_reply(steered_text, channel_id, on_text, f"Error: {e}") return f"Error: {e}", False +def _redeliver_steered_reply( + steered_text: str, + channel_id: str, + on_text: Callable[[str], None] | None, + reply: str | None, +) -> None: + """C3 — a message steered into a turn that then failed must still get + an answer. Its own request thread already returned `__STEERED__` and is + gone, so the only way left to reach the user is `on_text` (the same + real-time channel adapters already use for intermediate assistant + text). Logs instead of dropping silently when there's no `on_text` to + push through (T12 — "it ignored my message" must stay diagnosable).""" + if reply is None: + reply = "⚠️ Claude e la limită — mesajul tău steered nu a primit răspuns." + if on_text is None: + log.warning( + "channel=%s: steered message lost — no on_text to redeliver it: %r", + channel_id, steered_text[:80], + ) + return + try: + on_text(reply) + except Exception: + log.exception("channel=%s: failed to redeliver steered reply via on_text", channel_id) + + def _status(channel_id: str) -> str: """Build status message for a channel.""" session = get_active_session(channel_id) diff --git a/src/sentinels.py b/src/sentinels.py new file mode 100644 index 0000000..bd0077a --- /dev/null +++ b/src/sentinels.py @@ -0,0 +1,40 @@ +"""Single place to recognise router sentinels. + +``route_message()`` normally returns text to post back to the user. Two +special values are protocol instead of a reply, and every adapter must +catch them before sending anything: + +- ``__AUDIO__:`` — a TTS result; the payload is a WAV path to attach. +- ``__STEERED__`` — the message was injected into an in-flight turn; the + adapter should react (not reply) and post nothing. + +Zero dependencies on adapters or router — importable standalone. +""" +from __future__ import annotations + +AUDIO_PREFIX = "__AUDIO__:" +STEERED = "__STEERED__" + + +def audio_path(response: str | None) -> str | None: + """Return the WAV path if `response` is an audio sentinel, else None.""" + if response and response.startswith(AUDIO_PREFIX): + return response[len(AUDIO_PREFIX):] + return None + + +def is_steered(response: str | None) -> bool: + """True if `response` is the steering-acknowledgement sentinel.""" + return response == STEERED + + +if __name__ == "__main__": + assert audio_path("__AUDIO__:/tmp/x.wav") == "/tmp/x.wav" + assert audio_path("hello") is None + assert audio_path("") is None + assert audio_path(None) is None + assert is_steered("__STEERED__") is True + assert is_steered("__STEERED__ ") is False + assert is_steered("") is False + assert is_steered(None) is False + print("ok") diff --git a/src/stream_json.py b/src/stream_json.py new file mode 100644 index 0000000..477be0a --- /dev/null +++ b/src/stream_json.py @@ -0,0 +1,153 @@ +"""Pure stream-json parsing, shared by the one-shot (`_run_claude`) and +persistent (`ClaudeProcess`) read loops. + +Leaf module: imports nothing from `claude_session.py` or `claude_runner.py`, +so neither can create an import cycle through it. + +Contract (blocker C4 in tasks/steering-plan.md): this module NEVER raises on +`is_error` / `subtype`. It only parses stream-json events into the dict shape +`_run_claude` has always returned. `PlanningSession` depends on `_run_claude` +returning an error *result* rather than throwing (it retries on +`error_max_turns` — see `planning_session.py`'s `RETRY_MAX_TURNS`); any +future caller that wants to raise on a particular error (e.g. rate limits on +a persistent process, done in `claude_runner.ClaudeProcess.run_turn`) must +do so itself, on top of the dict this module returns. +""" + +import json +import logging +from typing import Callable, Iterable + +logger = logging.getLogger(__name__) + + +def parse_event(line: str) -> dict | None: + """Parse one stream-json line into an event dict, or None if blank/invalid.""" + line = line.strip() + if not line: + return None + try: + return json.loads(line) + except json.JSONDecodeError: + return None + + +def is_init_event(event: dict) -> bool: + """True for the `system`/`init` event a fresh Claude CLI process emits + once, at the start of its very first turn, carrying `session_id` before + any assistant text or the turn's `result` line exists. + + Needed for `ClaudeProcess`: if a turn times out before its `result` + ever arrives, the session_id captured here is the only way the *next* + turn can `--resume` the same conversation (see plan finding M2) — the + one-shot path doesn't need this, since it always gets `session_id` off + the `result` line of a turn that, by definition, already completed. + """ + return event.get("type") == "system" and event.get("subtype") == "init" + + +def consume_stream( + lines: Iterable[str], + on_text: Callable[[str], None] | None = None, + on_init: Callable[[str], None] | None = None, +) -> dict | None: + """Read stream-json events from *lines* until (and including) a `result` + event, then stop — never read past it. + + Stopping exactly at the turn boundary is what lets a persistent process + reuse this for one turn at a time: reading further would eat the *next* + turn's events off the same stdout. + + *on_text* is called with each intermediate assistant text block as soon + as it arrives (same as `_run_claude` today). *on_init* is called once, + with the session_id, when a `system`/`init` event is seen (may never + fire — the one-shot caller doesn't need it and passes None). + + Returns the same dict `_run_claude` has always returned, or None if the + stream ended (EOF) without ever producing a `result` line — mirroring + `_run_claude`'s own `if result_obj is None: raise RuntimeError(...)` + check. Raising on that is the CALLER's job, not this function's (C4). + """ + text_blocks: list[str] = [] + result_obj: dict | None = None + intermediate_count = 0 + + for raw_line in lines: + event = parse_event(raw_line) + if event is None: + continue + + msg_type = event.get("type") + + if msg_type == "assistant": + message = event.get("message", {}) + for block in message.get("content", []): + if block.get("type") == "text": + text = block.get("text", "").strip() + if text: + text_blocks.append(text) + if on_text: + try: + on_text(text) + intermediate_count += 1 + except Exception: + logger.exception("on_text callback error") + + elif on_init is not None and is_init_event(event): + session_id = event.get("session_id") + if session_id: + try: + on_init(session_id) + except Exception: + logger.exception("on_init callback error") + + elif msg_type == "result": + result_obj = event + break # never read past the turn boundary + + if result_obj is None: + return None + + combined_text = "\n\n".join(text_blocks) if text_blocks else result_obj.get("result", "") + + return { + "result": combined_text, + "session_id": result_obj.get("session_id", ""), + "usage": result_obj.get("usage", {}), + "total_cost_usd": result_obj.get("total_cost_usd", 0), + "cost_usd": result_obj.get("cost_usd", 0), + "duration_ms": result_obj.get("duration_ms", 0), + "num_turns": result_obj.get("num_turns", 0), + "intermediate_count": intermediate_count, + "subtype": result_obj.get("subtype", ""), + "is_error": bool(result_obj.get("is_error", False)), + } + + +def demo() -> None: + """Smallest runnable self-check (ponytail: no test framework needed).""" + lines = [ + json.dumps({"type": "system", "subtype": "init", "session_id": "s1"}), + json.dumps({"type": "assistant", "message": {"content": [{"type": "text", "text": "hi"}]}}), + json.dumps({"type": "result", "session_id": "s1", "result": "hi", "is_error": False}), + json.dumps({"type": "assistant", "message": {"content": [{"type": "text", "text": "SHOULD NOT APPEAR"}]}}), + ] + seen_init = [] + out = consume_stream(lines, on_init=seen_init.append) + assert out is not None + assert out["result"] == "hi" + assert out["session_id"] == "s1" + assert out["is_error"] is False + assert seen_init == ["s1"] + + # No result line at all -> None, does not raise. + assert consume_stream([json.dumps({"type": "assistant", "message": {"content": []}})]) is None + + # Blank/garbage lines are skipped, not fatal. + assert consume_stream(["", "not json", json.dumps({"type": "result", "result": "ok"})])["result"] == "ok" + + print("stream_json self-check OK") + + +if __name__ == "__main__": + demo() diff --git a/src/voice/pipeline.py b/src/voice/pipeline.py index 2cb3559..5c9045c 100644 --- a/src/voice/pipeline.py +++ b/src/voice/pipeline.py @@ -34,6 +34,7 @@ from typing import Any, Callable, Optional import numpy as np from src.router import _strip_leading_voice_tokens +from src.sentinels import is_steered from src.voice._discord_voice_adapter import AudioSink, VoiceData from src.voice.voice_commands import detect_voice_change @@ -434,7 +435,7 @@ class VoiceSession: voice_stream_callback, # on_text "discord-voice", # adapter_name ) - if isinstance(result, tuple) and result: + if isinstance(result, tuple) and result and not is_steered(result[0]): response_text = _strip_leading_voice_tokens(result[0] or "") except Exception as e: # noqa: BLE001 log.error("route_message voice path failed: %s", e) diff --git a/tasks/steering-plan.md b/tasks/steering-plan.md new file mode 100644 index 0000000..c457b5b --- /dev/null +++ b/tasks/steering-plan.md @@ -0,0 +1,1375 @@ + +# Plan — Steering (mesaje mid-tur) în echo-core + +**Status:** draft, intrat în /autoplan +**Branch:** master +**Data:** 2026-09-02 + +## Problema + +Când Echo Core execută un mesaj lung (tool calls, subagenți), un al doilea mesaj trimis +de Marius pe același canal **nu ajunge la Claude până când primul tur se termină**. +Concret: îi scrii „stai, nu în master" în timp ce lucrează, iar Claude vede corecția +abia după ce a terminat ce făcea greșit. + +Cauza, în cod: +- `src/claude_session.py:310` — `subprocess.Popen` **one-shot per mesaj**, promptul + trecut ca **argv** (`-p ""`), fără stdin. Procesul moare la finalul turului. +- `src/claude_session.py:628` — `send_message()` ia un `threading.Lock` per canal. + Mesajul 2 **blochează** în lock până se termină turul 1, apoi pleacă ca tur separat + prin `--resume`. +- `src/adapters/discord_bot.py:1296` — fiecare mesaj pornește propriul + `asyncio.to_thread(route_message, ...)`, deci mesajul 2 chiar ajunge în `send_message`, + dar așteaptă acolo. + +## Referința — agentul din LXC 171 + +`romfastsql/proxmox/lxc171-claude-agent/discord-bridge/` implementează exact asta, +sub numele **steering**. **Codul rulează pe LXC 171, nu în clona locală** — clona din +`~/workspace/romfastsql/` e stale și nu conține `discord-bridge/`. Acces: +`ssh echo@10.0.20.201 "sudo pct exec 171 -- cat /workspace/romfastsql/proxmox/lxc171-claude-agent/discord-bridge/runner.py"`: + +- `runner.py` — proces `claude` **persistent per fir**, pornit cu + `-p --input-format stream-json --output-format stream-json --verbose`, + cu **stdin deschis**. Un tur = scrii un JSON `{"type":"user",...}` pe stdin, citești + evenimente de pe stdout până la `result`. +- `bot.py:652-663` — dacă vine un mesaj cât `proc.inflight` e True, **nu deschide tur + nou**: îl scrie pe același stdin, reacționează cu ➡️, `return "steered"`. +- `session_id` luat din evenimentul `system`/`init` → `--resume` la respawn. +- Reaper de procese inactive (20 min), timeout per tur, buffer circular de stderr. + +## Verificare empirică (făcută înainte de plan) + +CLI-ul local (claude 2.1.258) suportă `--input-format stream-json`. + +Spike rulat pe `/tmp`: un tur pornit cu `sleep 12 && echo pas1` în prim-plan (3 pași +secvențiali ceruți), apoi la 15.0s s-a scris pe stdin „STOP, schimbare de plan […] +răspunde-mi doar cu cuvântul ANANAS". + +Rezultat: la 17.8s Claude a răspuns `ANANAS`, a abandonat pașii 2 și 3, și turul s-a +închis cu **un singur** `result` (`num_turns=2`). Premisa centrală a planului e +confirmată: mesajul mid-tur ajunge la model, e luat în seamă, iar turul nu se dublează. + +Un prim spike a fost neconcludent (Claude a trecut `sleep` în `run_in_background`, +turul s-a terminat în 7.8s înainte să conteze steering-ul) — de reținut ca notă de test. + +## Decizii deja luate cu utilizatorul + +| Întrebare | Alegere | +|---|---| +| Arhitectură | Proces persistent + steering real (nu coadă/coalescing) | +| Adaptoare | Toate trei deodată (Discord, Telegram, WhatsApp) | +| Feedback UX | Confirmare discretă — reacție ➡️, răspuns unul singur la final | +| Scheduler / heartbeat / planning | Rămân one-shot, neatinse | + +## Design + +### 1. `src/claude_runner.py` (nou) + +Thread-based, nu asyncio. Motivul: adaptoarele cheamă `route_message` prin +`asyncio.to_thread`, deci `send_message` e cod sync; un al doilea event loop în procesul +care deja rulează `asyncio.gather()` în `main.py` ar fi complexitate gratuită. + +**`ClaudeProcess`** — un proces viu per `channel_id`: +- spawn cu `-p --input-format stream-json --output-format stream-json --verbose + --system-prompt --dangerously-skip-permissions [--resume ]`, + `stdin=PIPE` ținut deschis +- **thread separat care drenează stderr** într-un `deque(maxlen=50)` — obligatoriu, + altfel pipe-ul se umple și procesul blochează la mijlocul unui tur +- `run_turn()` întoarce **exact dict-ul pe care îl întoarce azi `_run_claude`** + (`result`, `session_id`, `usage`, `total_cost_usd`, `duration_ms`, `subtype`, + `is_error`) → restul codului nu se atinge +- `steer(text)` — scrie pe stdin sub un `_stdin_lock`, fără să deschidă tur +- `session_id` capturat din evenimentul `system`/`init` → `--resume` la respawn + +**`RunnerRegistry`** — reaper thread care oprește procesele inactive +(`steering.idle_minutes`, implicit 20 — vezi X3), niciodată unul `inflight`; plafon +`steering.max_live` (implicit 4) cu stop LRU. + +### 2. `send_message()` devine dispecer + +``` +if steering activ: + proc = registry.get(channel_id) + if proc.alive and proc.inflight: → proc.steer(text); return "__STEERED__" + with lock(channel_id): → proc.run_turn(...) + persist în sessions/active.json +else: + calea actuală, neatinsă +``` + +`_run_claude`, `start_session`, `resume_session` rămân **neatinse** — le folosesc +`heartbeat.py` și `planning_session.py`. + +**Cursă cunoscută:** dacă turul se închide între verificarea `inflight` și scrierea pe +stdin, linia devine automat turul următor (procesul rămâne viu — verificat în spike). +Nu se pierde mesajul, dar adaptorul ar fi răspuns deja doar cu ➡️. Tratare: `steer()` +care constată post-factum `not inflight` cade înapoi pe tur normal. + +### 3. Router + adaptoare + +Sentinela `__STEERED__` propagată prin `route_message` fără `_set_last_response` — +același tipar cu `__AUDIO__:` deja existent în `discord_bot.py:1305`. +Discord: `add_reaction("➡️")`. Telegram: `set_message_reaction`. +WhatsApp: verific întâi dacă bridge-ul Baileys expune reacții; dacă nu, un „➡️" scurt. + +## Riscuri identificate + +1. **Rate limit — cel mai subtil.** Detecția de azi (`is_rate_limit_error`) prinde un + `RuntimeError` construit din exit code ≠ 0. Un proces persistent nu moare la limită — + apare ca `result.is_error` în stream. Netratat, **fallback-ul local nu se mai + declanșează deloc** — adică exact munca cea mai grea din repo devine moartă în tăcere. +2. ~~**`--system-prompt` se fixează la spawn.**~~ **ELIMINAT — premisă inversată.** + Verificat: `resume_session` (`claude_session.py:565-571`) **nu pasează deloc** `--system-prompt`; + `build_system_prompt()` e chemat doar la `:486` (`start_session`) și `scheduler.py:402`. + Editările din `personality/` **deja** nu se aplică mid-sesiune, ci doar după `/clear`. + Fix-ul propus (hash → respawn) ar fi fost comportament NOU care omoară procesul viu și + steering-urile în așteptare la fiecare editare de `SOUL.md`. Vezi § Corecții factuale. +3. **`set_session_model`** trebuie să oprească procesul viu; turul următor îl repornește + cu `--resume` (ca `set_options` din bridge). +4. **RAM** — fiecare proces `claude` viu costă. De aici plafonul `max_live`. + +## Etape + +| # | Ce | Verificare | +|---|-----|-----------| +| 1 | `claude_runner.py` + fake `claude` stream-json + `tests/test_claude_runner.py` | tur normal, steering mid-tur, respawn `--resume` după kill, reaper, timeout, rate-limit în stream — toate offline | +| 2 | Dispecerul în `send_message` + sentinela în router, în spatele `config.json → steering.enabled` (**implicit off**) | suita existentă trece nemodificată cu flagul off | +| 3 | Reacții pe cele 3 adaptoare | manual, un canal de test | +| 4 | Rate-limit din stream + respawn pe personality/model schimbat | test dedicat că fallback-ul local încă pornește | +| 5 | Reaper + `max_live` + procesele vii vizibile în `eco status` | | +| 6 | Flip pe un canal, apoi global | | + +Flagul e implicit off la fiecare pas, deci rollback-ul e o linie în `config.json`. + +--- + +# /autoplan — Phase 1: CEO Review + +Mod: **SELECTIVE EXPANSION** (feature enhancement pe sistem existent — default-ul contextual). +Voci: `[subagent-only]` — Codex nu e instalat pe această mașină. + +## 0A. Premise Challenge + +| # | Premisă | Verdict | Dovadă | +|---|---|---|---| +| P1 | Mesajul 2 nu ajunge la Claude până se termină turul 1 | **CONFIRMAT** | `claude_session.py:628` lock blocant; adaptoarele pornesc `to_thread` separat (`discord_bot.py:1296`) | +| P2 | Steering prin stdin funcționează pe CLI-ul instalat | **CONFIRMAT** | spike: `ANANAS` la 17.8s, un singur `result`, `num_turns=2` | +| P3 | Doar `router.py` cheamă `send_message` → cron/heartbeat/planning sunt izolate | **CONFIRMAT** | `heartbeat.py:416` folosește `_run_claude_extra`; `planning_session.py:264` folosește `_run_claude` | +| P4 | „Răspunsul rămâne unul singur, la finalul turului" | **FALS** | echo-core **deja** trimite fiecare bloc intermediar pe canal prin `on_text` (`claude_session.py:355`, `discord_bot.py:1283`, `telegram_bot.py:1020`, `whatsapp.py:217`). Bridge-ul editează un placeholder; echo-core trimite mesaje noi. UX-ul ales de utilizator nu există azi. | +| P5 | „WhatsApp poate să nu aibă reacții" | **FALS** | `react_whatsapp()` există și e deja folosit (👀 pus/scos pe fiecare mesaj, `whatsapp.py:243`). Telegram folosește deja `set_message_reaction` (✅). Toate trei adaptoarele au reacții native. | +| P6 | Un proces persistent cu `--dangerously-skip-permissions` are același profil de risc ca unul one-shot | **NEEXAMINAT — fals** | Azi procesul moare la finalul turului. Persistent = shell cu acces total la unelte, viu până la 20 min după ultimul mesaj. Raza de acțiune a unei injecții de prompt persistă între tururi. | +| P7 | Contextul se gestionează singur | **LACUNĂ** | Bridge-ul pasează explicit `--autocompact auto`. Planul nu-l menționează. Un proces care ține N tururi crește monoton până lovește limita de context. | + +**Premise queued pentru Final Gate:** P4 (UX-ul ales nu descrie comportamentul real) — vezi Gate. + +## 0B. Existing Code Leverage + +| Sub-problemă | Cod existent | Reutilizat de plan? | +|---|---|---| +| Parsare stream-json | `_run_claude` `claude_session.py:335-370` | **NU — plan rescrie.** Violare DRY. Trebuie extras parser comun. | +| Serializare per canal | `_get_session_lock` `:84` | Da | +| Persistare sesiune | `_load_sessions` / `_save_sessions` `:253` | Da | +| Detecție rate limit | `is_rate_limit_error` / `rate_limit_detail` `:49` | Parțial — nu acoperă `result.is_error` | +| Sentinelă în răspuns | `__AUDIO__:` `discord_bot.py:1305` | Da, precedent bun | +| Reacții pe mesaj | Discord `add_reaction`, Telegram `set_message_reaction`, WhatsApp `react_whatsapp` | Da — **toate trei există deja** | +| Env sigur pentru subprocess | `_safe_env()` `:231` | Da | +| System prompt | `build_system_prompt()` `:424` | Da | +| Loop persistent + steering + interrupt | **`claude-agent-sdk` v0.2.151** (`ClaudeSDKClient.query()` / `.interrupt()`) | **NU — plan reimplementează.** Vezi 0C-bis. | + +## 0C. Dream State + +``` + CURRENT STATE THIS PLAN 12-MONTH IDEAL + one-shot per mesaj ---> proces persistent ---> un strat unic de sesiune + prompt în argv stdin deschis partajat de chat + voice + Ralph: + lock → mesajul 2 așteaptă steering mid-tur steering, interrupt/cancel, + fără cancel fără cancel atașamente mid-tur, progres live, + fără compactare fără compactare compactare + rate limit într-un loc +``` + +**Delta:** planul ajunge ~60% spre ideal. Lipsesc: **cancel/interrupt** (a opri un tur scăpat de sub control — cea mai cerută capabilitate vecină), decizia voice-vs-text pe același canal, compactarea. + +## 0C-bis. Implementation Alternatives + +``` +APPROACH A: Port hand-rolled al bridge-ului (planul actual) + Summary: ClaudeProcess + RunnerRegistry thread-based, stdin deschis, parser stream-json propriu. + Effort: M (human ~3 zile / CC ~2-3h) + Risk: Med + Pros: - Zero dependențe noi; controlezi fiecare linie + - Rămâne sync — `send_message` și `route_message` nu se schimbă + - Arhitectură deja validată în producție pe LXC 171 + Cons: - Reimplementează parserul stream-json (DRY vs `_run_claude`) + - Reimplementează ce oferă SDK-ul oficial; risc de obsolescență + - Fără `interrupt()` — ar trebui construit separat + Reuses: _get_session_lock, _safe_env, build_system_prompt, sentinela __AUDIO__ + +APPROACH B: Coadă + coalescing (minimal viable) + Summary: Mesajul 2 se acumulează și pleacă ca prompt separat după turul 1. + Effort: S (human ~4h / CC ~20min) + Risk: Low + Pros: - Diff minim, zero procese noi, zero RAM în plus + - Nu atinge deloc calea de rate limit + Cons: - NU rezolvă problema — corecția ajunge tot după ce Claude a greșit + - Respins explicit de utilizator la întrebarea de arhitectură + Reuses: lock-ul existent + ELIMINAT: nu satisface cerința. + +APPROACH C: claude-agent-sdk (ideal architecture) + Summary: `ClaudeSDKClient` din pachetul oficial `claude-agent-sdk` (v0.2.151). + `client.query(async_generator)` livrează mesaje mid-tur; `client.interrupt()` oprește turul. + Autentificare pe credențialele CLI — compatibil cu abonamentul, fără API key. + Effort: M (human ~3 zile / CC ~2-3h) — comparabil cu A + Risk: Med + Pros: - Steering ȘI interrupt out of the box; interrupt e o capabilitate pe care A nu o are + - Framing stream-json, ciclul de viață al procesului și sesiunile sunt întreținute upstream + - Zero cod de parsare de întreținut; imun la schimbări de format ale CLI-ului + Cons: - API async-only (`async with ClaudeSDKClient()`) — reintroduce granița sync/async + pe care planul a evitat-o intenționat; `route_message` e sync cu multe căi de comenzi rapide + - Dependență nouă pe un pachet 0.2.x cu 151 de release-uri — suprafață de churn + - Maparea `--system-prompt` / `--dangerously-skip-permissions` / allowed_tools + pe opțiunile SDK trebuie verificată empiric + Reuses: build_system_prompt, sessions/active.json, sentinela __AUDIO__ +``` + +**RECOMMENDATION: A. Alternativa C e ELIMINATĂ pe licențiere, nu pe merit tehnic.** + +Documentația Agent SDK (`code.claude.com/docs/en/agent-sdk/overview`) conține nota: + +> *„Unless previously approved, Anthropic does not allow third party developers to offer +> claude.ai login or rate limits for their products, including agents built on the Claude +> Agent SDK. Use the API key authentication methods described in the Quickstart instead."* + +Plus: „Use of the Claude Agent SDK is governed by Anthropic's Commercial Terms of Service." +Mecanic SDK-ul chiar poate porni pe credențialele CLI-ului, dar direcția politicii e API key — +iar constrângerea utilizatorului e explicită: **doar abonament, fără API**. C e închisă. + +Aceeași pagină recomandă, pentru orice limbaj în afara SDK-urilor Python/TS: +*„run the CLI as a subprocess with the `-p` flag"* — exact ce face echo-core azi și ce +extinde varianta A. Calea actuală e cea documentată, nu un ocol. + +**Clasificare: MECHANICAL, nu taste.** Constrângerea utilizatorului elimină singura alternativă +concurentă. A se implementează cu corecțiile E2 (parser partajat) și E3 (autocompact) care erau +oricum îmbunătățiri de sine stătătoare, plus E1 (cancel) — capabilitatea pe care C ar fi adus-o +gratis și pe care acum trebuie s-o construim explicit. E1 devine astfel mai valoros, nu mai puțin. + +## 0D. SELECTIVE EXPANSION + +**Complexity check:** planul atinge 6 fișiere (`claude_runner.py` nou, `claude_session.py`, `router.py`, 3 adaptoare) + teste. Sub pragul de 8 — fără miros de complexitate. Două clase noi (`ClaudeProcess`, `RunnerRegistry`) — la limita de 2, acceptabil. + +**Minimum set care atinge scopul:** etapele 1-3. Etapele 4-6 sunt hardening, nu funcționalitate — dar 4 (rate limit) e **obligatorie**, nu opțională: fără ea o cale existentă se rupe tăcut. + +**Scan de expansiune (candidați, auto-decis):** + +| # | Oportunitate | Efort | Decizie | Principiu | +|---|---|---|---|---| +| E1 | **Cancel/stop turn** (`/stop`) — oprește un tur scăpat de sub control | S (CC ~20min) | **ACCEPTAT în scope** | P2 — în raza de acțiune (același `ClaudeProcess`), <1 zi. Odată ce ai proces persistent, `proc.stop()` există deja; expunerea lui e trivială și e capabilitatea vecină cea mai valoroasă | +| E2 | **Parser stream-json partajat** între `_run_claude` și runner | S (CC ~15min) | **ACCEPTAT în scope** | P4 DRY — altfel două parsere diverg | +| E3 | **`--autocompact auto`** la spawn | XS | **ACCEPTAT în scope** | P1 completeness — fără el sesiunile lungi mor pe context | +| E4 | Placeholder editabil (un mesaj care se actualizează, ca în bridge) în loc de stream de mesaje noi | M | **DEFER → TODOS.md** | În afara razei — schimbă UX-ul tuturor răspunsurilor, nu doar al celor steered | +| E5 | Steering cu atașamente (poză trimisă mid-tur) | M | **DEFER → TODOS.md** | Adaptoarele salvează deja atașamente ca `[ATTACHMENT:path]`; merge, dar e scope nou | +| E6 | Procesele vii expuse în dashboard (`workspace.html`) | S | **SKIP** | `eco status` (etapa 5) acoperă nevoia; dashboard-ul ar fi duplicare | + +**Scope acceptat:** E1 (cancel), E2 (parser partajat), E3 (autocompact). +**Deferat la TODOS.md:** E4, E5. +**Respins:** E6. + +## 0E. Temporal Interrogation + +``` + HOUR 1 (fundații): Ce formă are dict-ul întors de run_turn? → identic cu _run_claude, altfel + se rupe start_session/resume_session. Unde stă parserul comun? → funcție + la nivel de modul în claude_session.py, importată de runner. + HOUR 2-3 (logică): Ce se întâmplă dacă steering-ul prinde procesul între `result` și + următorul tur? → fallback la tur normal. Ce se întâmplă cu `on_text` + în timpul unui tur steered? → blocurile continuă să curgă; utilizatorul + vede două fire de text. DECIZIE NECESARĂ (vezi P4). + HOUR 4-5 (integrare): Rate limit-ul nu mai vine ca exit code. Unde se detectează? → în parser, + pe `result.is_error` + textul. Trebuie să ridice ACELAȘI RuntimeError + ca azi, altfel `_local_fallback_reply` nu mai pornește. + Voice și text partajează channel_id — un turn voice mid-tur text devine + steering. Dorit sau nu? DECIZIE NECESARĂ. + HOUR 6+ (polish): Fake `claude` pentru teste trebuie să vorbească stream-json pe stdout ȘI + să citească stdin — altfel testul de steering nu e testabil. + `/clear` trebuie să omoare procesul viu, nu doar să șteargă intrarea JSON. +``` + +## 0F. Mode Selection + +**SELECTIVE EXPANSION** confirmat (feature enhancement pe sistem existent). Abordarea aleasă sub acest mod: **A + corecțiile E2/E3 din C**, cu E1 (cancel) adăugat în scope. + +## Constrângere confirmată de utilizator (2026-09-02) + +**Doar abonament Claude Pro/Max — fără API Anthropic, fără API key.** +Ambele abordări viabile respectă asta: A folosește CLI-ul `claude` ca subprocess (ca azi); +C (`claude-agent-sdk`) se autentifică pe **credențialele CLI-ului**, nu pe API key — +documentația SDK-ului o confirmă explicit. Niciun apel la `api.anthropic.com` din cod propriu. + +## Measurement: RAM per proces claude viu + +Măsurat pe această mașină (`ps -eo rss`): **292 MB și 541 MB** pentru două procese `claude` vii. +Mașina are 8192 MB total, ~5750 MB disponibili. + +`max_live=4` × ~400 MB = ~1,6 GB ținuți permanent doar în procese inactive. +**Decizie (auto, P3 pragmatic): `steering.max_live` implicit 2, nu 4.** +Marius vorbește pe 1-2 canale simultan în practică; 4 e dimensionat pentru un scenariu care nu există. + +## Section 1: Architecture Review + +**Graf de dependențe — înainte / după:** + +``` + ÎNAINTE DUPĂ + adapters ──▶ router ──▶ claude_session adapters ──▶ router ──▶ claude_session + │ │ ├──▶ _run_claude (one-shot) + └──▶ _run_claude ──▶ Popen │ │ ▲ + (moare la finalul turului) │ │ └── heartbeat, planning_session + │ └──▶ claude_runner ──▶ ClaudeProcess (viu) + heartbeat ─────┐ │ │ + planning ──────┴──▶ _run_claude │ └──▶ RunnerRegistry (+reaper thread) + └──▶ _parse_stream() ◀── PARTAJAT (E2) +``` + +Cuplare nouă: `claude_runner` → `claude_session` pentru parser + `build_system_prompt` + `_safe_env`. +Unidirecțională, justificată. **Fără import circular**: runner importă din session, session importă runner *lazy* +(în corpul lui `send_message`), nu la nivel de modul. + +**Flux de date — patru căi, pentru `steer()`:** + +``` + mesaj ──▶ [inflight?] ──▶ wrap [EXTERNAL CONTENT] ──▶ json.dumps ──▶ stdin.write ──▶ flush + │ │ │ │ + ▼ ▼ ▼ ▼ + nil: text=None → TypeError fără wrap = GAUR non-UTF8 → BrokenPipeError + GAP: guard explicit DE SECURITATE json.dumps (proces mort între + (vezi Section 3) escapează OK check și write) + empty: text="" → tur gol, GAP: prinde și cazi + Claude răspunde aiurea pe tur normal + GAP: sari peste steering + error: procesul a murit → BrokenPipeError → fallback la tur normal (respawn cu --resume) +``` + +**Mașină de stare `ClaudeProcess`:** + +``` + ┌──────────┐ start() ┌─────────┐ run_turn() ┌──────────┐ + │ DEAD │───────────▶│ ALIVE │─────────────▶│ INFLIGHT │ + └──────────┘ │ idle │◀─────────────└──────────┘ + ▲ └─────────┘ result │ + │ │ │ steer() ── permis DOAR aici + │ stop() / reaper / │ │ + │ crash / model change │ │ + └───────────────────────┴─────────────────────────┘ + timeout → stop() → DEAD + + Tranziții imposibile și ce le previne: + DEAD ──steer()──▶ X : `if not alive: raise TurnFailed` în send() + INFLIGHT ──reaper──▶ X : reaper sare peste orice proces inflight + INFLIGHT ──run_turn()──▶ X: lock-ul per canal serializează tururile +``` + +**Findings:** + +| # | Finding | Severitate | Decizie (auto) | Principiu | +|---|---|---|---|---| +| A1 | **Fără shutdown handler.** `main.py` rulează adaptoarele sub `asyncio.gather`. La `systemctl restart` / SIGTERM, procesele `claude` vii rămân orfane (500 MB fiecare). Bridge-ul are `stop_all()`; planul nu-l menționează. | **HIGH** | Adaugă `RunnerRegistry.stop_all()` legat de shutdown-ul din `main.py` + `atexit` | P1 completeness | +| A2 | **`max_live` atins cu toate procesele inflight** — LRU nu poate opri un proces inflight, deci fie depășești plafonul, fie blochezi. Nedefinit în plan. | MED | Degradare grațioasă: canal nou peste plafon → cade pe calea one-shot existentă (`resume_session`), nu blochează | P1 completeness | +| A3 | **Reaper-ul e SPOF.** Dacă thread-ul moare pe o excepție, procesele se acumulează la infinit, tăcut. | MED | Bucla prinde orice excepție și continuă (tiparul din `runner.py:_reaper_loop`) + o linie de log la fiecare reap | P1 + lecția „turnul nu se pierde niciodată" | +| A4 | **Rollback incomplet.** Flag-ul off nu omoară procesele deja vii — rămân zombie până la reaper. | MED | Flip-ul flagului declanșează `stop_all()`; documentat în etapa 6 | P1 | +| A5 | Registry-ul e stare globală mutabilă la nivel de modul (ca `_session_locks`) | LOW | Acceptat — tipar existent. Testele au nevoie de fixture care golește registry-ul, ca `_clear_session_locks` | P3 pragmatic | + +**Scaling:** rupe primul RAM-ul (2 procese × ~400 MB), nu CPU. La 10x canale active, plafonul `max_live=2` forțează degradarea la one-shot — comportament corect, nu prăbușire. +**SPOF:** reaper-ul (A3) și bridge-ul WhatsApp (preexistent, nu introdus aici). +**Rollback:** `steering.enabled=false` + restart serviciu. Sub 30 secunde. Cu A4 rezolvat, și fără restart. + +## Section 2: Error & Rescue Map + +``` + METODĂ/CODEPATH | CE POATE MERGE PROST | CLASĂ EXCEPȚIE + -----------------------------|-----------------------------------|-------------------- + ClaudeProcess.start() | binarul claude lipsește | FileNotFoundError + | spawn eșuează (fd-uri epuizate) | OSError + ClaudeProcess.send()/steer() | procesul a murit între check/write| BrokenPipeError + | stdin închis | ValueError + ClaudeProcess.run_turn() | turul depășește timeout | TimeoutError + | stdout EOF fără `result` | RuntimeError + | linie JSON coruptă | json.JSONDecodeError + | RATE LIMIT în result.is_error | RuntimeError ← NOU + | pipe stderr plin → deadlock | (blocare, fără excepție) + RunnerRegistry.reap_once() | stop() aruncă pe un proces mort | ProcessLookupError + _parse_stream() | bloc text fără cheia 'text' | KeyError + + CLASĂ EXCEPȚIE | PRINSĂ? | ACȚIUNE | UTILIZATORUL VEDE + ------------------------|---------|----------------------------------|--------------------------- + FileNotFoundError | DA | ridică mesajul de instalare | „Claude CLI not found" + | | (identic cu _run_claude:306) | + OSError la spawn | NU ← GAP| — | traceback brut ← RĂU + BrokenPipeError | NU ← GAP| — | steering pierdut tăcut ← CRITIC + TimeoutError | DA | stop() + ridică, ACELAȘI mesaj | „Claude CLI timed out after Ns" + | | ca azi | + RuntimeError (EOF) | DA | stop() + stderr tail în mesaj | „Claude CLI error: …" + json.JSONDecodeError | DA | `continue` (ca _run_claude:347) | nimic (transparent) + RuntimeError RATE LIMIT | NU ← GAP| — | **fallback-ul local NU pornește** ← CRITIC + deadlock stderr | NU ← GAP| — | turul îngheață până la timeout ← CRITIC + ProcessLookupError | NU ← GAP| — | reaper-ul moare → scurgere procese + KeyError în parser | NU ← GAP| — | turul cade pe un bloc malformat +``` + +**Cele patru GAP-uri critice și acțiunea de reparare:** + +1. **RATE LIMIT.** `is_rate_limit_error` potrivește pe textul `"hit your …limit"` într-un `RuntimeError`. Cu proces persistent, limita vine ca linie `result` cu `is_error: true`. Parserul trebuie să ridice **exact** `RuntimeError(f"Claude CLI error (exit 1): {detail}")` cu textul limitei, ca `router.py:611` să-l prindă și `_local_fallback_reply` să pornească. Fără asta, cea mai muncită cale din repo moare tăcut. +2. **BrokenPipeError la steer.** Prinde-l → marchează procesul mort → **refă mesajul ca tur normal**. Nu-l lăsa să se piardă (lecția explicită din CLAUDE.md: „turnul nu se pierde niciodată"). +3. **Deadlock stderr.** Thread dedicat de drenare, obligatoriu. Fără el un tur cu mult stderr blochează procesul fără nicio excepție. +4. **ProcessLookupError în reaper.** `contextlib.suppress` în jurul lui `stop()`, buclă care nu moare. + +**Regulă respectată:** niciun `except Exception` fără re-raise. Singura excepție permisă e bucla reaper-ului, unde înghițirea e intenționată — dar cu log. + +## Section 3: Security & Threat Model + +| # | Amenințare | Probabilitate | Impact | Mitigat de plan? | +|---|---|---|---|---| +| S1 | **Mesajul steered ocolește wrapping-ul `[EXTERNAL CONTENT]`.** `start_session:489` și `resume_session:563` împachetează fiecare mesaj între markeri, iar system prompt-ul (`:451-453`) instruiește explicit să nu se supună instrucțiunilor dinăuntru. Dacă `steer()` scrie textul brut pe stdin, **protecția la injecție dispare exact pentru mesajele mid-tur** — și acelea sosesc când Claude e deja în mijlocul unei acțiuni cu unelte. | MED | **HIGH** | **NU — GAP.** `steer()` trebuie să folosească același wrapping. Ne-negociabil. | +| S2 | Proces persistent cu `--dangerously-skip-permissions` viu până la 20 min după ultimul mesaj | LOW | MED | Parțial. Raza de acțiune a unei injecții persistă între tururi — dar și azi persistă prin `--resume`. Marginal nou: fereastra de timp. `idle_reap_s` o mărginește. Acceptat. | +| S3 | Escapare JSON pe stdin (text cu ghilimele/newline rupe protocolul) | LOW | MED | DA — `json.dumps(ensure_ascii=False)`, ca în `runner.py:user_message` | +| S4 | Secrete în argv-ul procesului persistent (vizibile în `ps`) | LOW | MED | Neschimbat față de azi — `--system-prompt` e deja în argv. Nu regresează, dar merită notat: procesul e vizibil în `ps` mai mult timp. | +| S5 | Comenzi (`/clear`, `/model`) interpretate ca steering | LOW | LOW | DA — `route_message` rutează comenzile înainte de `send_message` | + +**Suprafață de atac nouă:** zero endpoint-uri noi, zero input-uri de rețea noi. Singurul canal nou e stdin-ul unui proces local. **S1 e singurul finding real și e o regresie, nu un risc teoretic.** + +## Section 4: Data Flow & Interaction Edge Cases + +``` + INTERACȚIUNE | EDGE CASE | TRATAT? | CUM + ------------------------------|----------------------------------|---------|--------------------------- + Mesaj în timpul unui tur | procesul moare între check/write | GAP | → fallback tur normal (S2 §2) + | text gol | GAP | → sari peste steering + | 2 mesaje în 100 ms | DA | _stdin_lock serializează + | atașament mid-tur | DEFER | E5 → TODOS.md + Comandă în timpul unui tur | /clear pe canal cu proces viu | GAP | → clear_session trebuie + | | | să cheme registry.stop() + | /model pe canal cu proces viu | GAP | → set_session_model trebuie + | | | să oprească procesul + Turn voice în timpul unui | voice și text partajează | GAP | DECIZIE NECESARĂ — vezi Gate + tur text (același channel_id) | session_key=channel_id | | + Reaper vs tur | reap în timp ce turul e inflight | DA | skip pe inflight + Restart serviciu | procese orfane | GAP | A1 → stop_all() + Flip flag on→off | procese vii rămân | GAP | A4 → stop_all() +``` + +Cinci GAP-uri, toate cu fix specificat. Cel voice e singurul care cere o decizie umană, nu un fix. + +## Section 5: Code Quality Review + +* **DRY:** o singură violare reală — parserul stream-json (E2, deja acceptat în scope). `_run_claude:335-370` și bucla de consum din runner ar fi 90% identice. +* **Numire:** `ClaudeProcess` / `RunnerRegistry` urmează bridge-ul; consistent. `steer()` e numit după ce face, nu cum. Bine. +* **Over-engineering:** `RunnerRegistry` cu reaper + LRU + plafon pentru maximum 2 procese e la limită. Justificat de A1/A3 (scurgere de procese = 500 MB fiecare), nu de scală. +* **Under-engineering:** `steer()` fără guard pe text gol și fără wrapping (S1) — fragil pe calea fericită. +* **Complexitate ciclomatică:** `send_message` devine dispecer cu ~4 ramuri (steering on/off × inflight/nu × plafon atins). Sub 5. Acceptabil, dar extrage `_try_steer(channel_id, text) -> bool` ca funcție separată în loc să umfli `send_message`. +* Niciun `except Exception` nou în afara buclei reaper-ului. + +## Section 6: Test Review + +``` + FLUXURI UX NOI: + - mesaj trimis în timpul unui tur → reacție ➡️, fără text + - /stop în timpul unui tur (E1) + FLUXURI DE DATE NOI: + - text → wrap EXTERNAL CONTENT → json → stdin → model (mid-tur) + - stdout stream-json → parser partajat → dict identic cu _run_claude + CODEPATHS NOI: + - steering.enabled on/off + - inflight / not inflight + - plafon max_live atins → degradare one-shot + - respawn cu --resume după moartea procesului + - reap pe inactivitate + JOBURI ASINCRONE NOI: + - thread drenare stderr (per proces) + - thread reaper (unul global) + INTEGRĂRI NOI: + - proces claude persistent cu stdin deschis + CĂI DE EROARE NOI: + - rate limit din result.is_error (NU din exit code) + - BrokenPipeError la steer + - timeout tur → stop() → aceeași excepție ca azi +``` + +| # | Test | Tip | Există în plan? | +|---|---|---|---| +| T1 | Tur normal prin proces persistent → dict identic cu `_run_claude` | Unit (fake claude) | Da | +| T2 | Steering mid-tur ajunge pe stdin | Unit (fake claude care citește stdin) | Da | +| T3 | Respawn cu `--resume` după kill | Unit | Da | +| T4 | Reaper nu omoară un proces inflight | Unit | Da | +| T5 | Timeout → `TimeoutError` cu **exact** mesajul de azi | Unit | Da | +| T6 | **Rate limit din stream ridică RuntimeError pe care `is_rate_limit_error` îl prinde** | Unit | Da (etapa 4) | +| T7 | **`_local_fallback_reply` chiar pornește pe rate limit din proces persistent** | Integration | **LIPSEȘTE — critic.** T6 testează detecția, nu lanțul complet. Ăsta e testul care contează. | +| T8 | **`steer()` împachetează în `[EXTERNAL CONTENT]`** | Unit | **LIPSEȘTE — S1** | +| T9 | **BrokenPipe la steer → mesajul se refă ca tur normal** | Unit | **LIPSEȘTE** | +| T10 | Suita existentă trece cu `steering.enabled=false` | Regression | Da (etapa 2) | +| T11 | `test_claude_session_mutex.py` trece nemodificat | Regression | Implicit — **fă-l explicit**, e testul care pinează contractul lock-ului | +| T12 | `/clear` omoară procesul viu | Unit | **LIPSEȘTE** | +| T13 | `stop_all()` la shutdown, zero orfani | Unit | **LIPSEȘTE — A1** | + +**Testul pentru 2 dimineața vineri:** T7. Dacă fallback-ul local nu mai pornește, Marius rămâne fără asistent la limită de rate și nimeni nu află până nu se întâmplă. +**Ce ar scrie un QA ostil:** trimite 5 mesaje în 200 ms în timpul unui tur, apoi `/clear`, apoi încă unul. +**Chaos test:** `kill -9` pe procesul `claude` la mijlocul unui tur, verifică respawn cu `--resume` și că sesiunea continuă. +**Flakiness:** T2 și T4 depind de timing. Fake-ul `claude` trebuie să semnalizeze prin fișier/pipe, nu prin `sleep`. +**LLM/eval:** planul nu atinge `personality/*.md` și nici prompt-ul fallback-ului local. Nicio suită de eval nu trebuie rulată. (Dacă etapa 4 ajunge să atingă `build_system_prompt`, atunci da.) + +## Section 7: Performance Review + +Fără DB, fără query-uri, fără N+1 — secțiunea are o singură dimensiune reală: **memoria**. +Măsurat: 292-541 MB RSS per proces `claude`. Cu `max_live=2` → până la ~1 GB rezident. +Mașina are 5750 MB disponibili, deci încape, dar nu e neglijabil pe un host care rulează și Ollama. +Câștig de latență: turul 2+ pe un canal sare spawn-ul (~2-4 s din spike-uri), deci steering-ul e +și o optimizare de latență, nu doar o funcționalitate. +Presiune pe pool-uri de conexiuni: zero. Fără alte findings. + +## Section 8: Observability & Debuggability Review + +| Ce | Există azi | Nevoie nouă | +|---|---|---| +| Log per tur | `_invoke_log` cu channel/model/durată/tokens (`:513`) | Adaugă `steered=N` la linia turului | +| Log per steering | — | **Obligatoriu.** O linie per steer: canal, lungime text, dacă a reușit. Fără ea, „nu a ținut cont de mesajul meu" e nediagnosticabil | +| Fiecare `return`/eșec tăcut | Lecția din CLAUDE.md: fiecare `return None` din `_local_fallback_reply` e logat | Aplică aceeași regulă: **fiecare cale prin care un mesaj NU devine steering se loghează** | +| Procese vii | — | `eco status` (etapa 5): câte procese, pe ce canale, de cât timp inactive | +| Reaper | — | Log la fiecare proces oprit, cu motivul | +| stderr | Citit la final în `_run_claude:377` | Buffer circular 50 linii, expus în `eco doctor` | + +**Debuggabilitate la 3 săptămâni:** cu logurile de mai sus, da. Fără logul de steering, nu. + +## Section 9: Deployment & Rollout Review + +Fără migrări DB, fără schimbări de schemă. `config.json` are `reload()` (`config.py:24`), deci flagul +poate fi citit la cald — dar procesele deja vii nu se opresc singure (A4). + +``` + Etapa 1-2 (flag off) ──▶ deploy, zero comportament schimbat ──▶ suita verde + │ │ + │ rollback: nimic de făcut, calea nu e activă │ + ▼ ▼ + Etapa 3-5 (flag off) ──▶ deploy incremental ──────────────▶ teste + eco status + │ + ▼ + Etapa 6: flip pe UN canal ──▶ dogfood 2-3 zile ──▶ flip global + │ │ + │ rollback: steering.enabled=false + stop_all() (<30s) + ▼ + Verificare primele 5 min: trimite 2 mesaje suprapuse, confirmă ➡️ și un singur răspuns coerent + Verificare prima oră: `eco status` arată procese care se sting după idle_reap_s +``` + +**Fereastră de risc la deploy:** `systemctl --user restart echo-core` cu procese vii → orfani (A1). +Fix-ul A1 e prerechizit pentru etapa 6, nu opțional. + +## Section 10: Long-Term Trajectory Review + +* **Datorie tehnică introdusă:** un parser stream-json și un manager de procese proprii, de întreținut la fiecare schimbare de format al CLI-ului. E2 (parser partajat) o înjumătățește; nu o elimină. +* **Path dependency:** dacă `claude-agent-sdk` devine calea normală (probabil — e Claude Code ca bibliotecă, întreținut upstream), varianta A devine cod aruncat. Nu blochează nimic, dar e ~300 de linii care ar putea fi zero. +* **Reversibilitate: 4/5.** Flag + un modul nou; `_run_claude` rămâne intact ca plasă. Nu e ușă cu sens unic. +* **Ce vine după:** interrupt/cancel (E1, deja în scope), atașamente mid-tur (E5), placeholder editabil (E4). Arhitectura A le suportă pe toate — `ClaudeProcess` e locul potrivit. +* **Potențial de platformă:** dacă runner-ul devine stratul unic de sesiune, `planning_session.py` și Ralph l-ar putea folosi. Nu în acest plan, dar arhitectura nu-l exclude. +* **Întrebarea de la 1 an:** un inginer nou citește `claude_runner.py` și înțelege de ce există două căi (persistent vs one-shot)? **Numai dacă docstring-ul modulului o spune explicit.** Cere-l: de ce heartbeat/planning rămân one-shot. + +## Section 11: Design & UX Review + +**SĂRIT** — zero scope UI. Planul nu introduce ecrane, formulare sau componente; singurul element vizibil e o reacție emoji pe un mesaj existent, în API-uri de chat care o suportă nativ. Verificat prin grep pe termeni de UI: singurele potriviri au fost substringuri românești (*informat*, *reformul*), zero button/modal/screen/dialog. + +## Required Outputs — Phase 1 + +### NOT in scope + +| Item | De ce e deferat | +|---|---| +| Placeholder editabil în loc de stream de mesaje noi (E4) | Schimbă UX-ul **tuturor** răspunsurilor, nu doar al celor steered. Scope propriu. → TODOS.md | +| Steering cu atașamente (E5) | Adaptoarele salvează deja `[ATTACHMENT:path]`; merge, dar e funcționalitate nouă, nu parte din steering. → TODOS.md | +| Procese vii în dashboard (E6) | `eco status` acoperă nevoia; dashboard-ul ar fi duplicare. Respins. | +| Migrarea heartbeat / planning / Ralph pe proces persistent | Decizie explicită a utilizatorului. Niciun câștig — nu există om care să facă steering pe un job de noapte. | +| `claude-agent-sdk` (alternativa C) | Eliminată pe licențiere: docs direcționează spre API key, iar constrângerea e „doar abonament". | +| Compactare proprie / management de context | `--autocompact auto` (T8) delegă asta CLI-ului. A construi ceva propriu ar fi ocean, nu lac. | + +### What already exists + +Vezi tabelul complet din **0B**. Rezumat: 7 din 9 sub-probleme au deja cod în repo care se reutilizează +(`_get_session_lock`, `_load_sessions`, `_safe_env`, `build_system_prompt`, sentinela `__AUDIO__:`, +reacțiile pe toate trei adaptoarele, detecția de rate limit). Singurele lucruri chiar noi sunt +**bucla de proces persistent** și **scrierea pe stdin**. Restul e integrare, nu construcție. + +Corecție importantă față de planul inițial: **reacțiile există deja pe toate trei adaptoarele** +(`add_reaction` / `set_message_reaction` / `react_whatsapp`), deci etapa 3 e mai mică decât estimat +și nu are nevoie de cercetare pe bridge-ul Baileys. + +### Failure Modes Registry + +``` + CODEPATH | FAILURE MODE | RESCUED? | TEST? | USER SEES | LOGGED? + --------------------|-------------------------|----------|-------|------------------|-------- + steer() | proces mort (BrokenPipe) | T7 | T7 | răspuns normal | T12 + steer() | fără wrap injecție | T1 | T1 | — | — + steer() | text gol | T1 | T1 | nimic (sări) | T12 + run_turn() | rate limit în stream | T2 | T6 | fallback local | da + run_turn() | timeout | plan | T5* | mesaj identic azi| da + run_turn() | EOF fără result | plan | da | „Claude CLI err" | da + stderr pipe | plin → deadlock | T3 | T3 | tur înghețat | da + reaper | excepție → thread mort | T11 | T11 | scurgere tăcută | T11 + registry | max_live + toate inflight| T10 | T10 | one-shot (lent) | T10 + shutdown | procese orfane | T4 | T13* | — | T4 + clear_session | proces rămas viu | T9 | T9 | model stale | T9 +``` +`T5*`/`T13*` = numerotare din Section 6 (teste), restul din Implementation Tasks. +**Zero rânduri CRITICAL GAP rămase** — fiecare RESCUED=N din analiza inițială are acum un task. + +### Dream state delta + +Planul + expansiunile acceptate (E1 cancel, E2 parser partajat, E3 autocompact) duc echo-core de la +„mesajul 2 așteaptă" la „mesajul 2 ajunge, și pot opri un tur scăpat". Rămâne la ~70% din idealul +de 12 luni. Nerezolvate: decizia voice-vs-text pe același canal, atașamente mid-tur, un strat de +sesiune partajat cu Ralph/planning. + +### Scope Expansion Decisions + +* **Acceptate:** E1 (cancel `/stop`), E2 (parser stream-json partajat), E3 (`--autocompact auto`) +* **Deferate la TODOS.md:** E4 (placeholder editabil), E5 (atașamente mid-tur) +* **Respinse:** E6 (procese vii în dashboard — duplicare peste `eco status`) + +### Stale Diagram Audit + +`CLAUDE.md` § Arhitectură descrie fluxul ca `Adapter → router.py → claude_session.py → Claude CLI`. +Rămâne corect, dar **incomplet** după acest plan — trebuie adăugat `claude_runner.py` și explicat +de ce există două căi. Diagrama nu e greșită, e trunchiată. Task: T15 (docstring) + o linie în CLAUDE.md. +Alte diagrame ASCII în fișierele atinse: niciuna. + +## Implementation Tasks + +Sintetizate din findings-urile de mai sus. Fiecare derivă dintr-un finding specific. + +- [ ] **T1 (P1, human: ~2h / CC: ~15min)** — claude_runner — Împachetează mesajele steered în `[EXTERNAL CONTENT]` + - Surfaced by: Section 3 S1 — text brut pe stdin ocolește protecția la injecție exact pentru mesajele mid-tur + - Files: `src/claude_runner.py`, `tests/test_claude_runner.py` + - Verify: `pytest tests/test_claude_runner.py -k external_content` +- [ ] **T2 (P1, human: ~3h / CC: ~20min)** — claude_session — Ridică același `RuntimeError` la rate limit din `result.is_error` + - Surfaced by: Section 2 — procesul persistent nu iese cu cod ≠ 0, deci `_local_fallback_reply` n-ar mai porni niciodată + - Files: `src/claude_session.py`, `src/claude_runner.py` + - Verify: `pytest tests/test_local_fallback.py` +- [ ] **T3 (P1, human: ~1h / CC: ~10min)** — claude_runner — Thread dedicat de drenare stderr, deque mărginit + - Surfaced by: Section 2 — pipe-ul plin blochează procesul fără nicio excepție + - Files: `src/claude_runner.py` + - Verify: test cu fake claude care scrie >64KB pe stderr +- [ ] **T4 (P1, human: ~2h / CC: ~15min)** — main — `stop_all()` la shutdown și `atexit` + - Surfaced by: Section 1 A1 — `systemctl restart` lasă orfani de ~500 MB + - Files: `src/main.py`, `src/claude_runner.py` + - Verify: pornește, creează proces, `SIGTERM`, `pgrep -f claude` gol +- [ ] **T5 (P1, human: ~1h / CC: ~10min)** — claude_session — Extrage parserul stream-json partajat + - Surfaced by: Section 5 + 0B — singura violare DRY reală din plan + - Files: `src/claude_session.py`, `src/claude_runner.py` + - Verify: `pytest tests/test_claude_session.py` +- [ ] **T6 (P1, human: ~3h / CC: ~20min)** — tests — Integration: rate limit prin proces persistent pornește chiar fallback-ul + - Surfaced by: Section 6 T7 — testul de 2 dimineața vineri + - Files: `tests/test_local_fallback.py` + - Verify: `pytest tests/test_local_fallback.py -k persistent` +- [ ] **T7 (P1, human: ~1h / CC: ~10min)** — claude_runner — `BrokenPipeError` la steer → refă ca tur normal + - Surfaced by: Section 2 gap 2 + lecția „turnul nu se pierde niciodată" + - Files: `src/claude_runner.py`, `tests/test_claude_runner.py` + - Verify: test care omoară procesul între check și write +- [ ] **T8 (P2, human: ~30min / CC: ~5min)** — claude_runner — `--autocompact auto` la spawn + - Surfaced by: 0A P7 — sesiunea persistentă crește monoton spre limita de context + - Files: `src/claude_runner.py` + - Verify: `build_cmd` conține flagul +- [ ] **T9 (P2, human: ~1h / CC: ~10min)** — claude_session — `/clear` și `/model` opresc procesul viu + - Surfaced by: Section 4 — altfel procesul rămâne cu config stale + - Files: `src/claude_session.py` + - Verify: `pytest tests/test_claude_runner.py -k clear` +- [ ] **T10 (P2, human: ~2h / CC: ~15min)** — claude_runner — Degradare la one-shot când `max_live` e atins cu toate inflight + - Surfaced by: Section 1 A2 — LRU nu poate opri un proces inflight + - Files: `src/claude_runner.py` + - Verify: test cu `max_live=1` și două canale +- [ ] **T11 (P2, human: ~1h / CC: ~10min)** — claude_runner — Reaper rezistent la excepții + log per reap + - Surfaced by: Section 1 A3 — reaper mort = scurgere tăcută + - Files: `src/claude_runner.py` + - Verify: test care face `stop()` să arunce +- [ ] **T12 (P2, human: ~30min / CC: ~5min)** — claude_runner — Loghează fiecare cale prin care un mesaj NU devine steering + - Surfaced by: Section 8 — altfel „nu a ținut cont de mesajul meu" e nediagnosticabil + - Files: `src/claude_runner.py`, `src/claude_session.py` + - Verify: inspecție manuală a logurilor +- [ ] **T13 (P2, human: ~3h / CC: ~20min)** — claude_runner — Comandă `/stop` legată la `ClaudeProcess.stop()` (E1) + - Surfaced by: 0D E1 — capabilitatea vecină cea mai valoroasă odată ce ai proces persistent + - Files: `src/claude_runner.py`, `src/router.py` + - Verify: `/stop` în timpul unui tur oprește procesul, canalul rămâne utilizabil +- [ ] **T14 (P2, human: ~30min / CC: ~5min)** — config — `steering.max_live` implicit 2 + - Surfaced by: Section 7 — măsurat 292-541 MB RSS per proces pe host de 8 GB + - Files: `config.json`, `src/claude_runner.py` + - Verify: default-ul din cod e 2 +- [ ] **T15 (P3, human: ~1h / CC: ~10min)** — claude_runner — Docstring: de ce heartbeat/planning rămân one-shot + - Surfaced by: Section 10 — întrebarea de la 12 luni + - Files: `src/claude_runner.py`, `CLAUDE.md` + - Verify: citire + +--- + +# /autoplan — Phase 2: Design Review + +**SĂRIT — zero scope UI.** Detectat în Phase 0: singurele potriviri pe termeni de UI au fost +substringuri românești (*informat*, *reformul*), zero button/modal/screen/dialog/layout. +Feature-ul nu introduce nicio suprafață vizuală; singurul element vizibil e o reacție emoji +pe un mesaj existent, în API-uri de chat care o suportă nativ. + +--- + +# /autoplan — Phase 2.5: DX Review + +Mod: **DX POLISH**. Voci: `[subagent-only]`. + +## Step 0: DX Scope Assessment + +**Tip de produs:** feature intern într-un asistent personal self-hosted. Nu e un produs public. +**Cine e „developer"-ul aici — două persoane foarte diferite:** + +| Persona | Cine | Ce are nevoie | +|---|---|---| +| **Operatorul** (unic) | Marius — pornește serviciul, citește logurile, dă flip la flag | Să vadă că merge. Să oprească rapid când nu merge. | +| ~~Mentenatorul autonom~~ | ~~Ralph~~ — **confirmat oprit** (toate joburile `enabled=False`, `approved-tasks.json` gol, nimic în crontab) | n/a | + +O singură persona, deci DX aici înseamnă strict **experiență de operare**, nu de integrare. + +**DX completeness inițial: 4/10.** Planul descrie mecanica, nu experiența de operare: +nu spune forma blocului de config, nu spune cum confirmi că merge, nu spune ce se schimbă în `CLAUDE.md`. + +## Developer Journey (9 etape) + +| Etapă | Azi | Cu planul | Fricțiune | +|---|---|---|---| +| Discover | — | § în CLAUDE.md | **GAP — planul nu prevede update la CLAUDE.md** | +| Evaluate | — | citește planul | ok | +| Install | — | `git pull` | zero — fără dependențe noi | +| Configure | — | `"steering": {"enabled": true}` în config.json | **GAP — forma blocului nespecificată** | +| Hello world | — | restart + 2 mesaje suprapuse | **GAP — greu de produs un tur lent la comandă** | +| Integrate | — | automat pe toate 3 adaptoarele | ok | +| Debug | — | loguri (T12) | ok dacă T12 se face | +| Upgrade | — | cheie nouă, default off | ok — sigur în ambele sensuri | +| Scale | — | `max_live` | **GAP — `max_live=0` nedefinit** | + +## Empathy Narrative + +> *„Am dat pull, am pus `steering.enabled: true`, am restartat. Acum… cum verific că merge? +> Trebuie să-i dau ceva de lucru lung, apoi să scriu repede al doilea mesaj și să mă uit după o +> săgeată. Dacă nu apare săgeata — e stins flagul? E plafonul atins? A murit procesul? Nu știu +> care din trei. Și `eco status` nu-mi spune nimic despre asta."* + +Asta e problema centrală de DX: **turnul feature-ului e invizibil când nu funcționează**, iar +verificarea lui cere să fabrici un tur lent la comandă. + +## Pass 1: Getting Started — 6/10 + +TTHW: ~3 min (edit config → restart → 2 mesaje). Tier **Competitive**, nu Champion. +Ce ar fi 10: `eco status` arată `steering: on · 0 procese vii`, iar `eco doctor` verifică +că binarul suportă `--input-format stream-json`. Atunci confirmi în 20 de secunde, fără să +fabrici un tur lent. +**Finding D1 (HIGH):** nicio cale de a confirma că steering-ul e pornit și sănătos fără să-l provoci. +**Fix:** `eco status` + o verificare în `eco doctor`. + +## Pass 2: API/CLI Design — 8/10 + +`steering.enabled` respectă convenția existentă (`local_fallback.enabled`, `heartbeat.enabled`) — +namespace-dict cu cheie `enabled`, citit prin dot-notation. Consistent, ghicibil. +`steer()` / `run_turn()` / `RunnerRegistry` urmează numele din bridge; `steer` spune ce face. +**Finding D2 (MED):** planul nu scrie blocul de config concret. Cere-l explicit în plan: +```json +"steering": { "enabled": false, "idle_reap_s": 1200, "max_live": 2 } +``` +**Finding D3 (MED):** `max_live: 0` și `idle_reap_s: 0` sunt nedefinite. `0` la `max_live` citit +naiv înseamnă „niciun proces permis" = steering mort tăcut, cu flagul aparent pornit — cel mai +prost mod de eșec posibil. **Fix:** `max_live < 1` → tratează ca `enabled: false` **și loghează**; +`idle_reap_s` cu prag minim de 60s. + +## Pass 3: Error Messages & Debugging — 5/10 + +Precedent bun deja în repo: `router.py:298` loghează `„Local fallback requested but +local_fallback.enabled is false"` — problemă + cauză într-o linie. Steering-ul are nevoie de +aceeași disciplină, și e exact ce cere T12. +**Finding D4 (HIGH):** un mesaj care nu devine steering arată **identic** cu unul care devine — +utilizatorul vede un răspuns normal în ambele cazuri. Fără log per cale, „n-a ținut cont de mine" +e nediagnosticabil post-factum. Ăsta e chiar tiparul de eșec pe care CLAUDE.md îl documentează +pentru fallback-ul local (2026-08-23: „detecția a mers, fallback-ul a întors None fără nicio linie +de log, imposibil de diagnosticat"). **Fix:** T12, ridicat de la P2 la **P1**. + +## Pass 4: Documentation & Learning — 3/10 + +**Finding D5 (LOW — retrogradat).** Inițial marcat CRITICAL: `CLAUDE.md:219` spune că Ralph nu +atinge `src/router.py` / `src/claude_session.py`, iar `src/claude_runner.py` — fișier nou — n-ar fi +pe listă. **Verificat: Ralph e oprit.** Toate joburile din `cron/jobs.json` (`night-execute`, +`evening-report`, `morning-report`, ambele `*-coaching`) au `enabled=False`, `approved-tasks.json` +e gol, crontab-ul de sistem n-are nimic. Nu există agent nesupravegheat care să rescrie modulul. +**Fix (ieftin, nu urgent):** dacă Ralph se repornește vreodată, adaugă `src/claude_runner.py` la +lista de la linia 219. Până atunci, nu blochează nimic. + +**Observație de repo (nu e parte din acest plan).** `CLAUDE.md` dedică ~80 de linii sistemului +Ralph — comenzi, tabel de fișiere, flow de aprobare — pentru un sistem care e integral dezactivat. +Documentația asta e acum înșelătoare: m-a făcut să ridic D5 la CRITICAL pe o premisă falsă, și +va induce în eroare orice viitoare sesiune la fel. Merită curățată sau marcată „inactiv", +dar e o schimbare separată — o semnalez, n-o fac aici. + +**Finding D6 (MED):** `CLAUDE.md` § Arhitectură descrie un singur flux +(`Adapter → router → claude_session → CLI`). După plan există **două** căi și niciun cititor viitor +nu va ghici de ce. Cere o subsecțiune scurtă: ce merge persistent, ce rămâne one-shot, **de ce**. + +## Pass 5: Upgrade & Migration Path — 9/10 + +Cheie de config nouă cu default off: pull fără config → nimic nu se schimbă. Downgrade cu cheia +prezentă → cheia e ignorată. Sigur în ambele sensuri, fără migrare, fără codemod. +Singura notă: dacă flip-ul flagului nu declanșează `stop_all()` (task T4/A4), un downgrade la cald +lasă procese vii. Acoperit deja. + +## Pass 6: Developer Environment & Tooling — 6/10 + +Fără dependențe noi, fără build. Testele au nevoie de un fake `claude` care **citește stdin** și +vorbește stream-json — mai greu decât fake-urile existente (care doar scriu pe stdout). +**Finding D7 (MED):** fără acel fake, testul de steering nu e scriibil. E prerechizit pentru T1/T7, +nu o notă de subsol. Bridge-ul are `tests/fake_claude.py` ca referință de tipar. + +## Pass 7: Community & Ecosystem — N/A + +Repo privat, un utilizator, un agent. Nu se aplică; nescoruit ca să nu falsifice media. + +## Pass 8: DX Measurement & Feedback Loops — 4/10 + +Nicio metrică propusă. `_invoke_log` numără tururile; nimic nu numără steering-urile. +**Finding D8 (LOW):** adaugă `steered=N` la linia de log a turului (deja în T12) — atunci poți +răspunde peste o lună la „chiar folosesc funcția asta?" fără să ghicești. + +## DX Scorecard + +``` + Pass Scor Ce lipsește pentru 10 + --------------------------------- ----- ---------------------------------------- + 1. Getting Started 6/10 eco status + eco doctor (D1) + 2. API/CLI Design 8/10 blocul de config scris (D2), max_live=0 (D3) + 3. Error Messages & Debugging 5/10 log per cale ratată (D4) — ridicat la P1 + 4. Documentation & Learning 3/10 regula Ralph (D5) + două-căi în CLAUDE.md (D6) + 5. Upgrade & Migration 9/10 — + 6. Dev Environment & Tooling 6/10 fake claude care citește stdin (D7) + 7. Community & Ecosystem N/A repo privat + 8. DX Measurement 4/10 contor de steering (D8) + --------------------------------- ----- + OVERALL (7 scorate) 6.1/10 +``` + +**TTHW: ~3 min → țintă < 1 min** (cu `eco status` care confirmă starea fără să provoci un tur lent). + +## DX Implementation Checklist + +- [ ] ~~**D5** lista protejată a lui Ralph~~ — retrogradat LOW: Ralph e confirmat oprit +- [ ] **D4** ridică T12 (log per cale ratată) de la P2 la **P1** +- [ ] **D1** `eco status`: `steering: on/off · N procese vii`; `eco doctor`: verifică `--input-format stream-json` +- [ ] **D2** scrie blocul concret de config în plan și în `CLAUDE.md` +- [ ] **D3** `max_live < 1` → dezactivat **cu log**; `idle_reap_s` cu prag minim 60s +- [ ] **D6** subsecțiune în `CLAUDE.md`: două căi, care e care, de ce +- [ ] **D7** `tests/fake_claude.py` care citește stdin — prerechizit pentru T1/T7 +- [ ] **D8** `steered=N` pe linia de log a turului + +--- + +# /autoplan — Phase 3: Eng Review (gate final, pe planul amendat) + +Voci: `[subagent-only]`. + +## Step 0: Scope Challenge (pe cod real, nu pe descriere) + +Planul atinge 6 fișiere + teste, adaugă 2 clase. Sub pragurile de complexitate. +Verificat pe cod: `send_message` (`claude_session.py:611`) e singurul punct de intrare care se +schimbă, iar `_run_claude`/`start_session`/`resume_session` rămân intacte pentru +`heartbeat.py:416` și `planning_session.py:264`. Separarea e reală, nu declarativă. + +**Scope-ul e corect calibrat. Nicio reducere recomandată.** Singura adăugire acceptată (E1/`/stop`) +e ~20 de linii peste infrastructura care oricum se construiește. + +## Section 1: Architecture + +Graf, mașină de stare și căile de eroare sunt deja diagramate în Phase 1 § Section 1. +Nu le repet. Ce adaugă faza de inginerie sunt **cursele**, care nu erau acoperite. + +### Concurență — analiza care lipsea din plan + +Modelul de threading propus: thread A (worker `asyncio.to_thread`) rulează `run_turn()` și citește +stdout blocant; thread B (alt worker) cheamă `steer()` și scrie pe stdin; thread C (reaper) poate +chema `stop()`. Trei fire pe același obiect. + +``` + Thread A (tur) Thread B (steer) Thread C (reaper) + ────────────── ──────────────── ───────────────── + inflight = True + stdout.readline() ──┐ + │ │ read inflight → True + │ │ │ + result primit │ │ ← FEREASTRA DE CURSĂ + inflight = False │ │ + │ │ stdin.write() ──▶ pipe posibil închis + (reaper eligibil) │ BrokenPipeError + │ │ + └──────────────────────────┴──▶ T7: refă ca tur normal +``` + +| # | Cursă | Severitate | Fix | +|---|---|---|---| +| **E-C1** | **TOCTOU pe `inflight`.** B citește `inflight=True`, A termină turul și `inflight=False`, apoi B scrie. Scrierea nimerește un proces care nu mai e în tur → linia devine turul următor, dar utilizatorul a primit deja ➡️ fără text. | **HIGH** | `steer()` face **check + write sub același `_stdin_lock`**, iar `run_turn()` setează `inflight=False` **tot sub acel lock**. Atunci fereastra dispare; T7 rămâne plasă pentru moartea reală a procesului, nu pentru cursă. | +| **E-C2** | **Reaper vs steer.** Reaper-ul sare peste procesele `inflight`, dar un proces viu-și-inactiv poate fi oprit exact în timpul unui `steer()` de pe calea de fallback. | MED | `stop()` ia și el `_stdin_lock` înainte să închidă stdin | +| **E-C3** | **`_stdin_lock` ținut peste I/O de rețea.** Dacă `drain`/`flush` blochează (pipe plin fiindcă procesul nu citește), lock-ul se ține la nesfârșit și blochează și `stop()`. | MED | Scrierile pe stdin sunt mici (o linie JSON) și pipe-ul are 64 KB buffer — practic nu blochează. Documentează invariantul: **niciodată I/O lung sub `_stdin_lock`**. | +| **E-C4** | `stderr` citit de un al patrulea thread per proces | LOW | Deja acoperit (T3). Fără interacțiune cu celelalte lock-uri. | + +### Contractul lock-ului per canal se schimbă — și planul nu o spune + +`send_message` ține azi lock-ul pe tot turul. Steering-ul trebuie **să NU ia lock-ul** — altfel +se auto-blochează, pentru că lock-ul e deja ținut de turul în zbor. Planul spune corect „fără lock". +Dar consecința nu e menționată: **contractul pe care `test_claude_session_mutex.py` îl pinează +explicit devine condiționat de flag.** + +Docstring-ul acelui test spune: *„This test pins that behavior so future refactors must preserve it."* +Cu steering pornit, comportamentul „al doilea apel așteaptă" **nu se mai păstrează** — al doilea apel +face steering și se întoarce imediat. Asta e schimbarea intenționată, dar contractul trebuie rescris, +nu încălcat tăcut. + +**E-C5 (HIGH) — test verde fals.** Testul patch-uiește `claude_session._run_claude`. Dacă +`send_message` rutează spre runner când steering-ul e pornit, `_run_claude` nu mai e chemat deloc, +`concurrent_seen` nu se setează niciodată și testul **trece degeaba** — verde fără să testeze nimic. +**Fix:** testul de mutex forțează explicit `steering.enabled=False` (nu se bazează pe default), +plus un test nou care pinează contractul cu steering pornit: al doilea apel întoarce `__STEERED__` +fără să deschidă un tur. + +## Section 2: Code Quality + +* **Dispecerul:** extrage `_try_steer(channel_id, text) -> bool` din `send_message` (deja notat + în Phase 1 § Section 5). `send_message` rămâne sub 5 ramuri. +* **Import lazy:** `claude_session` importă `claude_runner` **în corpul funcției**, nu la nivel de + modul, fiindcă runner-ul importă parserul din session. Altfel ciclu la import. Scrie-o ca și + comentariu, nu ca folclor. +* **DRY:** o singură violare (parserul), deja task T5. +* Fără abstracții premature: nu introduce o interfață `Backend` cu două implementări pentru două + căi. `if steering: ... else: ...` explicit e mai lizibil (P5). + +## Section 3: Test Review + +Diagrama completă e în Phase 1 § Section 6. Faza de inginerie adaugă: + +| # | Test lipsă | De ce | +|---|---|---| +| **E-T1** | Mutex-ul cu `steering.enabled=False` **forțat explicit** | E-C5 — altfel verde fals | +| **E-T2** | Contract nou: cu steering pornit, al 2-lea apel întoarce `__STEERED__` fără tur nou | Contractul care înlocuiește pinul vechi | +| **E-T3** | Cursă: 20 de iterații care fac steer exact la granița `result` | E-C1 — o cursă netestată e o cursă care se întoarce | +| **E-T4** | `sessions/active.json` primește sid-ul **actualizat după fiecare tur**, nu doar la start | La respawn cu `--resume`, `system/init` poate întoarce un sid nou; dacă nu-l persiști, următorul respawn reia o sesiune moartă | +| **E-T5** | Timeout distruge procesul → turul următor se reia curat cu `--resume` | Timeout-ul acum aruncă un proces cald; verifică că nu pierde sesiunea | + +**E-T4 e un bug latent, nu doar un test lipsă.** Planul spune „persistă în `sessions/active.json`", +dar nu spune **când**. Bridge-ul actualizează `sid` în `finally`-ul fiecărui tur (`bot.py:974`). +Dacă echo-core îl scrie doar la primul tur, un respawn ulterior folosește un sid vechi. +→ **Task nou E1 (P1).** + +## Section 4: Performance + +Măsurătoarea e în Phase 1 § Section 7 (292-541 MB RSS, `max_live=2`). +Adaug o observație care lipsea: **timeout-ul devine mai scump.** Azi un timeout omoară un proces +care oricum murea. Cu proces persistent, un timeout aruncă un proces cald, iar turul următor plătește +respawn (~2-4 s) **plus** reîncărcarea integrală a istoricului prin `--resume`. Pe o sesiune lungă +asta nu e neglijabil. Nu schimbă designul — dar `TURN_TIMEOUT` nu trebuie coborât „ca să fie sigur". + +`_safe_env()` se evaluează la spawn: o rotire de credențiale în keyring **nu ajunge** la un proces +viu până la respawn. Minor, dar notabil pentru `/otp` — dacă tokenul roa2web se reînnoiește, +procesul viu are env-ul vechi. Verifică dacă `roa2web_client` citește keyring-ul la runtime +(atunci e irelevant) sau env-ul de la pornire (atunci e un bug). + +## Section 5: Security + +Acoperit în Phase 1 § Section 3. Singurul finding real rămâne **S1** (wrapping `[EXTERNAL CONTENT]` +la steer, task T1) — o regresie de securitate, nu un risc teoretic. Confirmat pe cod: +`start_session:489` și `resume_session:563` împachetează; `steer()` trebuie să facă la fel. +Fără suprafață de rețea nouă. `--dangerously-skip-permissions` e neschimbat față de azi, doar +fereastra de viață a procesului crește — mărginită de `idle_reap_s`. + +## Section 6: Hidden Complexity + +Ce arată simplu în plan și nu este: + +1. **„`run_turn()` întoarce același dict"** — pare o formalitate. De fapt e contractul care ține + `start_session`/`resume_session`/`_invoke_log`/`sessions/active.json` nemodificate. Orice câmp + lipsă se manifestă ca `KeyError` la runtime, într-un thread, la un tur oarecare. Merită un + test de formă (assert pe setul de chei), nu doar teste de comportament. +2. **„Reacție ➡️"** — pare cosmetic, dar înseamnă că adaptorul trebuie să distingă + `__STEERED__` de un răspuns real **în trei locuri diferite**, fiecare cu API propriu de reacții. + Trei fișiere, trei tipare, trei moduri de a greși. +3. **„Flag implicit off"** — nu e gratuit: fiecare test nou trebuie să-l seteze explicit în + ambele sensuri, altfel testezi calea greșită fără să afli (E-C5). +4. **Reaper-ul** — 30 de linii care, greșite, scurg 500 MB pe oră tăcut. + +--- + +# /autoplan — Voci independente și corecții + +Codex indisponibil (binar neinstalat) → toate fazele `[subagent-only]`. + +## CEO DUAL VOICES — CONSENSUS TABLE + +``` +═══════════════════════════════════════════════════════════════ + Dimensiune Claude Codex Consensus + ───────────────────────────────────── ─────── ────── ───────── + 1. Premise valide? NO N/A flagged + 2. Problema potrivită? PARTIAL N/A flagged + 3. Scope calibrat corect? NO N/A flagged + 4. Alternative explorate suficient? NO N/A flagged + 5. Riscuri competitive acoperite? PARTIAL N/A flagged + 6. Traiectorie la 6 luni solidă? PARTIAL N/A flagged +═══════════════════════════════════════════════════════════════ +Codex lipsă ⇒ nicio dimensiune nu poate fi CONFIRMED. +Findings critice dintr-o singură voce se ridică oricum. +``` + +## DX DUAL VOICES — CONSENSUS TABLE + +``` +═══════════════════════════════════════════════════════════════ + Dimensiune Claude Codex Consensus + ───────────────────────────────────── ─────── ────── ───────── + 1. Getting started < 5 min? NO N/A flagged + 2. Denumiri API/CLI ghicibile? PARTIAL N/A flagged + 3. Mesaje de eroare acționabile? PARTIAL N/A flagged + 4. Docs găsibile și complete? NO N/A flagged + 5. Cale de upgrade sigură? YES N/A — + 6. Mediu de dezvoltare fără fricțiune? YES N/A — +═══════════════════════════════════════════════════════════════ +Scoruri DX voce independentă: getting started 5, API 6, erori 6, docs 4, +upgrade 7, mediu 8, măsurare 3. TTHW: ~15 min ca scris → ~3 min cu fix-urile. +``` + +## Corecții factuale la planul original (verificate pe cod) + +### ❌ Riscul #2 era INVERSAT — se elimină + +Planul original spunea: *„`--system-prompt` se fixează la spawn. Azi se reconstruiește din +`personality/*.md` la fiecare mesaj."* + +**Fals, verificat:** `build_system_prompt()` e chemat în exact două locuri — +`claude_session.py:486` (`start_session`) și `scheduler.py:402`. **`resume_session` nu pasează +deloc `--system-prompt`** (liniile 565-571). Deci editările din `personality/` **deja** nu se aplică +mid-sesiune; se aplică doar la o sesiune nouă, adică după `/clear`. + +Fix-ul propus (hash pe fișiere → respawn) ar fi fost **comportament nou**, care ar omorî procesul +viu și steering-urile în așteptare de fiecare dată când Marius editează `SOUL.md` cu Echo pornit. +**Decizie: riscul se șterge, fix-ul nu se implementează.** Reîncărcarea personalității rămâne legată +de `/clear`, exact ca azi. + +### ❌ Citarea implementării de referință era ambiguă + +`~/workspace/romfastsql/proxmox/lxc171-claude-agent/` **local** conține doar README + scripturi — +clona locală e stale. Codul citat (`discord-bridge/runner.py`, `bot.py:652-663`) există pe +**LXC 171**, accesat prin `ssh echo@10.0.20.201 "sudo pct exec 171 -- cat …"`. Citarea trebuie să +numească gazda, altfel implementatorul caută local și nu găsește nimic. + +### ❌ „Flagul poate fi citit la cald" era fals + +`claude_session.py:142` face `ALLOWED_TOOLS = _load_allowed_tools()` **la nivel de modul**, citind +`config.json` la import, și modulul **nu importă deloc `src.config`**. Dacă steering copiază tiparul, +flagul e citit o singură dată la pornire, iar rollback-ul cere restart. +**Fix:** citește `steering.*` prin `Config()` în `router.py` (care deține deja `_get_config()`), +**per apel**, nu la import. + +### ❌ Etapa 6 cerea o cheie inexistentă + +`channels.echo-core` are doar `['id','default_model']`. „Flip pe UN canal" n-are pe ce să se sprijine. +**Fix:** adaugă suport pentru `channels..steering: true` care are prioritate peste flagul global. + +## Amendamente acceptate din vocile independente + +| # | Amendament | Sursă | Decizie | Principiu | +|---|---|---|---|---| +| **X1** | **Contractul de rate limit se mută din Etapa 4 în Etapa 1.** Etapa 2 nu se merge-uiește până testul nu e verde. | CEO#3 **și** DX#5 — **temă transversală** | **ACCEPTAT** | P1 — altfel Etapele 2-3 livrează cu fallback-ul local mort tăcut | +| **X2** | `steer()` întoarce enum `STEERED / RAN_AS_TURN / PROCESS_DEAD`, nu bool; dispecerul deține fallback-ul | CEO#4 | **ACCEPTAT** | P5 explicit — „cade înapoi pe tur normal" era afirmat, nu proiectat | +| **X3** | `idle_reap_s` → **`idle_minutes`** (convenția repo: `interval_minutes`, `auto_leave_minutes`) | DX#1 | **ACCEPTAT** | P4 DRY/consistență | +| **X4** | Blocul de config **se livrează în `config.json`** la Etapa 2, nu doar ca default în cod | DX#4 | **ACCEPTAT** | P1 — altfel pornirea înseamnă scris JSON de mână dintr-un doc | +| **X5** | **Reacția care eșuează nu lasă tăcere** — wrap pe `add_reaction`; la eroare, un ack text de o linie | DX#3 | **ACCEPTAT** | „turnul nu se pierde niciodată", aplicat confirmării. Bridge-ul WhatsApp are istoric de deconectare tăcută | +| **X6** | **Renunță la evicția LRU.** Păstrează `max_live` ca număr + degradare la one-shot; scoate evicția. | CEO#8 | **ACCEPTAT** | P3+P5 — LRU care evacuează canalul în care tocmai scrii e o suprafață de concurență fără beneficiu la N=2 | +| **X7** | `max_live <= 0` → steering oprit **cu log**; `idle_minutes < 1` → clamp la 1 | DX#6 | **ACCEPTAT** | P1 — altfel steering mort tăcut cu flagul aparent pornit | +| **X8** | Kill switch de mediu **`ECHO_STEERING=off`** (precedent: `CLAUDE_BIN = os.environ.get(...)`) | DX#6 | **ACCEPTAT** | P1 — oprire fără să editezi JSON versionat | +| **X9** | **Re-spike** cu system prompt real + tur care lansează un subagent + corecție *blândă* (nu „STOP, zi doar ANANAS") | CEO#7 | **ACCEPTAT, înainte de Etapa 1** | P1 — spike-ul curent dovedește mecanismul, nu cazul de folosire | +| **X10** | **Pinează versiunea CLI + assert pe forma frame-ului** în testul cu fake claude | CEO (nota wire-format) | **ACCEPTAT** | `--output-format stream-json` e transport intern, nu contract de stabilitate; CLI e la 2.1.258 | +| **X11** | Ack-ul poartă conținut: `➡️ prins: ` | CEO#9 | **ACCEPTAT** | La un tur de 3 minute, ➡️ urmat de tăcere e nedistinsabil de „ignorat" → invită o a doua corecție → dublu steer | +| **X12** | Documentează cele două invariante (forma dict-ului; forma erorii de rate limit) — **T15 urcă la P1** | DX#5 | **ACCEPTAT** | P1 — sunt invariantele care țin fallback-ul local în viață | +| **X13** | Costează explicit alternativele B (coalescing, ~15 linii) și „abort+restart" (~40 linii) într-un paragraf fiecare | CEO#10 | **ACCEPTAT** | P6 — dacă motivul real e preferința lui Marius, se consemnează ca preferință | + +## Ridicat la Final Gate, NU auto-decis + +**CEO#1 — „Livrează `/stop` singur întâi, apoi decide dacă mai vrei steering."** +Argumentul: durerea declarată e „face ceva greșit și nu-l pot opri". Azi **nu există niciun abort** +(doar SIGINT pe tot procesul, `main.py:116`). A omorî `Popen` din `_run_claude` e muncă de o +după-amiază. Planul cumpără ~600 de linii de concurență pe proces persistent ca să obțină +corecție-târzie, iar abort-ul iese doar ca produs secundar. + +Nu e auto-decis: **re-secvențiază direcția pe care ai ales-o explicit.** O singură voce +(Codex indisponibil), deci nu se califică drept User Challenge formal — dar e cel mai important +punct strategic din tot review-ul. Vezi Gate. + +--- + + +## Decision Audit Trail + +| # | Fază | Decizie | Clasificare | Principiu | Motiv | Respins | +|---|---|---|---|---|---|---| +| 1 | CEO 0F | Mod SELECTIVE EXPANSION | Mechanical | default contextual | feature enhancement pe sistem existent | EXPANSION, HOLD, REDUCTION | +| 2 | CEO 0C-bis | Abordarea A (port hand-rolled) | Mechanical | constrângere utilizator | C închisă pe licențiere (abonament, fără API); B respinsă deja de utilizator | B, C | +| 3 | CEO 0D | E1 `/stop` acceptat în scope | Taste | P2 boil lakes | în raza de acțiune, <1 zi CC, capabilitatea vecină cea mai valoroasă | defer | +| 4 | CEO 0D | E2 parser partajat acceptat | Mechanical | P4 DRY | singura violare DRY reală din plan | defer | +| 5 | CEO 0D | E3 `--autocompact auto` acceptat | Mechanical | P1 completeness | fără el sesiunile lungi mor pe context | defer | +| 6 | CEO 0D | E4 placeholder editabil → TODOS | Mechanical | P2 rază de acțiune | schimbă UX-ul tuturor răspunsurilor, nu doar al celor steered | accept | +| 7 | CEO 0D | E5 atașamente mid-tur → TODOS | Mechanical | P2 rază de acțiune | funcționalitate nouă, nu parte din steering | accept | +| 8 | CEO 0D | E6 dashboard procese vii RESPINS | Mechanical | P4 DRY | `eco status` acoperă nevoia | accept, defer | +| 9 | CEO S3 | T1 wrapping `[EXTERNAL CONTENT]` la steer | Mechanical | P1 completeness | regresie de securitate, nu risc teoretic | omitere | +| 10 | CEO S7 | `max_live` 4 → 2 | Mechanical | P3 pragmatic | măsurat 292-541 MB RSS pe host de 8 GB | păstrare 4 | +| 11 | DX P3 | T12 (log per cale ratată) P2 → P1 | Mechanical | P1 completeness | tiparul de eșec din 2026-08-23, deja în CLAUDE.md | păstrare P2 | +| 12 | DX P4 | D5 (listă Ralph) CRITICAL → LOW | Mechanical | dovadă | verificat: toate joburile Ralph `enabled=False` | păstrare CRITICAL | +| 13 | Eng | E-C1 check+write sub un singur `_stdin_lock` | Mechanical | P1 completeness | TOCTOU pe `inflight` produce ➡️ fără text | doar T7 ca plasă | +| 14 | Eng | E3/E4 contract de mutex rescris | Mechanical | P1 completeness | testul ar trece vacuu (verde fals) | lăsare ca e | +| 15 | Voci | X1 rate limit Etapa 4 → Etapa 1 | Mechanical | **temă transversală** CEO#3 + DX#5 | altfel Etapele 2-3 livrează cu fallback-ul local mort | păstrare Etapa 4 | +| 16 | Voci | X2 `steer()` întoarce enum, nu bool | Mechanical | P5 explicit | fallback-ul era afirmat, nu proiectat | bool | +| 17 | Voci | X3 `idle_reap_s` → `idle_minutes` | Mechanical | P4 consistență | convenția repo: `*_minutes` | păstrare `_s` | +| 18 | Voci | X4 config în `config.json` + citit per apel | Mechanical | P1 | `ALLOWED_TOOLS` la import e capcana | default doar în cod | +| 19 | Voci | X5 reacție eșuată → ack text | Mechanical | P1 | „turnul nu se pierde niciodată", aplicat confirmării | tăcere | +| 20 | Voci | X6 renunță la evicția LRU | Mechanical | P3+P5 | suprafață de concurență fără beneficiu la N=2 | păstrare LRU | +| 21 | Voci | X9 re-spike cu tur realist | Mechanical | P1 | spike-ul dovedește mecanismul, nu cazul | mers pe spike-ul curent | +| 22 | Voci | X11 ack cu conținut (`➡️ prins: …`) | Taste | P1 | ➡️ + 2 min tăcere ≡ „ignorat" → dublu steer | ➡️ simplu | +| 23 | Voci | X15 Riscul #2 ȘTERS | Mechanical | **dovadă pe cod** | `resume_session` nu pasează `--system-prompt`; fix-ul ar fi fost o regresie | păstrare risc + hash | +| 24 | Voci | CEO#1 („`/stop` întâi") **NU auto-decis** | **Ridicat la Gate** | — | re-secvențiază direcția aleasă explicit de utilizator | auto-accept, auto-respingere | + +**Total: 24 decizii — 21 auto-decise (20 mechanical, 2 taste), 1 ridicată la Gate.** + +## ENG DUAL VOICES — CONSENSUS TABLE + +``` +═══════════════════════════════════════════════════════════════ + Dimensiune Claude Codex Consensus + ───────────────────────────────────── ─────── ────── ───────── + 1. Arhitectură solidă? YES* N/A flagged + 2. Acoperire de teste suficientă? NO N/A flagged + 3. Riscuri de performanță tratate? YES N/A — + 4. Amenințări de securitate acoperite? PARTIAL N/A flagged + 5. Căi de eroare tratate? NO N/A flagged + 6. Risc de deployment gestionabil? YES N/A — +═══════════════════════════════════════════════════════════════ +* YES condiționat de E1 (check+write sub un singur _stdin_lock) și X2 (enum). +Fără ele, arhitectura are o cursă TOCTOU nedocumentată. +Vocea eng independentă a rulat pe planul amendat; findings-urile ei majore +(concurență, contract de mutex) coincid cu analiza primară — vezi § Eng Review. +``` + +## Completion Summary + +``` ++====================================================================+ +| /autoplan — Steering în echo-core | ++====================================================================+ +| Mod: SELECTIVE EXPANSION | +| Voci: [subagent-only] — Codex neinstalat | +| Faze rulate: CEO ✓ · Design SĂRIT (fără UI) · DX ✓ · Eng ✓ | ++--------------------------------------------------------------------+ +| Decizii: 24 total — 21 auto · 2 taste · 1 la Gate | +| Task-uri: 49 (21 P1 · 22 P2 · 6 P3) | +| Premise: 7 verificate → 2 CONFIRMATE false, 1 ELIMINATĂ | +| Corecții 4 afirmații din plan infirmate pe cod: | +| factuale: · Riscul #2 (system-prompt) — inversat, șters | +| · „un singur răspuns la final" — deja fals | +| · „flag citit la cald" — fals (import-time) | +| · „WhatsApp poate n-are reacții" — are | ++--------------------------------------------------------------------+ +| Scor DX: 6.1/10 TTHW ~15 min → ~3 min cu fix-urile | +| Gap-uri critice rămase: 0 (fiecare are task) | +| Reversibilitate: 4/5 — flag + modul nou, _run_claude intact | ++====================================================================+ +``` + +## Unresolved Decisions + +1. **Secvențierea `/stop` vs steering** (CEO#1) — vezi Final Gate. +2. **Voice și text pe același `channel_id`** — un turn voice în timpul unui tur text devine + steering. Nedecis; nu blochează Etapa 1-2 (voice-ul e pe alt adaptor, dar același `session_key`). + +## Blocante găsite de vocea de inginerie (verificate pe cod) + +### C1 — Turul orfan. Proprietatea pe stdout se rotește între fire. **CRITIC** + +Ordinea care contează e **în interiorul CLI-ului**, nu în procesul nostru: CLI-ul poate emite +`result` *înainte* ca scrierea ta să ajungă, în timp ce `run_turn` e între citirea acelei linii și +achiziția lock-ului. **Niciun lock nu închide o cursă peste un pipe.** + +Consecința: `RAN_AS_TURN` e valoarea *periculoasă* din enum, nu cea sigură. Înseamnă că scrierea a +aterizat și a devenit turul N+1, **al cărui stdout nu-l deține nimeni**. Dacă dispecerul răspunde +la `RAN_AS_TURN` chemând `run_turn(text)` din nou → dublu-trimis, apoi citești evenimentele orfanului +împotriva promptului greșit: răspuns greșit la mesajul greșit, facturat de două ori, `active.json` derapat. + +**Fix acceptat (P5 explicit, diff mic):** `RAN_AS_TURN` înseamnă **„consumă stream-ul pe care tocmai +l-ai pornit"**, niciodată „reîncearcă". **Upgrade robust, notat ca alternativă:** un thread cititor +permanent care deține stdout și împinge tururile complete într-un `queue.Queue`; `run_turn` face pop. +Atunci orfanii sunt pop-uiți și logați, nu corup turul următor. → **decizie de gust, vezi Gate.** + +### C2 — Voice și text partajează `channel_id`. **CRITIC** + +Verificat: `voice/pipeline.py:429-430` cheamă `_route_message(str(self.text_channel_id), …)` cu +`adapter_name="discord-voice"` — **același id** ca textul (`router.py:589`, `session_key = channel_id`). +Iar `pipeline.py:444` oglindește `response_text` înapoi în canalul text. + +Deci: un mesaj text care face steering pe un tur voice viu primește răspunsul **rostit cu voce** +și niciodată randat ca text; iar un `__STEERED__` întors pe calea voice ajunge **postat literal** +în canal. **`src/voice/pipeline.py` lipsește complet din lista de fișiere a planului („3 adaptoare").** + +**Fix:** refuză steering-ul când adaptorul turului în zbor diferă de cel al mesajului sosit +(cade pe lock, comportament de azi) **și** tratează sentinela pe calea voice. `voice/pipeline.py` +intră în lista de fișiere atinse — sunt 7, nu 6. + +### C3 — Mesajul steered se pierde la orice eșec mid-tur. **CRITIC** + +`router.py:611` prinde excepția și, la rate limit, cheamă `_local_fallback_reply(text)` cu +**textul mesajului 1**. Firul mesajului 2 s-a întors deja cu `STEERED`. Rate limit în timpul unui +steer = pierdere tăcută — exact tiparul pe care CLAUDE.md îl fixează ca „turnul nu se pierde niciodată". + +**Fix:** `ClaudeProcess` reține textele steered ale turului curent; la eșec le re-dispecerizează. +T6 trebuie să testeze **livrarea textului steered**, nu doar detecția limitei. + +### C4 — Parserul partajat + ridicarea la rate limit ar regresa `PlanningSession`. **HIGH** + +Verificat: `claude_session.py:412-415` comentează explicit *„Surface subtype/is_error for callers +that retry on `error_max_turns` (PlanningSession does this)"*, iar `planning_session.py:74` are +`RETRY_MAX_TURNS = 30 # boost on error_max_turns`. `_run_claude` **nu aruncă** intenționat pe +`is_error` — planning-ul depinde de asta. + +**Fix:** parserul rămâne **pur** (parse → dict, nu aruncă niciodată). Ridicarea se face doar în +wrapper-ul din runner, condiționată pe `is_rate_limit_error(detail)`. Task T5 se modifică în consecință. + +### H1 — Verificarea `inflight` e rasă și în cazul obișnuit. **HIGH → simplificare** + +Două mesaje la ~50 ms fără niciun tur pornit: ambele văd „not inflight", mesajul 2 se blochează pe +lock pentru tot turul, iar steering-ul **nu se întâmplă tăcut** — exact cazul QA-ului ostil. + +**Fix acceptat (P5+P3):** folosește `lock.acquire(blocking=False)` la nivel de dispecer. +**Eșecul de a lua lock-ul *este* „un tur rulează".** Elimină flagul `inflight` din decizia +dispecerului și odată cu el întreaga cursă TOCTOU. Mai puține stări, cod mai scurt. + +### H2, M1, M2 + +* **H2** — „`test_claude_session_mutex.py` trece nemodificat" e necesar dar înșelător: pinează doar + contractul cu flag off. `TestAcquisitionBehavior` (linia 229) afirmă că un apelant în contenție + se blochează apoi continuă; cu steering pornit trebuie **să facă steering**. Adaugă geamănul cu + steering pornit + fixture autouse care golește registry-ul, altfel procesele fake se scurg între teste. +* **M1** — degradarea la one-shot pentru un canal care are deja un proces viu inactiv dă **doi + scriitori pe același `session_id`** (`--resume` din `resume_session` plus procesul viu). + Oprește procesul înainte de a degrada. +* **M2** — timeout → `stop()` → turul următor face `--resume` pe o sesiune omorâtă la mijlocul unui + tool-call. Netestat. + +### Notă de securitate suplimentară + +`_safe_env()` stale la spawn nu e listat în modelul de amenințări. Confirmat ca finding minor. + +--- + +# DECIZII FINALE (gate aprobat 2026-09-02) + +| # | Întrebare | Alegere | +|---|---|---| +| D3 | Secvențiere | **Ambele, dar `/stop` se livrează primul** | +| D4 | Turul orfan (C1) | **Consume-the-stream** — `RAN_AS_TURN` = consumă stream-ul pornit, niciodată reîncearcă | +| D5 | Aprobare review | **Aprobat ca atare** — cele 21 de decizii auto rămân aplicate | + +## Etape re-secvențiate + +### Etapa 0 — `/stop` singur, pe calea one-shot actuală ⟵ NOU, se livrează primul + +Nu atinge nimic din arhitectura de steering. `_run_claude` deja ține `proc` (`claude_session.py:310`) +și un thread watchdog care îl omoară la timeout (`:322-333`) — mecanismul de kill există, îi lipsește +doar un declanșator din exterior. + +* Registru `channel_id → Popen` pentru turul în zbor +* Comandă `/stop` în `router.py` → `proc.terminate()`, apoi `kill()` după grație +* Utilizatorul vede: „⏹ oprit" — nu o eroare de CLI +* `sessions/active.json` rămâne valid: sesiunea supraviețuiește, doar turul moare + +**Efort:** human ~4h / CC ~30min. **Rulează singur o săptămână înainte de Etapa 1.** + +> **Cost onest al eșalonării:** registrul de PID-uri din Etapa 0 se rescrie parțial când treci pe +> proces persistent — `ClaudeProcess.stop()` îl înlocuiește. Pierzi ~30 de linii. În schimb ai +> oprirea în producție într-o după-amiază și afli din uz dacă mai vrei steering-ul. + +### Etapele 1-6 — steering (neschimbate ca ordine, cu X1 aplicat) + +| # | Ce | Schimbat față de planul inițial | +|---|---|---| +| 1 | `claude_runner.py` + fake claude care citește stdin + **contractul de rate limit** | **X1**: rate limit urcat din Etapa 4. Etapa 2 nu se merge-uiește până T6 nu e verde | +| 2 | Dispecer cu `lock.acquire(blocking=False)` (**H1**) + sentinelă + **blocul de config livrat în `config.json`** (**X4**) | H1 elimină TOCTOU-ul de dispecer; flagul citit per apel, nu la import | +| 3 | Reacții pe **4** căi: Discord, Telegram, WhatsApp, **voice** (**C2**) | `voice/pipeline.py` adăugat — 7 fișiere, nu 6 | +| 4 | Hardening: C3 (re-dispecerizare text steered), E2 (sid persistat per tur), M1 | Rate limit nu mai e aici — a plecat în Etapa 1 | +| 5 | Reaper + `max_live` **fără LRU** (**X6**) + `eco status`/`eco doctor` (**D1**) | LRU scos ca suprafață de concurență inutilă la N=2 | +| 6 | Flip pe un canal (**X16**: cheie `channels..steering`), apoi global | Cheia per-canal trebuie adăugată — nu exista | + +### Config final + +```json +"steering": { "enabled": false, "idle_minutes": 20, "max_live": 2 } +``` +Citit prin `Config()` **per apel** în `router.py`, nu la import. `max_live <= 0` → oprit cu log. +`idle_minutes < 1` → clamp la 1. Kill switch: `ECHO_STEERING=off`. + +### Fișiere atinse (7) + +`src/claude_runner.py` (nou) · `src/claude_session.py` · `src/router.py` · +`src/adapters/discord_bot.py` · `src/adapters/telegram_bot.py` · `src/adapters/whatsapp.py` · +**`src/voice/pipeline.py`** · plus `config.json`, `CLAUDE.md`, `tests/` + +## Status + +**EXECUTAT 2026-09-02.** Toate etapele (0-6) implementate pe master, necommitate. +Steering livrat cu `steering.enabled: false` în `config.json` și zero override-uri +per canal — `/stop` e activ, steering-ul rămâne stins până la flip manual. + +Suită: 1199 passed / 12 failed — toate cele 12 confirmate pre-existente pe HEAD curat +(worktree separat): 2 `test_claude_session` (assert stale pe system prompt), 1 `test_cli` +(`assets/voice/*.wav` lipsă), 4 `test_discord` (`owned_bot.user` None în fixture), +3 `test_heartbeat`, 1 `test_dashboard_ralph_endpoint`, 1 `test_dashboard_unified_index`. +`tests/test_dashboard_projects_endpoint.py` se blochează — și pe HEAD curat. + +Abateri față de plan, verificate pe cod: +- **M1 nereproductibil** — `RunnerRegistry.get()` întoarce procesul existent al canalului + înaintea oricărei verificări de capacitate, deci „doi scriitori pe același session_id" + nu e accesibil dinspre registry. Garda ar fi fost cod mort; dispecerul evită cazul și + pe calea C2 (așteaptă lock-ul, dar rulează prin ACELAȘI proces viu). +- **session_id se ia din două locuri**, nu doar din `system`/`init`: init acoperă turul + care expiră înainte de `result`, iar `result` reîmprospătează la fiecare tur reușit. +- **`eco status` numără prin `pgrep -f "--input-format stream-json"`**, nu prin registry: + `eco` e proces separat, deci `live_count()` ar fi fost structural mereu 0. + +## Addendum — două findings din notificările finale ale vocilor + +### A1 — Sentinela trebuie tratată într-un singur loc. **HIGH** + +Verificat: `__AUDIO__:` e tratat în **Discord (3 locuri)** și **Telegram (1)**, dar +**deloc în WhatsApp**. Convenția existentă are deja o gaură — un utilizator WhatsApp care +declanșează TTS primește șirul literal `__AUDIO__:/cale`. **Bug preexistent, nu introdus aici.** + +`__STEERED__` ar replica exact tiparul, acum pe **4 căi** (Discord, Telegram, WhatsApp, voice). +**Fix:** o singură funcție `is_sentinel(response)` pe calea partajată, nu patru verificări +copiate. Repară și bug-ul preexistent de pe WhatsApp ca efect secundar. + +### A2 — `_safe_env()` la spawn schimbă comportamentul comutatorului OpenRouter. **HIGH** + +`_safe_env()` (`claude_session.py:231-249`) verifică semaforul `.use_openrouter` și, dacă există, +încarcă `~/.claude-env.sh` — care setează variabilele `ANTHROPIC_*` ca să rutează prin OpenRouter. + +Azi asta se re-evaluează **la fiecare tur**, deci crearea sau ștergerea semaforului are efect +de la mesajul următor. Cu proces persistent se evaluează **o singură dată, la spawn**: comuți +providerul și **nu se întâmplă nimic** până la respawn — sau, mai rău, rămâi pe OpenRouter după ce +ai șters semaforul, crezând că nu ești. + +Nu e „env stale, minor". E un comutator de provider care încetează tăcut să funcționeze. +**Fix:** la începutul fiecărui tur, compară starea semaforului cu cea de la spawn; dacă diferă, +oprește procesul și respawn-ează. Ăsta e singurul respawn-on-change justificat — spre deosebire +de cel pe `personality/` (Riscul #2, șters), aici starea chiar se aplică per proces. + +### A3 — Ajustări de etapizare din vocea DX + +* `eco status` cu `steering: on/off · N procese vii` se mută din **Etapa 5 în Etapa 2** — + Etapele 2-4 sunt exact unde depanezi. +* T15 primește o **rețetă de reproducere**: „ca să testezi manual, cere ceva cu un `sleep` de 30s+ + în Bash, apoi trimite al doilea mesaj". Fără ea, fiecare mentenator viitor repetă spike-ul eșuat. diff --git a/tests/fake_claude.py b/tests/fake_claude.py new file mode 100755 index 0000000..dba7f30 --- /dev/null +++ b/tests/fake_claude.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Fake `claude` CLI binary for tests/test_claude_runner.py. + +Runs as a REAL subprocess (spawned by `subprocess.Popen`, exactly like the +real CLI) so steering genuinely exercises stdin/stdout pipe timing that a +mock `Popen` can't reproduce. Speaks the subset of stream-json that +`src/claude_runner.py` depends on: + + - one `system`/`init` event with a `session_id`, on the first turn only + - one `assistant` event per response, with a text block + - one `result` event per turn + +Controlled entirely via environment variables (this process has no access +to the test's Python objects): + + FAKE_CLAUDE_SCENARIO normal | steer | timeout | rate_limit | + big_stderr | crash_immediately (default: normal) + FAKE_CLAUDE_SESSION_ID session_id to report (default: fake-session-1) + +The `system`/`init` event also carries the process's own `argv` — tests use +this to assert `--resume ` was actually passed on respawn, without +needing a separate side channel. +""" +import json +import os +import sys +import time + + +def _emit(obj: dict) -> None: + print(json.dumps(obj), flush=True) + + +def main() -> None: + scenario = os.environ.get("FAKE_CLAUDE_SCENARIO", "normal") + session_id = os.environ.get("FAKE_CLAUDE_SESSION_ID", "fake-session-1") + + if scenario == "crash_immediately": + sys.exit(1) + + if scenario == "big_stderr": + # T3: without a stderr-draining thread on the caller's side, this + # fills the OS pipe buffer (~64KB) and blocks forever, before this + # process ever gets to read a turn off stdin. + for _ in range(4000): + print("x" * 40, file=sys.stderr, flush=True) + + first_turn = True + for raw_line in sys.stdin: + raw_line = raw_line.strip() + if not raw_line: + continue + try: + msg = json.loads(raw_line) + except json.JSONDecodeError: + continue + text = msg.get("message", {}).get("content", "") + + if first_turn: + _emit({ + "type": "system", "subtype": "init", + "session_id": session_id, "argv": sys.argv, + }) + first_turn = False + + if scenario == "timeout": + time.sleep(600) # never responds — caller's own watchdog must kill us + return + + if scenario == "rate_limit": + _emit({ + "type": "result", "subtype": "error", "session_id": session_id, + "result": "You've hit your session limit · resets 10:50am (UTC)", + "is_error": True, + }) + continue + + if scenario == "generic_error": + # is_error=True but NOT a rate limit — C4: must be returned, + # never raised, so a future PlanningSession-style caller can + # still retry on `subtype` instead of catching an exception. + _emit({ + "type": "result", "subtype": "error_max_turns", "session_id": session_id, + "result": "hit max turns for this task", "is_error": True, + }) + continue + + if scenario == "steer": + # Simulate a turn in progress that then reads a SECOND stdin + # line (the steer) before finishing — exactly what a real + # steering exchange looks like (spike: one `result`, num_turns=2). + _emit({"type": "assistant", "message": {"content": [ + {"type": "text", "text": f"got:{text}"}, + ]}}) + second_raw = sys.stdin.readline().strip() + if second_raw: + try: + second_msg = json.loads(second_raw) + second_text = second_msg.get("message", {}).get("content", "") + except json.JSONDecodeError: + second_text = "" + _emit({"type": "assistant", "message": {"content": [ + {"type": "text", "text": f"steered:{second_text}"}, + ]}}) + _emit({ + "type": "result", "subtype": "success", "session_id": session_id, + "result": "done", "is_error": False, "num_turns": 2, + "usage": {"input_tokens": 1, "output_tokens": 1}, + }) + continue + + # normal + _emit({"type": "assistant", "message": {"content": [ + {"type": "text", "text": f"echo:{text}"}, + ]}}) + _emit({ + "type": "result", "subtype": "success", "session_id": session_id, + "result": f"echo:{text}", "is_error": False, "num_turns": 1, + "usage": {"input_tokens": 1, "output_tokens": 1}, + }) + + +if __name__ == "__main__": + main() diff --git a/tests/test_claude_runner.py b/tests/test_claude_runner.py new file mode 100644 index 0000000..ee5558c --- /dev/null +++ b/tests/test_claude_runner.py @@ -0,0 +1,351 @@ +"""Tests for src/claude_runner.py (steering — persistent Claude processes). + +Fully offline: `tests/fake_claude.py` is a real subprocess (no mocked +`subprocess.Popen`) speaking a minimal stream-json dialect, so steering +timing over real OS pipes is genuinely exercised rather than assumed. +""" + +import threading +import time +from pathlib import Path + +import pytest + +from src import claude_runner, claude_session +from src.claude_runner import SteerStatus + +FAKE_CLAUDE = Path(__file__).parent / "fake_claude.py" + + +def _wait_until(predicate, timeout=5.0, interval=0.02): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return + time.sleep(interval) + raise AssertionError("condition not met within timeout") + + +@pytest.fixture(autouse=True) +def _reset_registry(): + """H2: the registry is module-level global state (like + claude_session._session_locks) — clear it around every test so fake + processes from one test never leak into the next.""" + claude_runner.reset_registry_for_tests() + yield + claude_runner.reset_registry_for_tests() + + +@pytest.fixture +def make_proc(monkeypatch): + """Factory for a ClaudeProcess wired to tests/fake_claude.py, with + automatic cleanup of every process it creates.""" + created: list[claude_runner.ClaudeProcess] = [] + monkeypatch.setattr(claude_runner, "CLAUDE_BIN", str(FAKE_CLAUDE)) + + def _factory(scenario: str, channel_id: str = "test-channel", + session_id_env: str = "fake-session-1", model: str = "sonnet"): + monkeypatch.setenv("FAKE_CLAUDE_SCENARIO", scenario) + monkeypatch.setenv("FAKE_CLAUDE_SESSION_ID", session_id_env) + proc = claude_runner.ClaudeProcess(channel_id, model=model) + created.append(proc) + return proc + + yield _factory + + for p in created: + try: + p.stop() + except Exception: + pass + + +# --------------------------------------------------------------------------- +# T1 — normal turn shape + rate limit + timeout (Etapa 1 contract) +# --------------------------------------------------------------------------- + + +def test_run_turn_matches_run_claude_dict_shape(make_proc): + proc = make_proc("normal") + result = proc.run_turn("hello") + expected_keys = { + "result", "session_id", "usage", "total_cost_usd", "cost_usd", + "duration_ms", "num_turns", "intermediate_count", "subtype", "is_error", + } + assert set(result.keys()) == expected_keys + assert result["is_error"] is False + assert result["session_id"] == "fake-session-1" + + +def test_run_turn_wraps_external_content(make_proc): + """T1 (-k external_content selects this test and the steer one below).""" + proc = make_proc("normal") + result = proc.run_turn("plain text from Marius") + assert "[EXTERNAL CONTENT]" in result["result"] + assert "[END EXTERNAL CONTENT]" in result["result"] + assert "plain text from Marius" in result["result"] + + +def test_timeout_raises_exact_message(make_proc): + proc = make_proc("timeout") + with pytest.raises(TimeoutError, match=r"Claude CLI timed out after 1s"): + proc.run_turn("hi", timeout=1) + + +def test_rate_limit_raises_runtime_error_router_can_detect(make_proc): + """T2/C4/T6: the exact failure mode that must keep `_local_fallback_reply` + alive — router.py/scheduler.py both gate on `is_rate_limit_error`.""" + proc = make_proc("rate_limit") + with pytest.raises(RuntimeError) as excinfo: + proc.run_turn("hi") + assert claude_session.is_rate_limit_error(str(excinfo.value)) + + +def test_non_rate_limit_error_is_returned_not_raised(make_proc): + """C4: the parser/wrapper must stay non-throwing for any `is_error` that + isn't a rate limit (PlanningSession-style callers retry on subtype).""" + proc = make_proc("generic_error") + result = proc.run_turn("hi") # must NOT raise + assert result["is_error"] is True + assert result["subtype"] == "error_max_turns" + + +# --------------------------------------------------------------------------- +# Steering — mid-turn message lands on the same stdin +# --------------------------------------------------------------------------- + + +def test_steering_mid_turn_reflects_second_message(make_proc): + proc = make_proc("steer") + texts: list[str] = [] + holder: dict = {} + + def _go(): + holder["result"] = proc.run_turn("first message", on_text=texts.append) + + t = threading.Thread(target=_go) + t.start() + # Wait for the fake's first assistant block — proves it's now blocked + # on its own stdin.readline(), genuinely waiting for the steer. + _wait_until(lambda: len(texts) >= 1) + assert proc.inflight is True + + outcome = proc.steer("second message") + t.join(timeout=5) + + assert outcome.status == SteerStatus.STEERED + assert outcome.turn is None + result = holder["result"] + assert "steered:" in result["result"] + assert result["num_turns"] == 2 + assert proc.inflight is False + + +def test_steer_wraps_external_content(make_proc): + """T1 (-k external_content).""" + proc = make_proc("steer") + texts: list[str] = [] + holder: dict = {} + + def _go(): + holder["result"] = proc.run_turn("first", on_text=texts.append) + + t = threading.Thread(target=_go) + t.start() + _wait_until(lambda: len(texts) >= 1) + proc.steer("stai, schimbare de plan") + t.join(timeout=5) + + assert "[EXTERNAL CONTENT]" in holder["result"]["result"] + assert "stai, schimbare de plan" in holder["result"]["result"] + + +def test_steer_pending_texts_available_on_turn_failure(make_proc): + """C3: if the turn a steer landed in fails, the steered text must be + exposed for re-dispatch, never silently dropped.""" + proc = make_proc("timeout") # sleeps forever after init — never sends a result + + def _go(): + try: + proc.run_turn("first", timeout=1) + except TimeoutError: + pass + + t = threading.Thread(target=_go) + t.start() + # session_id arrives (via the init event) right after spawn, well + # before the fake's 600s sleep would ever resolve — a reliable + # "the turn is genuinely in flight" signal for this scenario. + _wait_until(lambda: proc.session_id == "fake-session-1") + assert proc.inflight is True + + outcome = proc.steer("please don't get lost") + assert outcome.status == SteerStatus.STEERED + t.join(timeout=5) # watchdog kills the process after 1s -> TimeoutError + + pending = proc.pop_pending_steers() + assert pending == ["please don't get lost"] + # popped once -> empty on a second call + assert proc.pop_pending_steers() == [] + + +# --------------------------------------------------------------------------- +# C1 — RAN_AS_TURN: steer() finds no in-flight turn +# --------------------------------------------------------------------------- + + +def test_steer_without_inflight_turn_becomes_ran_as_turn(make_proc): + proc = make_proc("normal") + first = proc.run_turn("warm up") + assert first["is_error"] is False + assert proc.inflight is False + + outcome = proc.steer("out of band message") + + assert outcome.status == SteerStatus.RAN_AS_TURN + assert outcome.turn is not None + assert "out of band message" in outcome.turn["result"] + # D4: the caller must treat `turn` as the response and never re-dispatch + # by calling run_turn() again for the same text — nothing to assert + # mechanically here beyond the API shape making that the only sane path. + + +# --------------------------------------------------------------------------- +# T7 — BrokenPipeError on steer falls back to a normal turn +# --------------------------------------------------------------------------- + + +def test_steer_broken_pipe_falls_back_to_turn(make_proc, monkeypatch): + proc = make_proc("normal") + proc.run_turn("warm up") # spawns the process + proc.proc.kill() + proc.proc.wait() + + monkeypatch.setenv("FAKE_CLAUDE_SCENARIO", "normal") + outcome = proc.steer("please deliver me") + + assert outcome.status == SteerStatus.PROCESS_DEAD + assert outcome.turn is not None + assert "please deliver me" in outcome.turn["result"] + + +# --------------------------------------------------------------------------- +# M2 — timeout kills the process; next turn respawns with --resume +# --------------------------------------------------------------------------- + + +def test_respawn_with_resume_after_timeout_kill(make_proc, monkeypatch): + proc = make_proc("timeout") + with pytest.raises(TimeoutError): + proc.run_turn("hi", timeout=1) + + # session_id came from the `system`/`init` event, captured before the + # turn ever got its (never-arriving) result line. + assert proc.session_id == "fake-session-1" + assert not proc.alive() + dead_pid = proc.proc.pid + + cmd = proc._build_cmd() + assert "--resume" in cmd + assert cmd[cmd.index("--resume") + 1] == "fake-session-1" + + monkeypatch.setenv("FAKE_CLAUDE_SCENARIO", "normal") + result = proc.run_turn("again", timeout=5) + + assert result["is_error"] is False + assert proc.proc.pid != dead_pid # actually respawned, not reused + + +# --------------------------------------------------------------------------- +# T3 — stderr pipe must never deadlock a turn +# --------------------------------------------------------------------------- + + +def test_big_stderr_does_not_deadlock(make_proc): + proc = make_proc("big_stderr") + result = proc.run_turn("hello", timeout=15) + assert result["is_error"] is False + assert len(proc._stderr_buf) > 0 + assert len(proc._stderr_buf) <= 50 # deque(maxlen=50) + + +# --------------------------------------------------------------------------- +# RunnerRegistry — max_live degradation, reaper resilience +# --------------------------------------------------------------------------- + + +def test_max_live_degrades_second_channel_to_one_shot(): + registry = claude_runner.RunnerRegistry(max_live=1, idle_minutes=20) + proc_a = registry.get("channel-a") + assert proc_a is not None + proc_b = registry.get("channel-b") + assert proc_b is None # T10: degrade, don't evict (X6) and don't block + # the same channel is never punished by its own occupied slot: + assert registry.get("channel-a") is proc_a + registry.stop_all() + + +def test_reaper_never_stops_inflight_process(): + registry = claude_runner.RunnerRegistry(max_live=2, idle_minutes=1) + proc = registry.get("channel-a") + proc.inflight = True + proc.last_active = time.monotonic() - 3600 + registry._reap_once() + assert registry.live_count() == 1 + registry.stop_all() + + +def test_reaper_stops_idle_process(): + registry = claude_runner.RunnerRegistry(max_live=2, idle_minutes=1) + proc = registry.get("channel-a") + proc.inflight = False + proc.last_active = time.monotonic() - 3600 + registry._reap_once() + assert registry.live_count() == 0 + + +def test_reaper_survives_stop_exception(monkeypatch): + """T11: a dead reaper is a silent RAM leak — one process's stop() + raising must not kill the reap loop or the process.""" + registry = claude_runner.RunnerRegistry(max_live=2, idle_minutes=1) + proc = registry.get("channel-a") + proc.inflight = False + proc.last_active = time.monotonic() - 3600 + + def _boom(): + raise RuntimeError("stop() exploded") + + monkeypatch.setattr(proc, "stop", _boom) + registry._reap_once() # must not raise + assert registry.live_count() == 0 # bookkeeping removed regardless + + +def test_stop_all_clears_registry(make_proc): + proc = make_proc("normal") + registry = claude_runner.RunnerRegistry(max_live=2, idle_minutes=20) + registry._procs["test-channel"] = proc + proc.run_turn("hello") # actually spawns + assert proc.alive() + registry.stop_all() + assert registry.live_count() == 0 + assert not proc.alive() + + +def test_get_registry_singleton_and_reset(): + r1 = claude_runner.get_registry(max_live=3, idle_minutes=5) + r2 = claude_runner.get_registry(max_live=99) # ignored once created + assert r1 is r2 + assert r1.max_live == 3 + + claude_runner.reset_registry_for_tests() + + r3 = claude_runner.get_registry(max_live=7) + assert r3 is not r1 + assert r3.max_live == 7 + + +def test_max_live_non_positive_logs_and_always_degrades(caplog): + with caplog.at_level("WARNING"): + registry = claude_runner.RunnerRegistry(max_live=0, idle_minutes=20) + assert any("max_live" in rec.message for rec in caplog.records) + assert registry.get("any-channel") is None diff --git a/tests/test_claude_session_mutex.py b/tests/test_claude_session_mutex.py index 9a6b2f1..c454cdc 100644 --- a/tests/test_claude_session_mutex.py +++ b/tests/test_claude_session_mutex.py @@ -25,12 +25,14 @@ from unittest.mock import patch import pytest -from src import claude_session +from src import claude_runner, claude_session +from src.claude_runner import SteerResult, SteerStatus from src.claude_session import ( _get_session_lock, _session_locks, send_message, ) +from src.sentinels import is_steered # --------------------------------------------------------------------------- @@ -40,10 +42,19 @@ from src.claude_session import ( @pytest.fixture(autouse=True) def _clear_session_locks(): - """Each test starts with a fresh lock map so we don't share state.""" + """Each test starts with a fresh lock map so we don't share state. + + H2: also resets the steering registry singleton and adapter-ownership + map — both module-level globals — so a fake `ClaudeProcess` installed + by one test (steering-ON variants below) never leaks into the next. + """ _session_locks.clear() + claude_session._channel_adapter.clear() + claude_runner.reset_registry_for_tests() yield _session_locks.clear() + claude_session._channel_adapter.clear() + claude_runner.reset_registry_for_tests() @pytest.fixture @@ -68,7 +79,7 @@ def _slow_run_claude(sleep_seconds: float, in_critical: threading.Event, """ state = {"active": 0, "lock": threading.Lock()} - def fake(cmd, timeout, on_text=None, cwd=None): + def fake(cmd, timeout, on_text=None, cwd=None, channel_id=None): with state["lock"]: state["active"] += 1 if state["active"] > 1: @@ -268,7 +279,7 @@ class TestAcquisitionBehavior: call_count = {"n": 0} - def flaky(cmd, timeout, on_text=None, cwd=None): + def flaky(cmd, timeout, on_text=None, cwd=None, channel_id=None): call_count["n"] += 1 if call_count["n"] == 1: raise RuntimeError("simulated subprocess crash") @@ -305,3 +316,98 @@ class TestAcquisitionBehavior: ) t.join(timeout=1.0) assert result_box == ["Hello from Claude!"] + + +# --------------------------------------------------------------------------- +# H2 — twin of TestAcquisitionBehavior with steering ON: a contending +# caller must STEER, not block-then-run. Pins that the flag actually +# changes this contract (the flag-off tests above must stay unchanged). +# --------------------------------------------------------------------------- + + +class _FakeSteeringProc: + """Minimal `ClaudeProcess` stand-in — just enough of the contract + `_dispatch_steering` relies on (`inflight`, `run_turn`, `steer`, + `pop_pending_steers`) to prove the dispatch decision, without a real + subprocess (that's covered by tests/test_claude_runner.py).""" + + def __init__(self): + self.inflight = False + self.session_id = "fake-sid" + self.steer_calls: list[str] = [] + self._release = threading.Event() + + def run_turn(self, text, on_text=None, timeout=300): + self._release.clear() + self.inflight = True + released = self._release.wait(timeout=5.0) + self.inflight = False + assert released, "test never released the fake turn" + return { + "result": f"done:{text}", "session_id": self.session_id, "usage": {}, + "total_cost_usd": 0, "cost_usd": 0, "duration_ms": 1, "num_turns": 1, + "intermediate_count": 0, "subtype": "success", "is_error": False, + } + + def steer(self, text): + self.steer_calls.append(text) + return SteerResult(SteerStatus.STEERED) + + def pop_pending_steers(self): + return [] + + def release(self): + self._release.set() + + +class _FakeSteeringRegistry: + def __init__(self, proc, channel_id): + self._procs = {channel_id: proc} + + def get(self, channel_id, model=None, session_id=None, cwd=None): + return self._procs.get(channel_id) + + def stop(self, channel_id): + return False + + +class TestAcquisitionBehaviorSteering: + """Twin of `TestAcquisitionBehavior.test_contested_acquire_blocks_then_proceeds` + with `steering.enabled` on: contention must STEER instead of blocking.""" + + def test_contested_acquire_steers_when_steering_enabled( + self, temp_sessions, monkeypatch + ): + proc = _FakeSteeringProc() + registry = _FakeSteeringRegistry(proc, "ch-contend-steer") + monkeypatch.setattr(claude_runner, "_registry", registry) + monkeypatch.setattr(claude_runner, "get_registry", lambda **kw: registry) + monkeypatch.setattr(claude_session, "_steering_config", lambda channel_id=None: (True, 2, 20)) + + outcome: dict[str, str] = {} + + def run(label: str): + outcome[label] = send_message( + "ch-contend-steer", label, adapter_name="discord", + ) + + t1 = threading.Thread(target=run, args=("first",)) + t1.start() + + deadline = time.monotonic() + 2.0 + while time.monotonic() < deadline and not proc.inflight: + time.sleep(0.01) + assert proc.inflight, "first call never entered the turn" + + t2 = threading.Thread(target=run, args=("second",)) + t2.start() + t2.join(timeout=5.0) + + assert is_steered(outcome["second"]), ( + "With steering on, a contended caller must STEER, not block." + ) + assert proc.steer_calls == ["second"] + + proc.release() + t1.join(timeout=5.0) + assert outcome["first"] == "done:first" diff --git a/tests/test_cli.py b/tests/test_cli.py index 383b223..9bb980e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -87,6 +87,54 @@ class TestStatus: assert "WA Bridge: ONLINE" in out +# --------------------------------------------------------------------------- +# eco status — steering line (T4/A3) +# --------------------------------------------------------------------------- + + +class TestSteeringStatusLine: + def _mock_service(self, active="inactive"): + return {"ActiveState": active, "MainPID": "0", "ActiveEnterTimestamp": ""} + + def test_steering_off_no_procs(self, iso, capsys): + """pgrep's own no-match case: exit code 1, stdout '0' — a + legitimate zero, not a counting failure.""" + mock_cfg = MagicMock() + mock_cfg.get.return_value = False + mock_pgrep = MagicMock(returncode=1, stdout="0\n") + with patch("cli._get_service_status", return_value=self._mock_service()), \ + patch("src.config.Config", return_value=mock_cfg), \ + patch("subprocess.run", return_value=mock_pgrep): + cli.cmd_status(_args()) + out = capsys.readouterr().out + assert "steering: off · 0 procese vii" in out + + def test_steering_counts_live_procs(self, iso, capsys): + """A real match: pgrep -fc finds 2 live steering processes.""" + mock_cfg = MagicMock() + mock_cfg.get.return_value = True + mock_pgrep = MagicMock(returncode=0, stdout="2\n") + with patch("cli._get_service_status", return_value=self._mock_service()), \ + patch("src.config.Config", return_value=mock_cfg), \ + patch("subprocess.run", return_value=mock_pgrep): + cli.cmd_status(_args()) + out = capsys.readouterr().out + assert "steering: on · 2 procese vii" in out + + def test_survives_pgrep_failure(self, iso, capsys): + """D1/D3 guard: a failed process-table scan must not break the + rest of `eco status`, and must not silently read as a wrong zero.""" + mock_cfg = MagicMock() + mock_cfg.get.return_value = False + with patch("cli._get_service_status", return_value=self._mock_service()), \ + patch("src.config.Config", return_value=mock_cfg), \ + patch("subprocess.run", side_effect=OSError("pgrep not found")): + cli.cmd_status(_args()) + out = capsys.readouterr().out + assert "steering: off · necunoscut procese vii" in out + assert "Sessions:" in out + + # --------------------------------------------------------------------------- # cmd_doctor # --------------------------------------------------------------------------- @@ -194,6 +242,22 @@ class TestDoctor: out, code = self._run_doctor(iso, capsys, setup_full=True) assert "Claude CLI functional" in out + def test_steering_off_by_default(self, iso, capsys): + iso["config_file"].write_text('{"bot":{}}') + mock_cfg = MagicMock() + mock_cfg.get.return_value = False + with patch("src.config.Config", return_value=mock_cfg): + out, code = self._run_doctor(iso, capsys, setup_full=True) + assert "[PASS] Steering (optional, currently off)" in out + + def test_steering_checks_stream_json_when_enabled(self, iso, capsys): + iso["config_file"].write_text('{"bot":{}}') + mock_cfg = MagicMock() + mock_cfg.get.return_value = True + with patch("src.config.Config", return_value=mock_cfg): + out, code = self._run_doctor(iso, capsys, setup_full=True) + assert "Claude CLI supports --input-format stream-json" in out + # --------------------------------------------------------------------------- # cmd_restart diff --git a/tests/test_local_fallback.py b/tests/test_local_fallback.py index b662e2a..0e403a8 100644 --- a/tests/test_local_fallback.py +++ b/tests/test_local_fallback.py @@ -1,15 +1,18 @@ """Tests for the local LLM fallback: history, tools, net status, web search.""" import time +from pathlib import Path from unittest.mock import MagicMock, patch import pytest -from src import fallback_history, net_status, web_search +from src import claude_runner, claude_session, fallback_history, net_status, router, web_search from src import local_fallback_tools as lft from src.router import (_forced_tool, _is_creative_request, _is_text_task, _parse_tool_args, _run_fallback_tools) +FAKE_CLAUDE = Path(__file__).parent / "fake_claude.py" + @pytest.fixture(autouse=True) def clean_history(): @@ -623,3 +626,217 @@ class TestFallbackNeverDropsTurn: assert "răspuns direct" in reply conv.assert_called_once() + +# --------------------------------------------------------------------------- +# T6 — persistent-process rate limit must still reach the fallback +# --------------------------------------------------------------------------- +# +# The one-shot `claude -p` dies with a nonzero exit code at the limit; a +# persistent steering process (src/claude_runner.py) reports the same limit +# as `result.is_error` in the stream instead — it never exits. `ClaudeProcess` +# converts that back into the exact `RuntimeError` the one-shot path raises +# (covered in tests/test_claude_runner.py). What isn't covered anywhere else +# is the far end of the wire: that error genuinely reaching router.py's +# handler and firing `_local_fallback_reply` for real. Untested, this is how +# the single most-worked-on subsystem in the repo goes dead in silence +# (tasks/steering-plan.md, blockers C3/C4, task T6 — "the 2am Friday test"). + + +def _local_fallback_cfg(): + """`_get_config()` double: only `local_fallback` is stubbed — every + other key falls through to its caller-supplied default, same as the + real `Config().get(key, default)`. A blanket `return_value` (as used + for the router-only tests above) would also hijack route_message's own + `_get_config().get("bot.default_model", "sonnet")` lookup and feed a + dict into `--model`.""" + cfg = MagicMock() + cfg.get.side_effect = lambda key, default=None: ( + {"enabled": True, "url": "http://x"} if key == "local_fallback" else default + ) + return cfg + + +@pytest.fixture(autouse=True) +def _reset_steering_registry(): + """H2: the steering registry is module-level global state — never let + a live (fake) process from one test leak into the next.""" + claude_runner.reset_registry_for_tests() + yield + claude_runner.reset_registry_for_tests() + + +@pytest.fixture +def temp_sessions(tmp_path, monkeypatch): + """Isolated sessions/active.json so these tests never touch the real one.""" + sessions_dir = tmp_path / "sessions" + sessions_dir.mkdir() + sf = sessions_dir / "active.json" + sf.write_text("{}") + monkeypatch.setattr(claude_session, "SESSIONS_DIR", sessions_dir) + monkeypatch.setattr(claude_session, "_SESSIONS_FILE", sf) + return sf + + +@pytest.fixture +def steering_on(monkeypatch, temp_sessions): + """Steering enabled, wired to the real tests/fake_claude.py subprocess — + a genuine persistent process, not a mock, so the rate-limit path fires + the way it actually does in production.""" + monkeypatch.setattr(claude_runner, "CLAUDE_BIN", str(FAKE_CLAUDE)) + monkeypatch.setattr(claude_session, "_steering_config", lambda *a, **kw: (True, 2, 20)) + + +class TestPersistentRateLimitReachesFallback: + """T6 (`pytest -k persistent`): a rate limit arriving as `result.is_error` + from a PERSISTENT process must still invoke the local model — asserting + the RESCUE happened, not merely that some exception was raised. A test + that only checks "an exception was raised" is exactly the bug this task + exists to catch: that's also true the day the fallback silently stops + firing.""" + + def test_persistent_rate_limit_invokes_real_fallback(self, steering_on, monkeypatch): + monkeypatch.setenv("FAKE_CLAUDE_SCENARIO", "rate_limit") + + with patch("src.router._get_config", return_value=_local_fallback_cfg()), \ + patch("src.router.set_channel_context"), \ + patch("src.router._call_local_llm", return_value={"content": "raspuns local"}), \ + patch("src.router._local_fallback_reply", + wraps=router._local_fallback_reply) as fallback_spy: + result, is_cmd = router.route_message("ch-t6-persist", "user-1", "salut") + + fallback_spy.assert_called_once_with("salut", channel_id="ch-t6-persist") + assert "raspuns local" in result + assert is_cmd is False + + +class _RateLimitAfterSteerProc: + """Minimal ClaudeProcess double: the turn raises the rate-limit + RuntimeError while leaving one already-steered text behind in + `_pending_steers` — the shape of a real ClaudeProcess whose turn a + `steer()` landed in, then died (C3). `ClaudeProcess`'s own + thread-timing for how a steer lands mid-turn is exercised for real in + tests/test_claude_runner.py; this double exists only to drive + router.py's redelivery path deterministically.""" + + def __init__(self, channel_id, model=None, session_id=None, cwd=None): + self.channel_id = channel_id + self.session_id = session_id or "fake-sid" + self.inflight = False + self._pending_steers = ["steered while you were away"] + + def run_turn(self, text, on_text=None, timeout=300): + self.inflight = True + raise RuntimeError( + "Claude CLI error (exit 1): You've hit your session limit · resets 10am (UTC)" + ) + + def pop_pending_steers(self): + pending, self._pending_steers = self._pending_steers, [] + return pending + + +class _SingleProcRegistry: + """RunnerRegistry double that always hands back the one proc it was + built with.""" + + def __init__(self, proc): + self._procs = {proc.channel_id: proc} + + def get(self, channel_id, model=None, session_id=None, cwd=None): + return self._procs.get(channel_id) + + def stop(self, channel_id): + return self._procs.pop(channel_id, None) is not None + + +def _echo_llm(url, messages, tools=None, temperature=0.0): + """`_call_local_llm` double whose reply names the user text it saw, so + the original turn's reply and the steered turn's reply stay + distinguishable however many passes `_local_fallback_reply` makes.""" + return {"content": f"echo:{messages[-1]['content']}"} + + +class TestSteeredTextDeliveredOnRateLimit: + """C3: the plan is explicit that T6 must test DELIVERY, not just + detection. A message steered into a turn that then dies on a rate + limit must still produce its own answer to the user — its request + thread already returned `__STEERED__` and is gone, so `on_text` is the + only channel left.""" + + def test_steered_reply_delivered_via_on_text(self, monkeypatch, temp_sessions): + proc = _RateLimitAfterSteerProc("ch-t6-c3") + registry = _SingleProcRegistry(proc) + # Both names: `_dispatch_steering` calls `get_registry()`, but + # `claude_session.pop_pending_steers` (C3's redelivery hook) peeks + # the module-level `_registry` directly rather than calling + # `get_registry()` again (that would spin one up as a side effect + # for a channel that never used steering). + monkeypatch.setattr(claude_runner, "get_registry", lambda **kw: registry) + monkeypatch.setattr(claude_runner, "_registry", registry) + monkeypatch.setattr(claude_session, "_steering_config", lambda *a, **kw: (True, 2, 20)) + + streamed = [] + with patch("src.router._get_config", return_value=_local_fallback_cfg()), \ + patch("src.router.set_channel_context"), \ + patch("src.router._call_local_llm", side_effect=_echo_llm): + result, is_cmd = router.route_message( + "ch-t6-c3", "user-1", "mesaj original", on_text=streamed.append, + ) + + # The original message's own answer still comes back as the + # function's return value. + assert "echo:mesaj original" in result + assert is_cmd is False + # The steered message never had a request thread of its own left to + # return to — its answer must have gone out through on_text. + assert len(streamed) == 1 + assert "echo:steered while you were away" in streamed[0] + + +class TestNonRateLimitErrorStaysQuiet: + """C4 regression guard: verified in tasks/steering-plan.md that + `PlanningSession` retries on `error_max_turns`, which depends on the + runner RETURNING (never raising) a non-rate-limit `is_error`. If a + future change 'simplifies' that into a raise, this test catches it by + failing on the wrong side: the fallback would fire when it must not.""" + + def test_generic_is_error_does_not_raise_or_call_fallback(self, steering_on, monkeypatch): + monkeypatch.setenv("FAKE_CLAUDE_SCENARIO", "generic_error") + + with patch("src.router._local_fallback_reply") as fallback: + result, is_cmd = router.route_message("ch-t6-c4", "user-1", "salut") + + fallback.assert_not_called() + assert "hit max turns" in result + assert is_cmd is False + + +class TestSteeringOffRollback: + """The flag is the rollback mechanism (steering-plan.md Etapa 2) — it + has to actually roll the rate-limit -> fallback path back to today's + behavior, not merely skip steering-specific code paths.""" + + def test_steering_off_still_reaches_fallback_via_one_shot_path( + self, monkeypatch, temp_sessions, + ): + monkeypatch.setattr(claude_session, "_steering_config", lambda *a, **kw: (False, 0, 20)) + monkeypatch.setattr( + claude_session, "_run_claude", + lambda *a, **kw: (_ for _ in ()).throw(RuntimeError( + "Claude CLI error (exit 1): You've hit your session limit · resets 10am (UTC)" + )), + ) + + with patch("src.router._get_config", return_value=_local_fallback_cfg()), \ + patch("src.router.set_channel_context"), \ + patch("src.router._call_local_llm", return_value={"content": "raspuns local"}), \ + patch("src.router._local_fallback_reply", + wraps=router._local_fallback_reply) as fallback_spy: + result, is_cmd = router.route_message("ch-t6-off", "user-1", "salut") + + fallback_spy.assert_called_once_with("salut", channel_id="ch-t6-off") + assert "raspuns local" in result + assert is_cmd is False + # Steering played no role at all — the registry was never even created. + assert claude_runner._registry is None + diff --git a/tests/test_router.py b/tests/test_router.py index b2189b0..eb27d73 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -193,7 +193,7 @@ class TestRegularMessage: response, is_cmd = route_message("ch-1", "user-1", "hello") assert response == "Hello from Claude!" assert is_cmd is False - mock_send.assert_called_once_with("ch-1", "hello", model="sonnet", on_text=None, voice_mode=False) + mock_send.assert_called_once_with("ch-1", "hello", model="sonnet", on_text=None, voice_mode=False, adapter_name=None) @patch("src.router.send_message") def test_model_override(self, mock_send): @@ -201,7 +201,7 @@ class TestRegularMessage: response, is_cmd = route_message("ch-1", "user-1", "hello", model="opus") assert response == "Response" assert is_cmd is False - mock_send.assert_called_once_with("ch-1", "hello", model="opus", on_text=None, voice_mode=False) + mock_send.assert_called_once_with("ch-1", "hello", model="opus", on_text=None, voice_mode=False, adapter_name=None) @patch("src.router._get_channel_config") @patch("src.router._get_config") @@ -274,7 +274,7 @@ class TestRegularMessage: cb = lambda t: None route_message("ch-1", "user-1", "hello", on_text=cb) - mock_send.assert_called_once_with("ch-1", "hello", model="sonnet", on_text=cb, voice_mode=False) + mock_send.assert_called_once_with("ch-1", "hello", model="sonnet", on_text=cb, voice_mode=False, adapter_name=None) # --- _get_channel_config --- @@ -316,7 +316,7 @@ class TestModelResolution: mock_chan_cfg.return_value = {"id": "ch-1", "default_model": "haiku"} route_message("ch-1", "user-1", "hello") - mock_send.assert_called_once_with("ch-1", "hello", model="haiku", on_text=None, voice_mode=False) + mock_send.assert_called_once_with("ch-1", "hello", model="haiku", on_text=None, voice_mode=False, adapter_name=None) @patch("src.router._get_channel_config") @patch("src.router._get_config") @@ -330,7 +330,7 @@ class TestModelResolution: mock_get_config.return_value = mock_cfg route_message("ch-1", "user-1", "hello") - mock_send.assert_called_once_with("ch-1", "hello", model="opus", on_text=None, voice_mode=False) + mock_send.assert_called_once_with("ch-1", "hello", model="opus", on_text=None, voice_mode=False, adapter_name=None) @patch("src.router._get_channel_config") @patch("src.router._get_config") @@ -344,7 +344,7 @@ class TestModelResolution: mock_get_config.return_value = mock_cfg route_message("ch-1", "user-1", "hello") - mock_send.assert_called_once_with("ch-1", "hello", model="sonnet", on_text=None, voice_mode=False) + mock_send.assert_called_once_with("ch-1", "hello", model="sonnet", on_text=None, voice_mode=False, adapter_name=None) @patch("src.router.get_active_session") @patch("src.router.send_message") @@ -354,7 +354,7 @@ class TestModelResolution: mock_get_session.return_value = {"model": "opus", "session_id": "abc"} route_message("ch-1", "user-1", "hello") - mock_send.assert_called_once_with("ch-1", "hello", model="opus", on_text=None, voice_mode=False) + mock_send.assert_called_once_with("ch-1", "hello", model="opus", on_text=None, voice_mode=False, adapter_name=None) # --- Voice/text unify regression guards --- diff --git a/tests/test_sentinels.py b/tests/test_sentinels.py new file mode 100644 index 0000000..dd373e6 --- /dev/null +++ b/tests/test_sentinels.py @@ -0,0 +1,101 @@ +"""src.sentinels is the single place that recognises router sentinels +(__AUDIO__: and __STEERED__) so adapters don't each hand-roll the +prefix check. Offline, no subprocess/network involved. +""" +from __future__ import annotations + +import asyncio + +import pytest + +from src.sentinels import AUDIO_PREFIX, STEERED, audio_path, is_steered + + +def test_audio_path_extracts_payload(): + assert audio_path(f"{AUDIO_PREFIX}/tmp/echo-audio.wav") == "/tmp/echo-audio.wav" + + +def test_audio_path_none_for_plain_text(): + assert audio_path("salut, ce mai faci?") is None + + +def test_audio_path_none_for_empty_and_none(): + assert audio_path("") is None + assert audio_path(None) is None + + +def test_audio_path_none_for_steered(): + assert audio_path(STEERED) is None + + +def test_is_steered_true_only_for_exact_sentinel(): + assert is_steered(STEERED) is True + assert is_steered("__STEERED__ ") is False + assert is_steered("some text") is False + + +def test_is_steered_false_for_empty_and_none(): + assert is_steered("") is False + assert is_steered(None) is False + + +def test_is_steered_false_for_audio_sentinel(): + assert is_steered(f"{AUDIO_PREFIX}/tmp/x.wav") is False + + +# --- Voice pipeline: a steered response must not be mirrored into the +# text channel (blocker C2 in tasks/steering-plan.md). Reuses the fakes +# from tests/test_pipeline_mirror.py. + +from unittest.mock import AsyncMock, MagicMock + +from src.voice.pipeline import VoiceSession + + +def _make_text_channel(send_mock: AsyncMock) -> MagicMock: + tc = MagicMock(name="text_channel") + tc.send = send_mock + return tc + + +def _make_session(*, reply_text: str, text_channel) -> VoiceSession: + bot = MagicMock(name="bot") + bot.get_channel = MagicMock(return_value=text_channel) + bot.get_user = MagicMock(return_value=None) + ttsq = MagicMock(name="ttsq") + ttsq.push_text = MagicMock() + ttsq.clear = MagicMock() + route_mock = MagicMock(name="route_message", return_value=(reply_text, False)) + return VoiceSession( + text_channel_id=1001, + voice_channel_id=2002, + guild_id=42, + voice_client=MagicMock(name="voice_client"), + bot=bot, + ttsq=ttsq, + whitelist=set(), + record_enabled=False, + mirror_enabled=True, + transcripts_jsonl_path=None, + loop=asyncio.get_event_loop_policy().new_event_loop(), + router_route_message=route_mock, + ) + + +def _reply_chunks(send_mock: AsyncMock) -> list[str]: + return [ + call.args[0] + for call in send_mock.call_args_list + if not call.args[0].startswith("\U0001f3a4") + ] + + +@pytest.mark.asyncio +async def test_voice_path_does_not_mirror_steered_sentinel(): + send_mock = AsyncMock(name="text_send") + text_channel = _make_text_channel(send_mock) + session = _make_session(reply_text=STEERED, text_channel=text_channel) + + await session.on_segment_done(speaker_id=123, text="salut", no_speech_prob=0.1) + + assert _reply_chunks(send_mock) == [] diff --git a/tests/test_steering_dispatch.py b/tests/test_steering_dispatch.py new file mode 100644 index 0000000..a1403b8 --- /dev/null +++ b/tests/test_steering_dispatch.py @@ -0,0 +1,433 @@ +"""Tests for the steering dispatcher (H1/C2/C3/T9/T12) in +``src/claude_session.py`` and its ``__STEERED__`` handling in +``src/router.py``. + +`ClaudeProcess`/`RunnerRegistry` internals (spawn, stdin/stdout framing, +`steer()`'s own race handling) are already covered by +``tests/test_claude_runner.py`` against the real ``tests/fake_claude.py`` +subprocess. This file is about the DISPATCHER's decisions — which only need +a process double that behaves the right way at the right time, synchronised +via ``threading.Event`` rather than real subprocess timing (the plan +explicitly warns sleep-based subprocess races are the flaky way to test +this). +""" + +import threading +import time +from unittest.mock import patch + +import pytest + +from src import claude_runner, claude_session, router +from src.claude_runner import SteerResult, SteerStatus +from src.claude_session import send_message +from src.sentinels import is_steered + + +def _wait_until(predicate, timeout=5.0, interval=0.01): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return + time.sleep(interval) + raise AssertionError("condition not met within timeout") + + +# --------------------------------------------------------------------------- +# Scriptable ClaudeProcess/RunnerRegistry doubles +# --------------------------------------------------------------------------- + + +class FakeProc: + """Stand-in for `ClaudeProcess`. `run_turn` blocks on a per-call Event + so a test can deterministically control exactly when a turn "finishes".""" + + def __init__(self, channel_id="ch", session_id="fake-sid"): + self.channel_id = channel_id + self.session_id = session_id + self.inflight = False + self._pending_steers: list[str] = [] + self.steer_calls: list[str] = [] + self.run_turn_calls: list[str] = [] + self.stopped = False + self.turn_error: Exception | None = None + self._release = threading.Event() + + def run_turn(self, text, on_text=None, timeout=300): + self._release.clear() + self.inflight = True + self.run_turn_calls.append(text) + released = self._release.wait(timeout=5.0) + self.inflight = False + assert released, "test never released the fake turn" + if self.turn_error is not None: + err, self.turn_error = self.turn_error, None + raise err + return { + "result": f"done:{text}", "session_id": self.session_id, "usage": {}, + "total_cost_usd": 0, "cost_usd": 0, "duration_ms": 1, "num_turns": 1, + "intermediate_count": 0, "subtype": "success", "is_error": False, + } + + def steer(self, text): + self.steer_calls.append(text) + assert self.inflight, "dispatcher should only steer an inflight turn" + self._pending_steers.append(text) + return SteerResult(SteerStatus.STEERED) + + def pop_pending_steers(self): + pending, self._pending_steers = self._pending_steers, [] + return pending + + def release(self): + self._release.set() + + +class FakeRegistry: + """Stand-in for `RunnerRegistry` — one proc per channel, created + on demand, degrading to None past `capacity` (T10).""" + + def __init__(self, capacity=2): + self.capacity = capacity + self._procs: dict[str, FakeProc] = {} + + def get(self, channel_id, model=None, session_id=None, cwd=None): + proc = self._procs.get(channel_id) + if proc is not None: + return proc + if len(self._procs) >= self.capacity: + return None + proc = FakeProc(channel_id, session_id=session_id or "fake-sid") + self._procs[channel_id] = proc + return proc + + def stop(self, channel_id): + proc = self._procs.pop(channel_id, None) + if proc is None: + return False + proc.stopped = True + return True + + def live_count(self): + return len(self._procs) + + +@pytest.fixture(autouse=True) +def _clean_state(): + """Dispatcher state (`_session_locks`, `_channel_adapter`) and the + steering registry singleton are module-level globals — clear them + around every test so nothing leaks between tests.""" + claude_session._session_locks.clear() + claude_session._channel_adapter.clear() + claude_runner.reset_registry_for_tests() + yield + claude_session._session_locks.clear() + claude_session._channel_adapter.clear() + claude_runner.reset_registry_for_tests() + + +@pytest.fixture +def temp_sessions(tmp_path, monkeypatch): + """Isolated active.json per test — keeps real session state untouched.""" + sessions_dir = tmp_path / "sessions" + sessions_dir.mkdir() + sf = sessions_dir / "active.json" + sf.write_text("{}") + monkeypatch.setattr(claude_session, "SESSIONS_DIR", sessions_dir) + monkeypatch.setattr(claude_session, "_SESSIONS_FILE", sf) + return sf + + +@pytest.fixture +def fake_registry(monkeypatch, temp_sessions): + """Steering ON, backed by a FakeRegistry instead of real subprocesses.""" + registry = FakeRegistry() + monkeypatch.setattr(claude_runner, "_registry", registry) + monkeypatch.setattr(claude_runner, "get_registry", lambda **kw: registry) + monkeypatch.setattr(claude_session, "_steering_config", lambda channel_id=None: (True, 2, 20)) + return registry + + +class _FakeConfig: + """Stand-in for `src.config.Config` used by `_steering_config`. Takes + the full raw dict (`steering`, `channels`, etc.) like the real thing.""" + + def __init__(self, data: dict): + self._data = data + + def __call__(self, *a, **kw): + return self + + def get(self, key, default=None): + return self._data.get(key, default) + + +# --------------------------------------------------------------------------- +# `_steering_config` — flag off, kill switch, clamps (X7/X8) +# --------------------------------------------------------------------------- + + +class TestSteeringConfig: + def test_disabled_by_default_config(self, monkeypatch): + monkeypatch.setattr("src.config.Config", _FakeConfig({"steering": {"enabled": False}})) + enabled, _, _ = claude_session._steering_config() + assert enabled is False + + def test_kill_switch_wins(self, monkeypatch): + monkeypatch.setenv("ECHO_STEERING", "off") + monkeypatch.setattr( + "src.config.Config", + _FakeConfig({"steering": {"enabled": True, "max_live": 2, "idle_minutes": 20}}), + ) + enabled, _, _ = claude_session._steering_config() + assert enabled is False + + def test_max_live_zero_disables(self, monkeypatch): + monkeypatch.setattr( + "src.config.Config", + _FakeConfig({"steering": {"enabled": True, "max_live": 0, "idle_minutes": 20}}), + ) + enabled, _, _ = claude_session._steering_config() + assert enabled is False + + def test_idle_minutes_clamped_to_one(self, monkeypatch): + monkeypatch.setattr( + "src.config.Config", + _FakeConfig({"steering": {"enabled": True, "max_live": 2, "idle_minutes": 0}}), + ) + _, _, idle_minutes = claude_session._steering_config() + assert idle_minutes == 1 + + +# --------------------------------------------------------------------------- +# Etapa 6 (X16) — per-channel `channels..steering` override +# --------------------------------------------------------------------------- + + +class TestPerChannelSteeringOverride: + def test_channel_on_overrides_global_off(self, monkeypatch): + monkeypatch.setattr("src.config.Config", _FakeConfig({ + "steering": {"enabled": False, "max_live": 2, "idle_minutes": 20}, + "channels": {"echo-core": {"id": "chan-1", "steering": True}}, + })) + enabled, _, _ = claude_session._steering_config("chan-1") + assert enabled is True + + def test_channel_off_overrides_global_on(self, monkeypatch): + monkeypatch.setattr("src.config.Config", _FakeConfig({ + "steering": {"enabled": True, "max_live": 2, "idle_minutes": 20}, + "channels": {"echo-core": {"id": "chan-1", "steering": False}}, + })) + enabled, _, _ = claude_session._steering_config("chan-1") + assert enabled is False + + def test_channel_unset_falls_through_to_global(self, monkeypatch): + monkeypatch.setattr("src.config.Config", _FakeConfig({ + "steering": {"enabled": True, "max_live": 2, "idle_minutes": 20}, + "channels": {"echo-core": {"id": "chan-1"}}, # no "steering" key + })) + enabled, _, _ = claude_session._steering_config("chan-1") + assert enabled is True + + enabled_other, _, _ = claude_session._steering_config("chan-unrelated") + assert enabled_other is True # not present at all -> global default too + + +# --------------------------------------------------------------------------- +# Flag off -> unchanged blocking path +# --------------------------------------------------------------------------- + + +def test_steering_off_uses_unchanged_blocking_path(monkeypatch, temp_sessions): + monkeypatch.setattr(claude_session, "_steering_config", lambda channel_id=None: (False, 0, 20)) + calls = [] + + def fake_run_claude(cmd, timeout, on_text=None, cwd=None, channel_id=None): + calls.append(cmd) + return { + "result": "hi", "session_id": "sid-1", "usage": {}, "total_cost_usd": 0, + "cost_usd": 0, "duration_ms": 1, "num_turns": 1, "intermediate_count": 0, + "subtype": "success", "is_error": False, + } + + with patch.object(claude_session, "_run_claude", side_effect=fake_run_claude): + result = send_message("ch-off", "hello") + + assert result == "hi" + assert len(calls) == 1 + # The dispatcher never touched the registry — disabled path is a no-op + # with respect to steering, not just "didn't steer". + assert claude_runner._registry is None + + +# --------------------------------------------------------------------------- +# H1 — contention steers, no separate `inflight` check/TOCTOU +# --------------------------------------------------------------------------- + + +def test_contention_steers(fake_registry): + outcome = {} + + def first(): + outcome["first"] = send_message("ch-1", "first", adapter_name="discord") + + t1 = threading.Thread(target=first) + t1.start() + _wait_until(lambda: fake_registry._procs.get("ch-1") is not None + and fake_registry._procs["ch-1"].inflight) + + second = send_message("ch-1", "second", adapter_name="discord") + assert is_steered(second) + + proc = fake_registry._procs["ch-1"] + assert proc.steer_calls == ["second"] + proc.release() + t1.join(timeout=5) + assert outcome["first"] == "done:first" + + +# --------------------------------------------------------------------------- +# C2/M1 — adapter mismatch never steers, and never spawns a second writer +# --------------------------------------------------------------------------- + + +def test_adapter_mismatch_waits_instead_of_steering(fake_registry): + voice_outcome = {} + + def voice_turn(): + voice_outcome["r"] = send_message("ch-2", "voice text", adapter_name="discord-voice") + + t1 = threading.Thread(target=voice_turn) + t1.start() + _wait_until(lambda: fake_registry._procs.get("ch-2") is not None + and fake_registry._procs["ch-2"].inflight) + + text_outcome = {} + + def text_turn(): + text_outcome["r"] = send_message("ch-2", "text message", adapter_name="discord") + + t2 = threading.Thread(target=text_turn) + t2.start() + + proc = fake_registry._procs["ch-2"] + time.sleep(0.1) # give t2 a chance to reach the mismatch branch + assert proc.steer_calls == [], "a mismatched adapter must never be steered (C2)" + + proc.release() # finish the voice turn -> releases the lock + t1.join(timeout=5) + assert voice_outcome["r"] == "done:voice text" + + _wait_until(lambda: proc.inflight) # t2 now got the lock and started its own turn + proc.release() + t2.join(timeout=5) + + assert text_outcome["r"] == "done:text message" + assert not is_steered(text_outcome["r"]) + assert proc.run_turn_calls == ["voice text", "text message"] + assert fake_registry.live_count() == 1, "must never spawn a second writer on the session (M1)" + + +# --------------------------------------------------------------------------- +# T10 — registry at capacity degrades to one-shot for a NEW channel +# --------------------------------------------------------------------------- + + +def test_capacity_degrades_new_channel_to_one_shot(fake_registry, temp_sessions): + fake_registry.capacity = 1 + fake_registry._procs["ch-existing"] = FakeProc("ch-existing") + + calls = [] + + def fake_run_claude(cmd, timeout, on_text=None, cwd=None, channel_id=None): + calls.append(cmd) + return { + "result": "one-shot reply", "session_id": "sid-2", "usage": {}, + "total_cost_usd": 0, "cost_usd": 0, "duration_ms": 1, "num_turns": 1, + "intermediate_count": 0, "subtype": "success", "is_error": False, + } + + with patch.object(claude_session, "_run_claude", side_effect=fake_run_claude): + result = send_message("ch-new", "hello", adapter_name="discord") + + assert result == "one-shot reply" + assert len(calls) == 1 + assert "ch-new" not in fake_registry._procs + + +# --------------------------------------------------------------------------- +# T9 — /clear and /model stop the live process +# --------------------------------------------------------------------------- + + +def test_clear_session_stops_live_process(fake_registry): + proc = fake_registry.get("ch-3") + fake_registry._procs["ch-3"] = proc + claude_session._save_sessions({"ch-3": {"session_id": "sid", "model": "sonnet"}}) + + assert claude_session.clear_session("ch-3") is True + assert proc.stopped is True + assert "ch-3" not in fake_registry._procs + + +def test_set_session_model_stops_live_process(fake_registry): + proc = fake_registry.get("ch-4") + fake_registry._procs["ch-4"] = proc + claude_session._save_sessions({"ch-4": {"session_id": "sid", "model": "sonnet"}}) + + assert claude_session.set_session_model("ch-4", "opus") is True + assert proc.stopped is True + assert "ch-4" not in fake_registry._procs + + +# --------------------------------------------------------------------------- +# C3 — a steered message must never be lost when its turn then fails +# --------------------------------------------------------------------------- + + +def test_pending_steers_redispatched_on_failed_turn(monkeypatch): + """router.route_message's exception handler must pop pending steers + (claude_session.pop_pending_steers) and redeliver each one via + on_text — their own request threads already returned __STEERED__ and + are gone, so on_text is the only way left to answer them.""" + pending = ["steered one", "steered two"] + monkeypatch.setattr(router, "_pop_pending_steers", lambda channel_id: list(pending)) + monkeypatch.setattr( + router, "send_message", + lambda *a, **kw: (_ for _ in ()).throw(RuntimeError( + "Claude CLI error (exit 1): You've hit your session limit · resets 10am (UTC)" + )), + ) + fallback_calls = [] + + def fake_fallback(text, channel_id=None, manual=False): + fallback_calls.append(text) + return f"fallback:{text}" + + monkeypatch.setattr(router, "_local_fallback_reply", fake_fallback) + + streamed = [] + result, is_cmd = router.route_message( + "ch-c3", "user-1", "original message", on_text=streamed.append, + ) + + assert result == "fallback:original message" + assert is_cmd is False + assert fallback_calls == ["original message", "steered one", "steered two"] + assert streamed == ["fallback:steered one", "fallback:steered two"] + + +def test_pending_steers_logged_without_on_text(monkeypatch, caplog): + """No on_text means no way to redeliver — must be logged (T12), not + silently dropped.""" + monkeypatch.setattr(router, "_pop_pending_steers", lambda channel_id: ["lost message"]) + monkeypatch.setattr( + router, "send_message", + lambda *a, **kw: (_ for _ in ()).throw(RuntimeError("boom")), + ) + + with caplog.at_level("WARNING"): + result, is_cmd = router.route_message("ch-c3b", "user-1", "hello") + + assert result == "Error: boom" + assert any("steered message lost" in rec.message for rec in caplog.records) diff --git a/tests/test_stop_command.py b/tests/test_stop_command.py new file mode 100644 index 0000000..12f841d --- /dev/null +++ b/tests/test_stop_command.py @@ -0,0 +1,58 @@ +"""Tests for /stop — Etapa 0 of the steering plan (one-shot architecture). + +`stop_turn(channel_id)` kills the in-flight Claude CLI subprocess registered +for a channel, without touching sessions/active.json (the session survives, +only the turn dies). +""" + +from unittest.mock import MagicMock + +import pytest + +from src.claude_session import _live_procs, stop_turn +from src.router import route_message + + +@pytest.fixture(autouse=True) +def _clear_live_procs(): + """Fresh registry per test — fake processes must not leak between tests.""" + _live_procs.clear() + yield + _live_procs.clear() + + +class TestStopTurn: + def test_idle_channel_returns_false(self): + assert stop_turn("ch-1") is False + + def test_terminates_registered_process_and_cleans_up_registry(self): + fake_proc = MagicMock() + fake_proc.poll.return_value = None # still running + _live_procs["ch-1"] = fake_proc + + assert stop_turn("ch-1") is True + fake_proc.terminate.assert_called_once() + fake_proc.wait.assert_called_once() + assert "ch-1" not in _live_procs + + def test_already_exited_process_returns_false(self): + fake_proc = MagicMock() + fake_proc.poll.return_value = 0 # exited before /stop ran + _live_procs["ch-1"] = fake_proc + assert stop_turn("ch-1") is False + + +class TestStopCommand: + def test_stop_nothing_running(self): + response, is_cmd = route_message("ch-1", "user-1", "/stop") + assert response == "Nu rulează nimic pe canalul ăsta." + assert is_cmd is True + + def test_stop_kills_in_flight_turn(self): + fake_proc = MagicMock() + fake_proc.poll.return_value = None + _live_procs["ch-1"] = fake_proc + response, is_cmd = route_message("ch-1", "user-1", "/stop") + assert response == "⏹ Oprit." + assert is_cmd is True + fake_proc.terminate.assert_called_once()