test: restored tests exercise their claim instead of passing vacuously

Independent review of #120220 found restored tests that could pass without
reaching the code they name. Each fix below was A/B'd against a sabotaged
production line (red) and the real one (green).

- test_clarify_progress_leak: the fake agent slept 2x0.35 s as its only sync.
  It now waits (<= 5 s) for the progress task's first bubble; the queue is
  FIFO, so a leaked clarify bubble is what lands first. Reverting the clarify
  skip in run_turn_runner goes red on the leaked JSON.
- test_desktop_lifecycle_windows_live holder-scan case: the kanban row was
  rebuilt after the serve kill, re-reading the dead pid; NoSuchProcess was
  swallowed into an empty scan and returned False without reaching the
  classifier. Rows are now snapshotted while both processes live; a
  classifier that accepts everything goes red (the old version stayed green).
- *_windows_live (control socket, desktop lifecycle, plan reconciliation,
  shim fail-closed): fixed time.sleep settles become bounded waits on the
  real condition (argv visible to psutil, pipe gone, handle released) and
  child readline() calls get a 60 s bound.
- test_gateway_proc_fallback BSD ps case: drop the is_windows stub and mark
  it linux_only like its neighbour; a revert to `ps -A eww` goes red.
- test_delivery_ledger_single_connection counts only connections to the
  ledger's own database, not every sqlite3.connect in the process; pruning on
  a second connection goes red.
- test_codex_models: the hardcoded gpt-5.6 list becomes a relationship to
  DEFAULT_CODEX_MODELS and the forward-compat table; adding a -pro slug to
  either goes red.
- group-chat-view inline-code: the renderer stub wrapped the code in .aui-md,
  which the generic inline-code rule themes on its own, so the room's
  data-slot rule was never tested. Without the wrapper, renaming the room's
  data-slot goes red.
This commit is contained in:
teknium1
2026-09-23 08:35:24 -07:00
committed by Teknium
parent 88e45a48b6
commit 76e08ebea5
9 changed files with 193 additions and 63 deletions

View File

@@ -8,14 +8,12 @@ import { afterEach, beforeAll, expect, it, vi } from 'vitest'
import { translateBots } from './i18n-test-helper'
// Room bodies render through the shell's message renderer, whose markdown
// root is `aui-md prose` — the same container the 1:1 chat uses, minus the
// `[data-slot='aui_assistant-message-content']` ancestor that scopes the
// themed inline-code rule in styles.css. Without a room-owned hook the room's
// `<code>` falls through to Tailwind Typography's fixed near-black ink, which
// is invisible on every dark theme (#114086). The stubs below emit the two
// shapes the room can produce (renderer path, raw Streamdown fallback) so the
// real stylesheet's cascade decides, not a regex over the source text.
// Without a room-owned hook the room's `<code>` falls through to Tailwind
// Typography's fixed near-black ink, which is invisible on every dark theme
// (#114086). The renderer stub emits a bare `<code>` with NO `.aui-md`
// wrapper, so only the room's own `[data-slot='group-chat-message-content']`
// rule in the real stylesheet can theme it: the cascade decides, not a regex
// over the source text.
vi.mock('@hermes/plugin-sdk', async () => {
const { pluginSdkMock, createGroupGateway } = await import('./group-test-utils')
const base = await pluginSdkMock(createGroupGateway().host)
@@ -43,11 +41,9 @@ vi.mock('@hermes/plugin-sdk', async () => {
DialogTitle: () => null,
Input: () => null,
MessageTextContent: ({ text }: { text: string }) => (
<div className="aui-md prose">
<p>
set <code data-testid="renderer-code">{text}</code> first
</p>
</div>
<p>
set <code data-testid="renderer-code">{text}</code> first
</p>
),
Tip: ({ children }: { children: ReactNode }) => children,
relativeTime: () => 'now',

View File

@@ -12,7 +12,7 @@ the rendered interactive prompt on Slack.
import importlib
import sys
import time
import threading
import types
import pytest
@@ -29,6 +29,13 @@ class ProgressCaptureAdapter(BasePlatformAdapter):
super().__init__(PlatformConfig(enabled=True, token="***"), platform)
self.sent = []
self.edits = []
# Set on the first send/edit. The fake agent (on an executor thread)
# waits on it, so the turn cannot end before the progress task has
# rendered its first bubble.
self.delivered = threading.Event()
def _record(self, content):
self.delivered.set()
async def connect(self, *, is_reconnect: bool = False) -> bool:
return True
@@ -38,10 +45,12 @@ class ProgressCaptureAdapter(BasePlatformAdapter):
async def send(self, chat_id, content, reply_to=None, metadata=None) -> SendResult:
self.sent.append({"chat_id": chat_id, "content": content})
self._record(content)
return SendResult(success=True, message_id="m-1")
async def edit_message(self, chat_id, message_id, content) -> SendResult:
self.edits.append({"chat_id": chat_id, "message_id": message_id, "content": content})
self._record(content)
return SendResult(success=True, message_id=message_id)
async def send_typing(self, chat_id, metadata=None) -> None:
@@ -55,7 +64,15 @@ class ProgressCaptureAdapter(BasePlatformAdapter):
class ClarifyThenToolAgent:
"""Emits a clarify tool.started (with raw args) then a normal tool."""
"""Emits a clarify tool.started (with raw args) then a normal tool, and
returns only once the progress task has delivered its first bubble.
Both events are queued before that wait, and the queue drains FIFO, so the
first bubble is the clarify one if clarify leaks and the terminal one if
not. A timeout means nothing drained and fails the test.
"""
adapter = None
def __init__(self, **kwargs):
self.tool_progress_callback = kwargs.get("tool_progress_callback")
@@ -70,9 +87,9 @@ class ClarifyThenToolAgent:
"Which environment?",
{"question": "Which environment?", "choices": ["staging", "production"]},
)
time.sleep(0.35)
cb("tool.started", "terminal", "pwd", {})
time.sleep(0.35)
if not type(self).adapter.delivered.wait(timeout=5.0):
raise AssertionError("progress task never delivered a bubble")
return {"final_response": "done", "messages": [], "api_calls": 1}
@@ -127,6 +144,7 @@ async def test_clarify_tool_never_renders_progress_bubble(monkeypatch, tmp_path,
"""
adapter = ProgressCaptureAdapter()
runner = _make_runner(adapter)
monkeypatch.setattr(ClarifyThenToolAgent, "adapter", adapter)
gateway_run = _install_fakes(monkeypatch, mode)
monkeypatch.setattr(gateway_run, "_hermes_home", tmp_path)
@@ -152,5 +170,6 @@ async def test_clarify_tool_never_renders_progress_bubble(monkeypatch, tmp_path,
# No clarify progress line at all (verb "Asking" / tool name).
assert "clarify" not in all_content
assert "Asking" not in all_content
# The unrelated terminal tool still renders progress normally.
# The unrelated terminal tool still renders progress normally (and proves
# the no-leak asserts above ran against a drained queue).
assert "pwd" in all_content

View File

@@ -21,8 +21,10 @@ from __future__ import annotations
import json
import os
import queue
import subprocess
import sys
import threading
import time
from pathlib import Path
@@ -32,6 +34,35 @@ pytestmark = pytest.mark.windows_only
PROJECT_ROOT = Path(__file__).resolve().parents[2]
def _wait_until(predicate, timeout: float = 15.0, interval: float = 0.05) -> bool:
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if predicate():
return True
time.sleep(interval)
return bool(predicate())
def _readline(stream, timeout: float = 60.0) -> str:
"""``stream.readline()`` bounded by *timeout* (a hung child fails, not hangs)."""
got: queue.Queue = queue.Queue()
threading.Thread(target=lambda: got.put(stream.readline()), daemon=True).start()
try:
return got.get(timeout=timeout)
except queue.Empty:
return ""
def _argv_visible(pid: int, marker: str) -> bool:
"""True once the process table shows *pid* with *marker* in its argv."""
import psutil
try:
return marker in " ".join(psutil.Process(pid).cmdline())
except (psutil.NoSuchProcess, psutil.AccessDenied):
return False
_CHILD_CODE = r"""
import asyncio, os, sys
sys.path.insert(0, sys.argv[1])
@@ -65,10 +96,10 @@ def live_server(tmp_path: Path):
text=True,
cwd=str(PROJECT_ROOT),
)
line = proc.stdout.readline().strip()
line = _readline(proc.stdout).strip()
if not line.startswith("SERVER_STARTED"):
err = proc.stderr.read() if proc.poll() is not None else ""
proc.kill()
_kill_tree(proc)
pytest.fail(f"pipe server child failed to start: {line!r} {err}")
server_pid = int(line.split()[1])
yield proc, home, server_pid
@@ -126,9 +157,10 @@ def test_pipe_gone_after_kill_falls_back(live_server, monkeypatch):
assert identify_gateway(home, timeout=5.0) is not None
_kill_tree(proc)
time.sleep(0.5)
assert identify_gateway(home, timeout=2.0) is None
# taskkill /T returns once the tree is signalled; the pipe disappears when
# the kernel tears the server's handles down.
assert _wait_until(lambda: identify_gateway(home, timeout=0.5) is None)
# Consumer falls back to the state file. That file is a claim, not an
# identity: its sha classifies a row only when live_gateway_pid_for_home
@@ -163,8 +195,7 @@ def test_pipe_gone_after_kill_falls_back(live_server, monkeypatch):
stderr=subprocess.DEVNULL,
)
try:
time.sleep(0.5)
assert standin.poll() is None
assert _wait_until(lambda: _argv_visible(standin.pid, "gateway")), "stand-in argv never visible"
_write_state(standin.pid)
fleet = ur.collect_fleet_versions()
assert len(fleet) == 1, fleet

View File

@@ -7,17 +7,23 @@ second connection opened after the first one closed.
from __future__ import annotations
import sqlite3
from pathlib import Path
from gateway import delivery_ledger as dl
def test_recording_a_reply_does_not_open_a_second_connection(tmp_path, monkeypatch):
monkeypatch.setattr(dl, "_db_path", lambda: tmp_path / "state.db")
db = tmp_path / "state.db"
monkeypatch.setattr(dl, "_db_path", lambda: db)
real_connect = sqlite3.connect
opened: list[str] = []
def counting_connect(*args, **kwargs):
opened.append(str(args[0]) if args else str(kwargs.get("database")))
target = args[0] if args else kwargs.get("database")
# Only the ledger's own database counts: unrelated sqlite users in the
# process (other modules, background threads) must not move the tally.
if Path(str(target)).resolve() == db.resolve():
opened.append(str(target))
return real_connect(*args, **kwargs)
monkeypatch.setattr(sqlite3, "connect", counting_connect)

View File

@@ -2,29 +2,34 @@ import json
from unittest.mock import patch
from hermes_cli.codex_models import (
_FORWARD_COMPAT_TEMPLATE_MODELS,
DEFAULT_CODEX_MODELS,
get_codex_model_ids,
)
CHATGPT_REJECTED_CODEX_PRO_SLUGS = {
"gpt-5.6-sol-pro",
"gpt-5.6-terra-pro",
"gpt-5.6-luna-pro",
}
def _pro_slugs(model_ids):
return [m for m in model_ids if m.removesuffix("-900k").endswith("-pro")]
def test_curated_codex_fallback_excludes_chatgpt_rejected_pro_slugs(monkeypatch):
"""OAuth fallback retains real models but never synthesizes rejected ones."""
retained_models = {"gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna"}
def test_codex_catalog_never_offers_chatgpt_rejected_pro_slugs(monkeypatch, tmp_path):
"""The ChatGPT Codex OAuth backend 400s every ``-pro`` slug (#52492), so
neither the offline fallback nor forward-compat synthesis over a live
catalog may offer one, while the fallback still keeps every curated model."""
monkeypatch.setenv("CODEX_HOME", str(tmp_path)) # no config.toml default, no cache
offline = get_codex_model_ids()
assert set(DEFAULT_CODEX_MODELS) <= set(offline)
assert _pro_slugs(offline) == []
# Live discovery returning only template slugs fires every forward-compat
# synthesis rule; none of what it adds may be -pro.
templates = list(dict.fromkeys(t for _, ts in _FORWARD_COMPAT_TEMPLATE_MODELS for t in ts))
monkeypatch.setattr(
"hermes_cli.codex_models._fetch_models_from_api",
lambda access_token: ["gpt-5.5"],
"hermes_cli.codex_models._fetch_models_from_api", lambda access_token: templates
)
model_ids = get_codex_model_ids(access_token="codex-access-token")
assert retained_models.issubset(model_ids)
assert CHATGPT_REJECTED_CODEX_PRO_SLUGS.isdisjoint(model_ids)
live = get_codex_model_ids(access_token="codex-access-token")
assert {synthetic for synthetic, _ in _FORWARD_COMPAT_TEMPLATE_MODELS} <= set(live)
assert _pro_slugs(live) == []

View File

@@ -31,6 +31,25 @@ pytestmark = pytest.mark.windows_only
PROJECT_ROOT = Path(__file__).resolve().parents[2]
def _wait_until(predicate, timeout: float = 15.0, interval: float = 0.05) -> bool:
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if predicate():
return True
time.sleep(interval)
return bool(predicate())
def _argv_visible(pid: int, marker: str) -> bool:
"""True once the process table shows *pid* with *marker* in its argv."""
import psutil
try:
return marker in " ".join(psutil.Process(pid).cmdline())
except (psutil.NoSuchProcess, psutil.AccessDenied):
return False
@pytest.fixture()
def sleeper():
procs: list[subprocess.Popen] = []
@@ -42,8 +61,7 @@ def sleeper():
stderr=subprocess.DEVNULL,
)
procs.append(p)
time.sleep(0.5)
assert p.poll() is None
assert _wait_until(lambda: _argv_visible(p.pid, "time.sleep(120)")), "sleeper argv never visible"
return p
yield _spawn
@@ -98,8 +116,7 @@ def test_live_supervised_serve_suppresses_cold_start(sleeper, monkeypatch, tmp_p
# Dead serve → ownership drops → plan returns.
serve.kill()
serve.wait()
time.sleep(0.5)
assert update_cmd._desktop_owns_gateway_lifecycle() is False
assert _wait_until(lambda: update_cmd._desktop_owns_gateway_lifecycle() is False)
token = update_cmd._pause_windows_gateways_for_update()
assert token is not None and token.get("cold_start_if_installed") is True
@@ -116,27 +133,29 @@ def test_holder_scan_fallback_respects_token_classifier(sleeper, monkeypatch, tm
# Lookalike from the #90778 class — must NOT confer ownership.
kanban_like = sleeper("-m", "hermes_cli.main", "kanban", "--preserve-cache")
def fake_holders():
import psutil
import psutil
out = []
for p in (serve_like, kanban_like):
proc = psutil.Process(p.pid)
out.append((p.pid, proc.name(), " ".join(proc.cmdline())))
return out
# Snapshot both holder rows while both processes are alive. Building the
# kanban row later (after the serve kill) would re-read the dead serve pid,
# raise NoSuchProcess and hand the fallback an empty scan, which returns
# False without ever reaching the token classifier.
serve_row, kanban_row = (
(p.pid, psutil.Process(p.pid).name(), " ".join(psutil.Process(p.pid).cmdline()))
for p in (serve_like, kanban_like)
)
assert "--preserve-cache" in kanban_row[2]
monkeypatch.setattr(
"hermes_cli.main._detect_venv_python_processes", fake_holders
"hermes_cli.main._detect_venv_python_processes", lambda: [serve_row, kanban_row]
)
# serve-shaped holder with a live parent (us) → owns
assert update_cmd._desktop_owns_gateway_lifecycle() is True
# Only the kanban lookalike left → classifier rejects → does not own
# Only the (still live) kanban lookalike left → classifier rejects → does not own
serve_like.kill()
serve_like.wait()
assert kanban_like.poll() is None
monkeypatch.setattr(
"hermes_cli.main._detect_venv_python_processes",
lambda: [fake_holders()[1]],
"hermes_cli.main._detect_venv_python_processes", lambda: [kanban_row]
)
assert update_cmd._desktop_owns_gateway_lifecycle() is False

View File

@@ -114,16 +114,18 @@ class TestProcFallback:
mock_ps.assert_not_called() # /proc dir existed, so ps not called
@pytest.mark.linux_only
class TestPsFallbackBsdCompat:
"""The ps fallback must use flags BSD/macOS ps accepts (#73626, #74075).
``ps -A eww`` fails on macOS (BSD ``e`` is not the procps flag), which
made gateway discovery silently return nothing whenever /proc is absent.
Linux-only like ``TestProcFallback``: the real host selects the POSIX arm
and only /proc's absence is faked, to force the ps rung.
"""
def test_ps_fallback_uses_bsd_compatible_flags_and_columns(self):
with (
patch("hermes_cli.gateway.is_windows", return_value=False),
patch("os.path.isdir", side_effect=lambda p: p != "/proc"),
patch("hermes_cli.gateway._get_ancestor_pids", return_value=set()),
patch("subprocess.run") as mock_run,

View File

@@ -25,6 +25,25 @@ import pytest
pytestmark = [pytest.mark.windows_only, pytest.mark.spawns_gateway_lookalike]
def _wait_until(predicate, timeout: float = 15.0, interval: float = 0.05) -> bool:
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if predicate():
return True
time.sleep(interval)
return bool(predicate())
def _argv_visible(pid: int, marker: str) -> bool:
"""True once the process table shows *pid* with *marker* in its argv."""
import psutil
try:
return marker in " ".join(psutil.Process(pid).cmdline())
except (psutil.NoSuchProcess, psutil.AccessDenied):
return False
def test_plan_reconciliation_live_windows(tmp_path, monkeypatch):
home = tmp_path / ".hermes"
home.mkdir()
@@ -43,9 +62,8 @@ def test_plan_reconciliation_live_windows(tmp_path, monkeypatch):
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
)
try:
time.sleep(0.5)
assert child.poll() is None
assert foreign.poll() is None
assert _wait_until(lambda: _argv_visible(child.pid, "gateway")), "stand-in argv never visible"
assert _wait_until(lambda: _argv_visible(foreign.pid, "time.sleep(120)")), "sleeper never visible"
import psutil

View File

@@ -13,8 +13,10 @@ the real rename attempt hitting the real sharing violation.
from __future__ import annotations
import os
import queue
import subprocess
import sys
import threading
import time
from pathlib import Path
@@ -25,6 +27,36 @@ pytestmark = pytest.mark.windows_only
PROJECT_ROOT = Path(__file__).resolve().parents[2]
def _wait_until(predicate, timeout: float = 15.0, interval: float = 0.05) -> bool:
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if predicate():
return True
time.sleep(interval)
return bool(predicate())
def _readline(stream, timeout: float = 60.0) -> str:
"""``stream.readline()`` bounded by *timeout* (a hung child fails, not hangs)."""
got: queue.Queue = queue.Queue()
threading.Thread(target=lambda: got.put(stream.readline()), daemon=True).start()
try:
return got.get(timeout=timeout)
except queue.Empty:
return ""
def _rename_round_trips(path: Path) -> bool:
"""True once no handle blocks a rename of *path* (the holder is gone)."""
probe = path.with_name(path.name + ".probe")
try:
os.rename(path, probe)
except OSError:
return False
os.rename(probe, path)
return True
# Child that opens a file with GENERIC_READ and NO FILE_SHARE_DELETE —
# the exact sharing mode a running .exe image / desktop backend exhibits.
_HOLDER_CODE = r"""
@@ -55,7 +87,7 @@ def held_shim(tmp_path: Path):
stdout=subprocess.PIPE,
text=True,
)
line = holder.stdout.readline().strip()
line = _readline(holder.stdout).strip()
if line != "HOLDING":
holder.kill()
pytest.fail(f"lock-holder child failed: {line!r}")
@@ -129,10 +161,12 @@ def test_release_then_strict_quarantine_succeeds(tmp_path, monkeypatch):
stdout=subprocess.PIPE,
text=True,
)
assert holder.stdout.readline().strip() == "HOLDING"
assert _readline(holder.stdout).strip() == "HOLDING"
holder.kill()
holder.wait()
time.sleep(0.3) # handle teardown
# The handle is released when the kernel tears the (possibly trampolined)
# holder down, not when wait() returns.
assert _wait_until(lambda: _rename_round_trips(scripts / "hermes.exe")), "holder handle never released"
install_ran: list = []
monkeypatch.setattr(