diff --git a/emrg/client/daemon_manager.py b/emrg/client/daemon_manager.py index d48436a2..4c5a3717 100644 --- a/emrg/client/daemon_manager.py +++ b/emrg/client/daemon_manager.py @@ -371,6 +371,29 @@ async def _await_daemon_ready( ) +def _port_probe(_pid: int) -> bool: + """`is_running()` wearing `pid_alive`'s ``win_probe`` contract — ``probe(pid)``. + + The two probes this module supplies to `pid_alive` do not have the same + signature, and that is not a detail: `_win_pid_alive` answers *about a pid* + (it opens the process handle), while :func:`is_running` answers about the + **port file** and takes no argument at all. Passing the latter straight + through was a `TypeError` on every Windows restart: `is_running() takes 0 + positional arguments but 1 was given`, raised from inside `pid_alive` on the + first iteration of the wait loop — i.e. on the path that exists to keep one + daemon from becoming two. Nothing on POSIX reaches it (`platform` is `win*` + there), which is exactly why it survived local runs, and it was measured on + Windows by the windows-2025 leg of run 35283518915. + + The argument is accepted and dropped rather than the call being made without + one, because the *contract* is `pid_alive`'s to define and it is the one + asking; the port probe is what this caller has to answer with (a Windows + SIGTERM is a hard kill, so the pid is gone and the port file is the thing + that lingers). + """ + return is_running() + + def _old_daemon_alive(pid: int, *, platform: str = "", kill=None, win_probe=None) -> bool: """Is the daemon we just signalled gone? — asked once, not respelled. @@ -382,13 +405,33 @@ def _old_daemon_alive(pid: int, *, platform: str = "", kill=None, ``GenerateConsoleCtrlEvent`` and the call becomes a Ctrl+C delivered to that pid's console process group — every process sharing it, this CLI's shell included. Windows SIGTERM is an immediate hard kill, so a *port* probe is - what answers there, and :func:`is_running` is this caller's Windows probe. + what answers there, and :func:`_port_probe` is this caller's Windows probe — + a port probe, because ``is_running`` answers for the port rather than for a + pid and cannot be handed over as it stands. ``platform`` / ``kill`` / ``win_probe`` are forwarded to `pid_alive` so both answers can be pinned on every runner: the Windows branch is the unsafe one, and a probe whose Windows behaviour is only observable on Windows is a defect discovered on Windows. + Both halves of the answer are sourced from **this** module when the caller + injected neither, and that symmetry is load-bearing rather than tidy: the + Windows half already had to come from here (``_port_probe`` over our + ``is_running``), and the + POSIX half has to as well, or the restart route's signalling is split across + two modules. It briefly was: `pid_alive` defaults its ``kill`` to its *own* + module's ``os.kill``, so the probe left the tripwire + ``conftest::_guard_no_live_daemon_is_signalled`` installs on this module — the + substitute ``os`` the guard scopes here (and the escape hatch it documents: + "tests that drive the restart logic patch the module's ``os.kill``") silently + stopped covering the probe, because the patch lands on the substitute while + the probe asked the real module. Two tests in ``tests/test_daemon_manager.py`` + fake that answer to pin the wait loop, and they then read a real + ``os.kill(, 0)``: ``ESRCH``, i.e. *gone*, on the first iteration — + no wait, and no SIGKILL fallback. Naming our ``os`` here makes the faked + answer and the guarded answer the same object again, in production and in + tests. + One reading changes, and it is worth naming rather than discovering: the copy here read ``EPERM`` as *alive* (the process exists, we may not signal it) and `pid_alive` reads any ``OSError`` as *gone*. The pid this is asked about is @@ -401,8 +444,8 @@ def _old_daemon_alive(pid: int, *, platform: str = "", kill=None, return pid_alive( pid, platform=platform, - kill=kill, - win_probe=is_running if win_probe is None else win_probe, + kill=os.kill if kill is None else kill, + win_probe=_port_probe if win_probe is None else win_probe, ) @@ -486,8 +529,14 @@ def _old_pid_alive() -> bool: else: # SIGTERM didn't work — force kill logger.warning("old daemon (pid=%d) didn't die, sending SIGKILL", server_pid) + # Windows has no `signal.SIGKILL` (measured — the AttributeError + # escaped the whole restart path, because nothing here catches + # it: `ensure_connected()` died instead of respawning the + # daemon). Its `os.kill` ignores the number and terminates the + # process, which is what this fallback is for, so ask for + # whatever force the platform has. try: - os.kill(server_pid, signal.SIGKILL) + os.kill(server_pid, getattr(signal, "SIGKILL", signal.SIGTERM)) except (ProcessLookupError, OSError): pass for _ in range(10): # up to 2s for SIGKILL to land diff --git a/tests/conftest.py b/tests/conftest.py index b6003c0b..28434a64 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,7 +22,8 @@ """ from __future__ import annotations -from pathlib import Path +import re +from pathlib import Path, PurePosixPath, PureWindowsPath import pytest @@ -183,6 +184,546 @@ def _guard_stop_log_is_not_host_state(monkeypatch, _stop_log_scratch): monkeypatch.setenv("EMRG_STOP_LOG_DIR", str(_stop_log_scratch)) +# ── the daemon is EMRG's life core: no suite run may signal it ──────────────── +# +# Issue #1337, item 2. `_guard_stop_all_hermeticity` above covers +# `emrg._stop_all`'s five stop functions — the *in-process* route. Two other +# routes reach a live daemon without passing through them, and neither was +# covered by anything: +# +# * the client-side restart, `emrg.client.daemon_manager. +# check_and_restart_if_stale()` — it SIGTERMs the pid the pong frame named +# whenever the source looks newer than the daemon's start time; +# * a child process — `python -m emrg server stop`, `emrg server restart`, +# `pkill -f emrg.server`. +# +# The shape this exists for is measured, not hypothetical: on 2026-09-17 a +# full-suite run SIGTERMed the live daemon mid-run (`~/.emrg/emrgd-exit.log`: +# `reason sigterm exit_code 143`). It respawned, the scheduler re-sent the cycle +# task while the tree still held uncommitted work, and the dirty-tree guard +# pinned that cycle to read-only — the cost of one unguarded route. +# +# Both halves refuse *before* anything is signalled or spawned, and that is what +# makes the positive controls in tests/test_hermeticity_guard.py safe to write: +# they call these paths and never cause the event they forbid. + +_RED_LINE = "⛔ red-line violation (host 2026-08-18T22:58, issue #1337)" + + +def _kill_is_a_liveness_probe(sig, platform: str = "") -> bool: + """Is `os.kill(pid, sig)` on this platform a pure existence check? + + POSIX `kill(pid, 0)` is: it delivers nothing and reports ESRCH / EPERM. + **Windows has no such call.** `signal.CTRL_C_EVENT` is **0**, and CPython's + `os_kill_impl` routes `CTRL_C_EVENT` / `CTRL_BREAK_EVENT` to + `GenerateConsoleCtrlEvent(pid, sig)` before it ever reaches + `TerminateProcess` — so there `kill(pid, 0)` is a **Ctrl+C to that process + group**, delivered to every process sharing the console, pytest included. + + This is why the guard below cannot delegate `sig == 0` unconditionally: on + Windows that single value *is* a signal, and signalling the daemon is the one + act the red line forbids. `emrg/client/daemon_manager.py`'s restart path + skips its probe on win32 for the same reason, so refusing it there costs the + suite nothing. + + The platform is a parameter rather than a `sys.platform` read so the decision + can be pinned on any runner — a guard whose behaviour on Windows is only + testable on Windows is a guard whose Windows behaviour is only discovered on + Windows. + """ + if not platform: + import sys + + platform = sys.platform + return sig == 0 and not platform.startswith("win") + + +class _NoSignalOs: + """`os` as `emrg/client/daemon_manager.py` sees it: identical, minus signals. + + Scoped to that module's own namespace rather than patching `os.kill` itself, + because `daemon_manager` is the only place in the client that signals a + process and the only process it signals is the daemon. A global patch would + also intercept `Popen.kill()` on a child a test spawned deliberately — a + different act, and one the suite needs. + + `kill(pid, 0)` is delegated only where it is genuinely a probe — see + `_kill_is_a_liveness_probe`: on POSIX it is the liveness check this module + uses to wait for the old daemon to die (rant 2026-08-18T12:49:09 ②) and + refusing it would replace a benign check with an error; on Windows the same + value is `signal.CTRL_C_EVENT`, i.e. a delivered signal, so it is refused + like any other. + + `is_probe` is injected so a test can drive both decisions on either platform + without ever delegating to a real `os.kill`. + + `killpg` is refused alongside `kill`. `__getattr__` would otherwise delegate it + to the real `os` — measured on the revision before this one, + `guard.killpg(4242, 15)` reached the stand-in `os` — which made "signal the + daemon's process group" the one signal route out of this wrapper. Nothing in + the restart path signals a group today, so this is scope rather than a live + hole; it is here because the act the red line forbids does not get narrower + when the target is a group, and because the blast radius is *larger*: the + daemon shares its group with whatever else the host started. + + ``holder`` is the module the shim is installed on. It is named in the refusal + because two modules carry a kill that reaches the daemon and the message is + the only thing that says which one fired — `emrg.client.daemon_manager`'s + restart path and `emrg.__main__`'s own `emrg server stop` fallback. One class + for both keeps one rule: a second wrapper would be a second place to forget + something, and the two routes are the same act. + """ + + def __init__(self, real, is_probe=_kill_is_a_liveness_probe, + holder="emrg.client.daemon_manager"): + self._real = real + self._is_probe = is_probe + self._holder = holder + + def __getattr__(self, name): + return getattr(self._real, name) + + def kill(self, pid, sig): + if self._is_probe(sig): + return self._real.kill(pid, sig) + raise AssertionError( + f"{_RED_LINE}: {self._holder} tried to signal pid {pid} " + f"with signal {sig!r}. A test must never stop or restart the live " + f"daemon — it is EMRG's life core. Isolate the restart path you are " + f"testing instead, as tests/test_daemon_manager.py's restart tests do " + f"(`@patch('emrg.client.daemon_manager.os.kill')`)." + ) + + def killpg(self, pgid, sig): + raise AssertionError( + f"{_RED_LINE}: {self._holder} tried to signal the process " + f"group {pgid} with signal {sig!r}. A test must never stop or restart " + f"the live daemon — it is EMRG's life core. Isolate the restart path " + f"you are testing instead, as tests/test_daemon_manager.py's restart " + f"tests do (`@patch('emrg.client.daemon_manager.os.kill')`)." + ) + + +_EMRG_PROGRAMS = ("emrg", "emrgd") +_SHELLS = ("sh", "bash", "dash", "zsh", "ksh") +_SIGNALLERS = ("pkill", "killall", "kill") +_STOP_VERBS = ("stop", "restart") + +#: Punctuation a shell may leave glued to a token's **ends**: `(emrg`, `emrg)`, +#: `'emrg'`, `` `emrg` ``, `$(which` — see `_normalise_token`. +_SHELL_PUNCTUATION = "(){}[]$`'\"<>" + +#: Characters a **shell drops wherever they appear** rather than at an edge: a +#: backslash escapes the character after it, and quotes join adjacent pieces into +#: one word. Applied only where a shell re-parses the token — a `sh -c` payload, or +#: an argv handed to `Popen` as a *string* (which `shell=True` sends to a shell) — +#: never to a list argv, which reaches exec/CreateProcess literally. That is the +#: `shell_parsed` argument of `_normalise_token` / `_spawns_a_daemon_stop_or_restart`. +_SHELL_DROPPED = "\\'\"" + + +def _normalise_token(token: str, *, shell_parsed: bool = False) -> str: + """The token as the *program* would receive it — glued shell punctuation stripped. + + A shell line arrives here already split on whitespace by `Popen`, and shell + punctuation is not whitespace, so it stays glued to its neighbour: + `sh -c "(emrg server restart)"` tokenises to `["(emrg", "server", + "restart)"]` and `sh -c "$(which emrg) server stop"` to `["$(which", + "emrg)", "server", "stop"]`. Read literally, neither carries an `emrg` + basename or a bare `stop`/`restart` verb, so the first revision of this guard + allowed **all** of them (measured, this PR's veto, cycle `cyc20260918-043412`): seven spellings of the + one act the red line forbids, including the substitution form a test reaches + for when it wants the installed script's path — and that form reaches the + *live* daemon. + + None of them needs a shell parser. The punctuation is a fixed set, it is + stripped from both ends, and what is left is what the program is handed; where + a spelling is still ambiguous after this, the caller keeps its bias toward + refusing (`_spawns_a_daemon_stop_or_restart`). Stripping is deliberately blind + to *purpose*: `$(which emrg)` is not evaluated here, because evaluating it + would mean running a command to answer a question about an argv. + + Two spellings survived the first pass of that rule and are closed here, both + measured with a stub `emrg` on `PATH` (the stub ran, with argv + ``server stop``, in both cases — i.e. they reach the **live** daemon, not a + lookalike): + + * a **leading backslash** — `sh -c "\\emrg server stop"`: the shell removes + the escape and runs `emrg`. The old set had no backslash, so the token was + `"\\emrg"`, whose basename is not `emrg`; + * **quote concatenation** — `sh -c "'e''mrg' server stop"`: adjacent quoted + and unquoted pieces are one word to the shell, so the program is `emrg`. + `str.strip` only reaches the *ends* of a token and left `e''mrg`. + + That is why this is a *removal* rule and not a wider strip: the shell drops + those characters wherever they are, so `e\\mrg` and `sto\\p` are the program + `emrg` and the verb `stop` too, and an edge-only rule would close the two + measured spellings while leaving their siblings open. The set stays small on + purpose — these are the characters whose shell meaning *is* "delete me"; a + substitution (`$(…)`, `` `…` ``) is only closed at an edge, and a token that + expands to something else is the same evasion class as a `-c` string. + + And one row that looks like the third of the family and is **refused**, on + purpose, at a measured cost: `sh -c "emrg\\ server\\ stop"`. The shell reads + the escapes as joining three words into one command name, `emrg server stop`, + which cannot exist — it answers `emrg server stop: command not found` + (measured), so refusing it is over-broad. It is refused anyway because the + split here is whitespace-only **by design** (a shell-parsing split is the thing + this function exists to avoid), so the token stream after it is exactly + `["emrg", "server", "stop"]` — the act's own spelling — and the two are not + distinguishable without modelling `\\ ` as a joiner. Accepted rather than + repaired: no test writes an escaped space before a verb, and the alternative + (deciding "the verb is only a verb when nothing was escaped before it") + reopens the hole this rule closes. `tests/test_hermeticity_guard.py` pins the + row so both halves of that trade are visible. + + **Those two are shell transformations, so they apply only where a shell + re-parses the token** (`shell_parsed`): inside a `sh -c` payload, or in an + argv handed over as a *string* (which `shell=True` sends to a shell). A list + argv is handed to exec/CreateProcess as it stands — nothing drops anything — + and on Windows the backslash in that token is a **path separator**, so + dropping it moved the basename off `emrg` and the guard stopped refusing the + act outright. Measured on the windows-2025 leg of run 35286596898, the first + CI round of this rule: `test-windows` failed both ways at once — the + known-cost row `git -C <…>\\emrg log --grep restart` no longer refused, and + the refusal corpus really spawned its stub (`OSError: [WinError 193] %1 is + not a valid Win32 application`), because the refusal that keeps those stubs + from being executed is the very thing that had stopped firing. So the flag is + the fix, not a caution: a rule about what a *shell* deletes cannot be applied + to an argv no shell touches, on either platform. + """ + stripped = token.strip(_SHELL_PUNCTUATION) + if not shell_parsed: + return stripped + for dropped in _SHELL_DROPPED: + stripped = stripped.replace(dropped, "") + return stripped + + +def _token_readings(token: str, *, shell_parsed: bool = False) -> tuple[str, ...]: + """Every spelling of this token a shell/platform might really hand over. + + `_normalise_token` answers "what does the *shell* make of this token"; that is + the right answer only where a shell is, and only on a platform whose shell + does the deleting. The windows-2025 leg of run 35287972569 is the report for + treating it as universal: a string argv (`shell=True`) naming a Windows path — + `C:\\ws\\bin\\pkill -f 'python -m emrg'` — is *shell-parsed*, and the drop rule + then deleted the path separators inside `tokens[0]`, whose basename stopped + being `pkill`. The signaller check missed, the guard allowed, and the corpus + spawned its stub (`OSError: [WinError 193]`). `cmd.exe` does not delete a + backslash; `sh` does. Neither reading is wrong — the mistake was picking one. + + So a token is judged under **every** reading, and the guard refuses if any of + them is the act (the bias below). A list argv has exactly one reading, because + nothing drops anything before exec/CreateProcess. + """ + literal = _normalise_token(token, shell_parsed=False) + if not shell_parsed: + return (literal,) + dropped = _normalise_token(token, shell_parsed=True) + return (literal,) if dropped == literal else (literal, dropped) + + +def _basenames(token: str, *, shell_parsed: bool = False) -> tuple[str, ...]: + """The program names this token could be, under either path flavour. + + `Path(...).name` is the *host's* basename, which is what made the verdict + depend on which machine evaluated it: `C:\\ws\\bin\\emrg` has the basename + `emrg` to `ntpath` and the whole string to `posixpath`. The guard's question is + about the shape of an argv, so both flavours are asked and a match on either + refuses. On POSIX this widens nothing that a test writes (a backslash in a + *list* argv token is a filename character there); on Windows it is the flavour + that was already in force, now visible to a control that runs on any host. + """ + names: list[str] = [] + for reading in _token_readings(token, shell_parsed=shell_parsed): + for name in (PurePosixPath(reading).name, PureWindowsPath(reading).name): + if name not in names: + names.append(name) + return tuple(names) + + +def _emrg_entry_index(tokens, *, shell_parsed: bool = False) -> int: + """Where in this argv the emrg program itself would be run, or -1. + + The program is not necessarily `tokens[0]`: a wrapper hands it over (`env emrg + …`, `nohup emrg …`, `timeout 5 emrg …`, `uv run emrg …` — `uv run` is how this + repo runs its own tools), and an interpreter names it with `-m emrg` / + `-m emrg.server`. Reading only `tokens[0]` missed every one of those: measured + on the revision before this one, `uv run emrg server stop`, `env emrg stop`, + `nohup emrg stop`, `timeout 5 emrg server restart` and `nice -n 5 pkill -f + emrg.server` all passed a guard whose own docstring claims it refuses "the emrg + entry points … carrying a `stop`/`restart` verb". + + An interpreter's `-c` string is *not* read here: that would mean parsing Python, + and a test hiding the act inside a language string is evading the guard rather + than reaching the daemon by an ordinary route. + + Matching goes through `_normalise_token`, so a program the shell glued to its + own punctuation (`$(which emrg)`, `(emrg`, `'emrg'`) is the same program here. + + The scan accepts **any** position whose basename is `emrg`/`emrgd`, not only + command position, and that is a deliberate asymmetry rather than an oversight: + the wider rule costs a false refusal whenever an `emrg`-named path is used as + data (`git -C log --grep restart` is refused — measured by this + PR's veto, with this repo's own path), while narrowing it to `tokens[0]` plus + the wrappers would allow an `emrg` program handed over by anything not on that + list (`xargs -I{} emrg {} stop`). Of the two, only the second is the incident: + a false refusal fails a test at its own assertion, naming the red line, and a + false allowance SIGTERMs the live daemon mid-suite (#1337 item 2, one cycle + lost to read-only). The verb test that follows is what keeps the common + data-verb shapes allowed — `git commit -m stop`, `git log --grep emrg` — and + they are pinned in `tests/test_hermeticity_guard.py`. + """ + for i, token in enumerate(tokens): + if any(name in _EMRG_PROGRAMS for name in _basenames(token, shell_parsed=shell_parsed)): + return i + if token == "-m" and tokens[i + 1 : i + 2] in (["emrg"], ["emrg.server"]): + return i + 1 + return -1 + + +def _spawns_a_daemon_stop_or_restart(args, *, shell_parsed: bool = False) -> bool: + """Would this `Popen` argv stop or restart the emrg daemon? + + Keyed on the **act**, not on the mention: the emrg program carrying a + `stop`/`restart` verb, or a process-signalling command whose text names emrg. + The verb must come *after* the program, so a verb passed as data (`git commit + -m stop`) or a name in a pattern (`git log --grep emrg`) stays allowed — the + suite spawns git, node, pytest and the read-only `emrg` verbs, and a guard that + refused those would break them and teach the next reader to distrust it. + + Two spellings the first revision read as somebody else's argv, both of them + this repo's own idiom, and both closed by looking past `tokens[0]`: + + * a **shell** runs the string it was handed, so `sh -c "emrg server stop"` is + the same act as `emrg server stop` — the line inside is classified; + * a **wrapper** hands the program over — `env emrg stop`, `nohup emrg stop`, + `timeout 5 emrg server restart`, `uv run emrg server stop`, `nice -n 5 pkill + -f emrg.server`. + + The bias is deliberate: a false refusal is loud, immediate and cheap (the test + fails where it stands, with a message naming the red line), while a false + allowance is the incident — a SIGTERM to the live daemon, which cost a cycle + read-only on 2026-09-17 and is unrecoverable mid-run. Where the two spellings + of the act are distinguishable only by shell parsing this cannot do, the guard + refuses rather than guesses. + + **Every token match goes through `_normalise_token`** (this PR's veto, cycle + `cyc20260918-043412`), which closed seven measured spellings that the + token-literal reading allowed — all of them the same act, and one of them + (`$(which emrg) server stop`) the form a test reaches for when it wants the + installed script's path, i.e. a route to the *live* daemon: + + * `sh -c "(emrg server restart)"` — the program glued to a grouping paren; + * `sh -c "$(which emrg) server stop"` — a command substitution's result; + * ``sh -c "`emrg server stop`"`` — the older substitution spelling; + * `sh -c "'emrg' server stop"` — a quoted program name; + * `sh -c 'env -S "emrg server stop"'` — `env -S` handing over one string; + * `[..., "emrg", "server", "(stop)"]` — the *verb* glued to punctuation, no + shell involved at all. + + The rules they all sit on top of are unchanged, and the shapes that must stay + allowed are pinned beside these in `tests/test_hermeticity_guard.py`: the verb + must come *after* the program, so `git commit -m stop` and `emrg --help | grep + stop` remain allowed. One qualification the veto measured, kept rather than + papered over: the program scan accepts any position whose basename is + `emrg`/`emrgd`, so an `emrg`-named path used as *data* makes the verb decisive + — `git -C log --grep restart` is refused. That direction is loud + and cheap and its repair is a narrowing whose own hole (an `emrg` program + handed over by a wrapper not on any list) is the incident; see + `_emrg_entry_index`, and `test_an_emrg_named_path_costs_a_false_refusal` for + the shape pinned as a known cost. + + **Shell semantics are applied where a shell is** (`shell_parsed`): the `sh -c` + recursion and the string-argv branch, never a list argv. Getting that wrong is + not a subtlety — it failed the windows-2025 leg of run 35286596898 outright, + in both directions at once, because a Windows `str(tmp_path / "emrg")` is + backslash-separated and the drop rule deleted those backslashes: the basename + stopped being `emrg`, the refusal stopped firing, and the corpus below then + really spawned its stub. Two of its rows are the report + (`OSError: [WinError 193] %1 is not a valid Win32 application`, and "the known + cost narrowed" on the `git -C … log --grep restart` row). + `test_a_list_argv_is_not_shell_dropped` pins the rule on any host. + + **No single reading is treated as *the* reading** (`_token_readings`, + `_basenames`): the confirmation that no shell is involved is not the same as + knowing which shell *is*, so a shell-parsed token is judged both as the shell + would hand it over and as exec/CreateProcess would receive it, and a token is + compared under both path flavours. That is the second windows-2025 report + (run 35287972569): with the drop applied to the only reading, a *string* argv + naming a Windows path had its separators deleted inside `tokens[0]`, so + `C:\\ws\\bin\\pkill -f 'python -m emrg'` stopped being a signaller and the stub + it named was really executed. The guard refuses if *any* reading is the act, + which is the bias this function already documents — with two readings there is + no longer a guess to bias against. + """ + if isinstance(args, bytes): + args = args.decode("utf-8", "replace") + if isinstance(args, str): + tokens = args.split() + # A string argv goes to a shell (`shell=True`), which drops escapes and + # joins quotes. A list argv does not — it is passed through literally — + # so only the string form and the `sh -c` recursion below are shell-parsed. + shell_parsed = True + else: + try: + tokens = [str(a) for a in args] + except TypeError: + return False # not an argv at all — let Popen raise its own error + if not tokens: + return False + + if any(name in _SHELLS for name in _basenames(tokens[0], shell_parsed=shell_parsed)): + # `sh -c ` / `bash -lc `: what runs is the line, so decide on + # what the line would run — one *simple command* at a time, because a line + # is a pipeline of them and only one of them may be the act. Splitting on + # the separators is what keeps `sh -c "emrg --help | grep stop"` allowed + # (the verb is a pattern there) while `sh -c "emrg server stop && echo ok"` + # is refused. A newline separates two commands exactly as `;` does, so it + # is a separator too. The split drops quotes and leaves the punctuation at + # the segment edges, which is why both the token match below and the verb + # test go through `_normalise_token` — with `shell_parsed=True`, because a + # shell is what will run every segment of this line. + return any( + _spawns_a_daemon_stop_or_restart(segment.split(), shell_parsed=True) + for segment in re.split(r"[;&|\n]+", " ".join(tokens[1:])) + ) + + for i, token in enumerate(tokens): + if ( + any(name in _SIGNALLERS for name in _basenames(token, shell_parsed=shell_parsed)) + and "emrg" in " ".join(tokens[i + 1 :]).lower() + ): + return True + + entry = _emrg_entry_index(tokens, shell_parsed=shell_parsed) + if entry < 0: + return False + return any( + reading in _STOP_VERBS + for token in tokens[entry + 1 :] + for reading in _token_readings(token, shell_parsed=shell_parsed) + ) + + +@pytest.fixture +def daemon_spawn_refusal(): + """The argv predicate the guard keys on, for a test that must classify a shape + without spawning it. + + The shapes worth pinning as *allowed* include the live `emrg` CLI, and a test + that proves allowance by running it is asserting two things at once: that the + guard permitted the spawn, and that the CLI behaves on this platform. The + second is `tests/test_cli_output_encoding.py`'s job, and it already runs this + CLI here (`--help` under ascii and cp1252). Exposing the predicate lets this + file pin the first, over more shapes than one invocation could cover. + """ + return _spawns_a_daemon_stop_or_restart + + +@pytest.fixture +def token_normaliser(): + """`_normalise_token`, so a token's reading can be pinned without an argv. + + The predicate above answers "would this argv be refused"; the `shell_parsed` + rule underneath it is about a single token, and the Windows failure that made + the rule explicit (run 35286596898) is visible at that level on any host — + `Path(...).name` on Windows is `ntpath.basename`, so a test can evaluate the + guard's own comparison the way Windows would. Exposed for the same reason the + predicate is: no spawn, no signal, just the classification. + """ + return _normalise_token + + +@pytest.fixture +def daemon_kill_is_a_probe(): + """The guard's "is this call a probe?" decision, for the platform table. + + Exposed for the reason `daemon_spawn_refusal` is: Windows is the platform + where the decision flips, and a test that could only observe it by running on + Windows would leave the flip unmeasured on every other runner. + """ + return _kill_is_a_liveness_probe + + +@pytest.fixture +def daemon_kill_refusal(): + """The guard's `os` substitute, constructible with a supplied decision. + + A test that wants to see the refusal for a Windows-shaped `kill(pid, 0)` + cannot do it by calling the installed guard: on POSIX that reaches a real + `os.kill`, and on Windows the whole point is that it must not reach one. + Building the guard over a stand-in `os` lets both branches be pinned while + nothing is signalled on any platform. + """ + return _NoSignalOs + + +@pytest.fixture(autouse=True) +def _guard_no_live_daemon_is_signalled(monkeypatch): + """⛔ No suite run may stop or restart a live daemon, by any route. + + The in-process route is covered by `_guard_stop_all_hermeticity`; this covers + the other three — the client-side restart's `os.kill`, the `emrg server stop` + CLI's own SIGTERM fallback, and a child process spawned from a test (issue + #1337, item 2). + + **Two modules, not one.** `emrg/__main__.py::_stop_daemon` SIGTERMs the pid it + read from a `ping` frame, and it does *not* go through `emrg._stop_all`'s five + stop functions — so `_guard_stop_all_hermeticity` above never sees it, and a + test calling it in-process would signal the daemon the evolution is running + on. Measured shape, not a hypothesis: both files that describe the stop path + (`tests/test_cli_failure_reporting.py`, `tests/test_stop_all.py`) say in prose + that they must never run it, and prose is not a guard. One shim class covers + both so there is one rule to read and one place to change. + + Tests that really do exercise the restart path keep working: their own + `@patch('emrg.client.daemon_manager.os.kill')` layers over this fixture and + replaces the refusal with their mock, which is the visible, deliberate act + the red line asks for. + """ + import subprocess + + import emrg.__main__ as cli_mod + import emrg.client.daemon_manager as daemon_manager + + for module, holder in ( + (daemon_manager, "emrg.client.daemon_manager"), + (cli_mod, "emrg.__main__"), + ): + monkeypatch.setattr(module, "os", _NoSignalOs(module.os, holder=holder)) + + real_popen = subprocess.Popen + + class _GuardedPopen(real_popen): + """`Popen` with the refusal in front of it — and still a *type*. + + A plain function patched over `subprocess.Popen` answers + `isinstance(x, subprocess.Popen)` with `TypeError: isinstance() arg 2 must + be a type, a tuple of types, or a union`. The patch is autouse for the whole + suite, so a plain function would make that question unanswerable everywhere + — and the error it raises names `isinstance`, not the red line, so the next + reader debugs the wrong thing. No test asks it today (grepped the tree); + subclassing removes the class of failure instead of pinning it, since + `isinstance`, `issubclass` and the inherited `__init__` all stay honest + while the refusal is identical. + """ + + def __init__(self, args, *rest, **kwargs): + if _spawns_a_daemon_stop_or_restart(args): + raise AssertionError( + f"{_RED_LINE}: a test tried to spawn {args!r}, which stops or " + f"restarts the emrg daemon. A child process can kill the live " + f"daemon just as a direct call can; test the CLI's behaviour by " + f"calling it in-process with its stop path isolated." + ) + super().__init__(args, *rest, **kwargs) + + _GuardedPopen.__name__ = "Popen" + monkeypatch.setattr(subprocess, "Popen", _GuardedPopen) + + @pytest.fixture(autouse=True) def _guard_upgrade_hermeticity(monkeypatch, tmp_path): """⛔ Red line (host 2026-08-21T10:35:57): tests must NEVER trigger the diff --git a/tests/test_daemon_manager.py b/tests/test_daemon_manager.py index e3944e71..04a4f8ee 100644 --- a/tests/test_daemon_manager.py +++ b/tests/test_daemon_manager.py @@ -283,6 +283,58 @@ def test_restart_force_kills_stuck_old_pid( "stuck old pid must be SIGKILLed after the SIGTERM grace window") assert mock_cleanup.called + @patch("emrg.client.daemon_manager._get_server_source_mtime", return_value=1e12) + @patch("emrg.client.daemon_manager.is_running", return_value=True) + @patch("emrg.client.daemon_manager.cleanup_server") + @patch("emrg.client.daemon_manager.os.kill") + @patch("emrg.client.daemon_manager.connect_to_server", new_callable=AsyncMock) + def test_the_force_kill_asks_for_a_signal_the_platform_has( + self, mock_connect, mock_kill, mock_cleanup, mock_running, + mock_src, tmp_path, monkeypatch): + """Windows has no `signal.SIGKILL`, and naming it killed the restart path. + + The fallback used to call `os.kill(pid, signal.SIGKILL)` outright. On + Windows that attribute does not exist, so the `AttributeError` left + `check_and_restart_if_stale()` — nothing on the way out catches it — + and `ensure_connected()` died *instead of* respawning the daemon: the + wait loop had already spent its 10s, so the one outcome the force kill + exists to produce (a dead old daemon, then a fresh one) became a crash. + + A platform that has no `SIGKILL` is simulated over the module's own + `signal` name, the same way the guard simulates one over its `os`. POSIX + cannot reach the row by running it, and a defect whose only observation + platform is Windows is a defect discovered on Windows — which is how this + one was (the windows-2025 leg of run 35283518915). + + Both signals asserted are `SIGTERM` here, and that is the point rather + than a coincidence: with no force signal to escalate to, the fallback + must still *make* the call with the strongest force the platform has. + """ + import signal as real_signal + + token_file = tmp_path / "emrgd.token" + token_file.write_text("token\n") + mock_connect.return_value = FakeWS([_ping_pong_frame()]) + mock_kill.side_effect = lambda pid, sig: None # pid stays "alive" forever + + class _NoSigkill: + """`signal` as Windows has it: `SIGTERM` exists, `SIGKILL` does not.""" + + SIGTERM = real_signal.SIGTERM + + monkeypatch.setattr(daemon_manager, "signal", _NoSigkill) + + with patch("emrg.client.daemon_manager.get_server_path", + return_value=str(token_file)): + asyncio.run(daemon_manager.check_and_restart_if_stale()) + + forces = [sig for _pid, sig in (c.args for c in mock_kill.call_args_list) + if sig != 0] + assert forces == [real_signal.SIGTERM, real_signal.SIGTERM], ( + "the graceful signal, then the fallback — the fallback must ask for " + f"the force this platform has: {forces}") + assert mock_cleanup.called, "and the port file is still released after it" + @patch("emrg.client.daemon_manager._get_server_source_mtime", return_value=0.0) @patch("emrg.client.daemon_manager.connect_to_server", new_callable=AsyncMock) def test_server_unreachable_silent(self, mock_connect, mock_src, tmp_path): @@ -366,12 +418,28 @@ def _would_signal(pid, sig): assert seen == [4321] def test_windows_default_answer_is_the_port_probe(self): - """With no probe injected, Windows is answered by `is_running()`.""" - with patch("emrg.client.daemon_manager.is_running", - return_value=False) as mock_running: + """With no probe injected, Windows is answered by `is_running()`. + + The stand-in is a **zero-argument** callable on purpose. `is_running` takes + no pid — it asks the port file — while `pid_alive`'s `win_probe` contract is + ``probe(pid)``. A `MagicMock` (what this test used to patch in) accepts + either call, so it stayed green while the production path raised `TypeError: + is_running() takes 0 positional arguments but 1 was given` on every Windows + restart — measured on the windows-2025 leg of run 35283518915, where a test + that reproduced the shape failed. A stand-in that can be called the wrong + way is not a control: this one is called the wrong way the moment the + adapter stops being the supplier. + """ + calls: list = [] + + def _port_is_up() -> bool: + calls.append(None) + return False + + with patch("emrg.client.daemon_manager.is_running", new=_port_is_up): assert daemon_manager._old_daemon_alive( 4321, platform="win32") is False - assert mock_running.called + assert calls == [None] def test_the_platform_decision_is_not_respelled_here(self): """One spelling of the rule — the deletion is what is being pinned. diff --git a/tests/test_hermeticity_guard.py b/tests/test_hermeticity_guard.py index efe03b9e..62bf0303 100644 --- a/tests/test_hermeticity_guard.py +++ b/tests/test_hermeticity_guard.py @@ -10,6 +10,7 @@ """ from __future__ import annotations +import ntpath from pathlib import Path import pytest @@ -56,3 +57,500 @@ def test_guard_rejects_real_tasks_yml(): real = (Path.home() / ".emrg" / "tasks.yml").resolve() with pytest.raises(AssertionError, match="hermetic"): sched_mod.atomic_write_yaml([], real, prefix=".tasks_") + + +# ── no suite run may stop or restart the daemon (issue #1337, item 2) ──────── +# +# The two routes `_guard_stop_all_hermeticity` cannot see: the client-side +# restart's `os.kill`, and a child process spawned from a test. Each guard is +# pinned in BOTH directions — the #455 lesson, and the acceptance #1337 asks +# for: "a check that refuses everything would be as wrong as one that refuses +# nothing." +# +# Every positive control below calls the guarded path and is safe by +# construction: the refusal happens *before* any signal is sent or any process +# is spawned, so the forbidden act never occurs. The pid 4242 is never signalled +# and `emrg server stop` is never executed. + + +def test_guard_refuses_signalling_the_daemon(): + """Positive: the client-side restart's kill is refused, term and kill. + + The wrapper's identity is asserted first, in the same spirit as + `test_guard_installed_in_both_modules` above: the behavioural controls below + would also pass if someone patched the kill inside the test body, whereas a + refusal that comes from the *installed* guard is what a real regression + would meet. + """ + import signal + + import emrg.client.daemon_manager as daemon_manager + + assert type(daemon_manager.os).__name__ == "_NoSignalOs" + # SIGKILL is Unix-only; SIGTERM is the signal the restart path actually uses. + signals = [signal.SIGTERM] + ( + [signal.SIGKILL] if hasattr(signal, "SIGKILL") else [] + ) + for sig in signals: + with pytest.raises(AssertionError, match="red-line violation"): + daemon_manager.os.kill(4242, sig) + + +def test_guard_still_allows_a_liveness_probe(daemon_kill_is_a_probe): + """Negative: a genuine probe is not a signal — `kill(pid, 0)` must reach `os`. + + This is the arm that keeps the guard from being "refuse every os.kill": + `daemon_manager` uses the probe to wait for the old daemon to die + (rant 2026-08-18T12:49:09 ②), so refusing it would turn a benign check into + an error. + + POSIX only, and that is a property of the *value* rather than a shortcut. On + Windows `0` is `signal.CTRL_C_EVENT`, so the call this test exists to allow is + the call the guard must refuse there — and it is not a call this file may make + to find that out. Measured on the windows-2025 leg of run 35252423114: this + test passed at 17:33:41.5 and the *next* test that spawned a child died at + 17:33:44.4 with `KeyboardInterrupt` at `threading.py:359` (`waiter.acquire()` + inside `Condition.wait`, i.e. `Thread.start()` waiting on `_started`) — the + Ctrl+C this line sent to its own console group, noticed at the next blocking + call. So the Windows half is pinned where it can be, without sending it: + `test_kill_zero_is_a_probe_on_posix_and_a_ctrl_c_on_windows` for the decision + and `test_the_refusal_asks_that_decision_instead_of_assuming_it` for the wiring. + """ + if not daemon_kill_is_a_probe(0): + pytest.skip("kill(pid, 0) is CTRL_C_EVENT here — see the two tests named above") + + import os as real_os + + import emrg.client.daemon_manager as daemon_manager + + # Our own pid is alive, so the real probe returns normally. + assert daemon_manager.os.kill(real_os.getpid(), 0) is None + + +def test_guard_refuses_spawning_the_stop_or_restart_cli(tmp_path): + """Positive: a child that would stop/restart the daemon is refused. + + The guard raises before anything is spawned, so the refusal itself is + observable without running a CLI. Both routes are covered because an instance + need not use the installed script: `python -m emrg server restart` is the same + act, and `pkill -f emrg.server` is the one a shell reaches for. + + Every argv names a program in a directory this test created. The guard reads + the *shape* of the argv — the `emrg`/`emrgd` basename carrying a `stop` or + `restart` verb, or a signalling command whose text names emrg — and a stub's + basename is the only part of that shape which is load-bearing here. Pointed at + the real PATH instead, this control would be the one place where a regression + to "refuse nothing" *is* the incident it forbids (`killall emrgd` on a live + host), i.e. a test whose safety depends on the code it is testing. With stubs, + a regression still fails the test — the spawn raises nothing — and the host is + untouched. + + The wrapper and shell rows are here because the first revision of this guard + read only `tokens[0]` and let all of them through (measured): a shell runs the + line it is handed, and `env`/`nohup`/`timeout`/`nice` hand the program over, so + the *same* act arrives as somebody else's argv. Every program in those rows is + a stub as well, the wrapper included — so the "a regression must still be + harmless" property holds for them too. + + The last block is the veto of cycle `cyc20260918-043412`, which measured seven + spellings that glued shell punctuation to the program or the verb and got + through the token-literal reading: `(emrg`, `$(which emrg)`, `` `emrg ``, a + quoted `'emrg'`, `env -S "…"`, and `(stop)` as an argv element with no shell + involved. `$(which emrg) server stop` is the one that must never come back — it + is how a test asks for the installed script's path, and it reaches the **live** + daemon. Those rows carry a `PATH` pinned to the stubs, because the substitution + is resolved by the shell *if* the guard ever lets it through: with the stub + `which` on `PATH` printing nothing, a regression expands to an empty command + instead of the host's real `emrg`. + """ + import subprocess + + stubs = tmp_path / "bin" + stubs.mkdir() + for name in ( + "emrg", + "emrgd", + "pkill", + "killall", + "kill", + "python", + "sh", + "bash", + "env", + "nohup", + "timeout", + "nice", + "which", + ): + script = stubs / name + script.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + script.chmod(0o755) + + for argv in ( + [str(stubs / "emrg"), "server", "stop"], + [str(stubs / "emrg"), "server", "restart"], + [str(stubs / "python"), "-m", "emrg", "server", "stop"], + [str(stubs / "python"), "-m", "emrg", "server", "restart"], + [str(stubs / "pkill"), "-f", "emrg.server"], + [str(stubs / "killall"), "emrgd"], + f"{stubs / 'pkill'} -f 'python -m emrg'", + # a shell runs the line it is handed: the same act, one level in + [str(stubs / "sh"), "-c", f"{stubs / 'emrg'} server stop"], + [str(stubs / "sh"), "-c", f"{stubs / 'emrg'} server stop && echo done"], + [str(stubs / "bash"), "-lc", f"{stubs / 'python'} -m emrg server restart"], + # a wrapper hands the program over: the emrg program is not tokens[0] + [str(stubs / "env"), str(stubs / "emrg"), "stop"], + [str(stubs / "nohup"), str(stubs / "emrg"), "server", "restart"], + [str(stubs / "timeout"), "5", str(stubs / "emrg"), "server", "stop"], + [str(stubs / "nice"), "-n", "5", str(stubs / "pkill"), "-f", "emrg.server"], + ): + with pytest.raises(AssertionError, match="red-line violation"): + subprocess.Popen(argv) + + # shell punctuation glued to the program or the verb (veto cyc20260918-043412). + # `emrg` is a stub here *and* on `PATH`, so the shell cannot reach the host's + # real CLI even if the refusal this block asserts were removed. + pinned = {"PATH": str(stubs)} + for argv in ( + [str(stubs / "sh"), "-c", "(emrg server restart)"], + [str(stubs / "sh"), "-c", "$(which emrg) server stop"], + [str(stubs / "sh"), "-c", "`emrg server stop`"], + [str(stubs / "sh"), "-c", "'emrg' server stop"], + [str(stubs / "sh"), "-c", 'env -S "emrg server stop"'], + [str(stubs / "sh"), "-c", "emrg server (stop)"], + [str(stubs / "sh"), "-c", "(pkill -f emrg.server)"], + # a newline separates two commands exactly as `;` does + [str(stubs / "sh"), "-c", "emrg --help\nemrg server stop"], + # no shell at all: the verb itself is glued to punctuation + [str(stubs / "emrg"), "server", "(stop)"], + # characters a shell DROPS rather than a token edge: an escape and quote + # concatenation. Both were measured to reach the live daemon — a stub + # `emrg` on PATH ran, with argv `server stop`, for each — and both were + # allowed by the edge-strip rule, because `str.strip` never saw them + # (cycle cyc20260918-071815, reviewing this PR's own veto fix). + [str(stubs / "sh"), "-c", "\\emrg server stop"], + [str(stubs / "sh"), "-c", "'e''mrg' server stop"], + [str(stubs / "sh"), "-c", "e\\mrg server stop"], + [str(stubs / "sh"), "-c", "emrg sto\\p"], + [str(stubs / "sh"), "-c", "e''mrg server stop"], + # the accepted over-refusal: the shell reads this as ONE command name with + # spaces (`emrg server stop: command not found`, measured), but the split + # here is whitespace-only by design, so it arrives as the act's own token + # stream and is refused. Cheap and loud; no test writes it. + [str(stubs / "sh"), "-c", "emrg\\ server\\ stop"], + ): + with pytest.raises(AssertionError, match="red-line violation"): + subprocess.Popen(argv, env=pinned) + + +def test_a_list_argv_is_not_shell_dropped(token_normaliser): + """The windows-2025 failure of run 35286596898, pinned on any host. + + `str(tmp_path / "emrg")` is backslash-separated on Windows, and a drop rule + that deletes backslashes *anywhere* in a token moved the basename off `emrg` — + so the refusal stopped firing and the corpus above really spawned its stub + (`OSError: [WinError 193] %1 is not a valid Win32 application`), while the + known-cost row below stopped being refused. `Path(...).name` on Windows is + `ntpath.basename`, so the guard's own comparison can be evaluated the way + Windows would, here, on any host. + + Both directions are pinned: the drop is confined to tokens a shell re-parses, + and inside one it still closes the escapes and quotes it was added for. + """ + win_spelled = "C:\\ws\\bin\\emrg" + + # a list argv reaches exec/CreateProcess literally — nothing drops anything + assert token_normaliser(win_spelled) == win_spelled + assert ntpath.basename(token_normaliser(win_spelled)) == "emrg", ( + "the Windows spelling must keep its basename on `emrg` — the refusal keys " + "on it, and this is the comparison that silently changed on the Windows leg" + ) + + # where a shell DOES run it, the drop is what closes the measured holes + assert token_normaliser("\\emrg", shell_parsed=True) == "emrg" + assert token_normaliser("e''mrg", shell_parsed=True) == "emrg" + assert token_normaliser("sto\\p", shell_parsed=True) == "stop" + + +def test_a_windows_spelled_argv_is_judged_as_windows_would_judge_it(daemon_spawn_refusal): + """The windows-2025 failure of run 35287972569, pinned on any host. + + The revision before this one confined the drop rule to shell-parsed tokens and + stopped there, so a shell-parsed token was still read **one way only**. A string + argv goes to a shell (`shell=True`), and a token in it may be a Windows path + whose separators `cmd.exe` does not delete: the drop turned `C:\\ws\\bin\\pkill` + into `C:wsbinpkill`, the basename stopped being `pkill`, the signaller check + missed, and the corpus above really spawned the stub it names (`OSError: + [WinError 193] %1 is not a valid Win32 application`; 1 failed / 2939 passed on + that leg). + + The root cause is wider than the drop rule: `Path(...).name` is the *host's* + basename, so the same argv was judged differently depending on which machine + read it. The guard now asks both path flavours (`PurePosixPath` / + `PureWindowsPath`) of every reading, which makes this test classify the Windows + spelling exactly as the Windows leg does, wherever it runs. + """ + win_bin = "C:\\ws\\bin" + + # a string argv is shell-parsed (`shell=True`) and its program is a Windows path + assert daemon_spawn_refusal(f"{win_bin}\\pkill -f 'python -m emrg'") is True + # the same act one level in, and a wrapper handing the program over + assert daemon_spawn_refusal([f"{win_bin}\\sh", "-c", f"{win_bin}\\emrg server stop"]) is True + assert daemon_spawn_refusal([f"{win_bin}\\env", f"{win_bin}\\emrg", "stop"]) is True + assert daemon_spawn_refusal([f"{win_bin}\\emrg", "server", "restart"]) is True + + # the mirror direction: a Windows path that is not the program stays allowed, so + # "read both flavours" is not simply "refuse anything with a backslash" + assert daemon_spawn_refusal([f"{win_bin}\\emrg-notes.txt", "server", "stop"]) is False + assert daemon_spawn_refusal(f"{win_bin}\\pytest -q tests/") is False + + +def test_an_emrg_named_path_costs_a_false_refusal(tmp_path, daemon_spawn_refusal): + """The mirror direction: an `emrg` basename in *any* position can make a verb decisive. + + Measured by this PR's veto (`cyc20260918-043412`) with this repo's own path: + the program scan accepts any token whose basename is `emrg`/`emrgd`, so + `git -C log --grep restart` is refused — a false refusal for an + argv that runs nothing but git. + + Pinned rather than repaired, and the reason is the guard's own bias: the repair + is to require *command position*, and the hole that opens is an `emrg` program + handed over by anything not on the wrapper list (`xargs -I{} emrg {} stop`). + A false refusal fails at its own assertion with the red line in the message; a + false allowance SIGTERMs the live daemon mid-suite. So the asymmetry is kept, + and what this pins is its exact scope: the *verb* is what decides, so the same + path with a read-only verb stays allowed. + """ + emrg_named = tmp_path / "emrg" + emrg_named.mkdir() + + assert not daemon_spawn_refusal(["git", "-C", str(emrg_named), "log"]) + assert daemon_spawn_refusal( + ["git", "-C", str(emrg_named), "log", "--grep", "restart"] + ), "the known cost narrowed: re-check the docstring in conftest before relaxing it" + + +def test_guard_refuses_the_cli_stop_fallback_signal(): + """Positive: `emrg.__main__`'s own SIGTERM route is guarded too. + + `emrg/__main__.py::_stop_daemon` signals the pid it read from a `ping` frame + and does **not** pass through `emrg._stop_all`'s five stop functions, so + `_guard_stop_all_hermeticity` never saw it: a test calling that function + in-process would SIGTERM the daemon this evolution is running on, and the two + files that describe the stop path said so only in prose (issue #1337, item 2's + three named routes — in-process, subprocess, client-side restart — are only + covered when this one is). + + The refusal comes before any signal, so pid 4242 is never signalled, and the + shim's identity is asserted first so the refusal is the *installed* guard's + rather than one a test body arranged. + """ + import signal + + import emrg.__main__ as cli_mod + + assert type(cli_mod.os).__name__ == "_NoSignalOs" + with pytest.raises(AssertionError, match="red-line violation"): + cli_mod.os.kill(4242, signal.SIGTERM) + # and it names the module it fired in — two shims, one rule, so the message is + # the only thing that says which route was reached + with pytest.raises(AssertionError, match=r"emrg\.__main__ tried to signal"): + cli_mod.os.kill(4242, signal.SIGTERM) + + +def test_the_cli_shim_still_answers_everything_else(): + """Negative: only `kill`/`killpg` are replaced — the CLI runs on the rest of `os`.""" + import os + + import emrg.__main__ as cli_mod + + assert cli_mod.os.getpid() == os.getpid() + assert cli_mod.os.sep == os.sep + assert cli_mod.os.environ is os.environ + + +def test_guard_allows_read_only_spawns(daemon_spawn_refusal): + """Negative: the refusal keys on the verb, not on "it mentions emrg". + + The suite spawns git, node, pytest and the `emrg` CLI itself for read-only + verbs, and a guard that refused every emrg invocation would break those tests + and teach the next reader to distrust it. Those shapes have to stay allowed, + and this test pins that on the argv predicate — which is the guard's own + question, and the one thing only this file can ask. Read together with + `test_guard_refuses_spawning_the_stop_or_restart_cli` above, which drives the + installed `Popen` wrapper and sees it raise, the refusal is pinned in both + directions: a guard that refused everything fails here, and a guard that + refused nothing fails there. + + Nothing is spawned here, deliberately. Two earlier revisions of this test did + spawn, and the windows-2025 leg reddened on the later one twice: first a + `UnicodeDecodeError` raised in the parent while decoding the child, then — + once the child's bytes were captured instead of decoded — a + `KeyboardInterrupt` reported in the parent while its main thread was blocked + in a `Condition.wait`. The second is the reason this test spawns nothing at + all: the spawn is not this guard's subject, `tests/test_cli_output_encoding.py` + already runs this CLI on every platform and pins its output in both codecs, + and the suite's other spawns already pin that an allowed argv reaches `Popen`. + What this guard owns is which argv it refuses. + """ + import sys + + allowed = [ + # the CLI forms — the discriminating twins of `server stop` / `restart` + [sys.executable, "-m", "emrg", "--help"], + [sys.executable, "-m", "emrg", "server", "status"], + [sys.executable, "-m", "emrg", "rant", "hello"], + ["emrg", "--version"], + ["emrg", "update"], + # the same wrappers and shells carrying a read-only verb: the refusal keys + # on the verb, so closing the wrapper spellings must not close these + ["sh", "-c", "emrg server status"], + ["bash", "-lc", "python3 -m emrg --help"], + # a read-only emrg command whose *pipeline* carries the verb as data + ["sh", "-c", "emrg --help | grep stop"], + ["env", "emrg", "--version"], + ["timeout", "5", "emrg", "server", "status"], + # emrg mentioned without being the program being run + ["git", "log", "--grep", "emrg"], + ["nice", "-n", "5", "git", "log", "--grep", "emrg"], + # the verb present as *data* rather than as the verb being asked for + ["git", "commit", "-m", "stop"], + # everything else the suite spawns + ["git", "status"], + ["node", "--version"], + ["uv", "run", "python", "-m", "pytest", "tests/", "-v"], + [sys.executable, "-c", "print('ok')"], + ] + for argv in allowed: + assert not daemon_spawn_refusal(argv), argv + + +def test_guard_refuses_signalling_the_daemon_group(): + """Positive: `killpg` is refused too — the same act, wider blast radius. + + `__getattr__` delegates every attribute of the `os` substitute to the real + module, so before this arm `killpg` was the one signal route *out* of the + guard: measured, `guard.killpg(4242, 15)` reached the stand-in `os` and + returned normally. Nothing in the restart path signals a group today, which is + why this is a scope arm rather than a live hole — but a group signal is the + same act with a wider target, and the wrapper's own docstring would otherwise + be the only place saying so. + + The group `4242` is never signalled: the refusal precedes the delegation, and + the assertion is on the refusal. This is also the arm that keeps the `os` + substitute from being "refuse `kill`": the passthrough test below pins what + must still delegate. + """ + import signal + + import emrg.client.daemon_manager as daemon_manager + + assert type(daemon_manager.os).__name__ == "_NoSignalOs" + with pytest.raises(AssertionError, match="red-line violation"): + daemon_manager.os.killpg(4242, signal.SIGTERM) + + +def test_the_guarded_popen_is_still_a_type(): + """The suite-wide `Popen` patch must not make `isinstance` unanswerable. + + `isinstance(proc, subprocess.Popen)` is the question a reader asks about a + child, and the revision before this one installed the refusal as a plain + function over the attribute — which answers that question with `TypeError: + isinstance() arg 2 must be a type, a tuple of types, or a union` (measured). No + test asks it today, so nothing was red; the failure it would produce names + `isinstance`, not the red line, and would send the next reader to the wrong + place. A subclass refuses identically and keeps the question answerable, and + the name is kept because that is what the attribute is called. + + The child spawned here is read-only — the guard allows it, which is the other + half of this test: a guard that refused every spawn would fail on its own + setup line rather than on an assertion. + """ + import subprocess + import sys + + assert isinstance(subprocess.Popen, type) + assert subprocess.Popen.__name__ == "Popen" + # Identity, the same way `test_guard_refuses_signalling_the_daemon` asserts + # `type(daemon_manager.os).__name__ == "_NoSignalOs"`: the installed object must + # be the guard's own subclass, not the real class left in place. A bare `type` + # check would pass on an unpatched `subprocess.Popen`; under the previous + # function patch this line raises `AttributeError` (a function has no `__mro__`) + # — i.e. the arm discriminates in the direction the property is about. + installed = subprocess.Popen + assert installed is not installed.__mro__[1], ( + "the installed subprocess.Popen is the real class, not the guard's subclass" + ) + assert installed.__mro__[1].__name__ == "Popen" + + proc = subprocess.Popen( + [sys.executable, "-c", "print('guarded')"], + stdout=subprocess.PIPE, + text=True, + encoding="utf-8", + errors="replace", + ) + # The line that raised `TypeError` before this arm exists. + assert isinstance(proc, subprocess.Popen) + assert proc.communicate()[0].strip() == "guarded" + + +def test_kill_zero_is_a_probe_on_posix_and_a_ctrl_c_on_windows(daemon_kill_is_a_probe): + """`kill(pid, 0)` means "does this pid exist" on POSIX and "interrupt" on Windows. + + The guard delegates `kill(pid, 0)` so the restart path's liveness wait keeps + working, and the whole justification for that delegation is the POSIX reading. + It is not the Windows one: `signal.CTRL_C_EVENT` is **0**, and CPython's + `os_kill_impl` sends `CTRL_C_EVENT` through `GenerateConsoleCtrlEvent(pid, + sig)` — a Ctrl+C to that process group. A guard that delegated that value on + Windows would permit exactly the act it exists to refuse, and would do it by + signalling the console the test process is sitting on. + + Pinned as a table rather than by calling `os.kill`: the Windows row cannot be + exercised on this runner without sending the signal it describes. + """ + assert daemon_kill_is_a_probe(0, "linux") + assert daemon_kill_is_a_probe(0, "darwin") + assert not daemon_kill_is_a_probe(0, "win32") + # Cygwin's `os.kill` is the POSIX one and Cygwin Python has no + # `signal.CTRL_C_EVENT`, so the boundary is the `win*` platform string and + # not "anything that runs on Windows". + assert daemon_kill_is_a_probe(0, "cygwin") + # every real signal is a signal — zero is the only value in question + assert not daemon_kill_is_a_probe(15, "linux") + assert not daemon_kill_is_a_probe(9, "win32") + + +def test_the_refusal_asks_that_decision_instead_of_assuming_it(daemon_kill_refusal): + """The wrapper routes `kill` through the probe decision, both ways. + + Driven over a stand-in `os`, so the delegated branch appends to a list + instead of reaching a real `os.kill`: the Windows row's assertion is that + nothing is delivered, which a test cannot make by delivering it. + """ + delivered: list = [] + + class _RecordingOs: + def kill(self, pid, sig): + delivered.append((pid, sig)) + + delegating = daemon_kill_refusal(_RecordingOs(), is_probe=lambda sig: True) + assert delegating.kill(4711, 0) is None + assert delivered == [(4711, 0)] + + refusing = daemon_kill_refusal(_RecordingOs(), is_probe=lambda sig: False) + with pytest.raises(AssertionError, match="red-line violation"): + refusing.kill(4711, 0) + assert delivered == [(4711, 0)], "the refused call reached an os.kill anyway" + + +def test_other_os_attributes_pass_through_unchanged(): + """The guard replaces `kill` and nothing else — `daemon_manager` uses more of `os`.""" + import os + + import emrg.client.daemon_manager as daemon_manager + + installed = daemon_manager.os # the wrapper the autouse guard installed + assert installed.getpid() == os.getpid() + assert installed.path is os.path + assert installed.sep == os.sep