diff --git a/CHANGELOG.md b/CHANGELOG.md index 90d860edf..aeb86d09d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,10 @@ breaking changes may land in a minor release. ### Changed +- Start bmad-loop's psmux servers with `PSMUX_NO_WARM=1`: their warm pool and hidden + `__warm__` standby were never used, and dropping them cuts an idle project from 16 psmux + processes and about 935 MiB of summed working sets to 4 and about 175 MiB. The operator's + own psmux keeps its pool (#903). - Make an un-enabled builtin `[python]` plugin wholly inert: its hooks, workflows and seeds no longer run without its module (#496, #497). - Run granted project plugins from a verified out-of-tree snapshot: `{scripts}` points @@ -79,6 +83,9 @@ breaking changes may land in a minor release. directory. Under worktree isolation on Windows psmux, the session held the first unit's worktree, so its teardown journaled `worktree-teardown-degraded` and left an empty `worktrees/1` behind (#902). +- Refuse psmux option values shaped `-X=value`, `-t…` or `--`: psmux 3.4.0 rewrites the + first silently and rejects the others, so the value is now warned about and its key freed + (#905). - Fire the TUI's stale state-root note on psmux too: the psmux backend now answers which value a new pane inherits through psmux 3.4.0's named `show-environment` (#660). - Fix the OpenCode V2 SSE-reconnect end-to-end test failing on slow runners (Windows): the diff --git a/docs/multiplexer-backends.md b/docs/multiplexer-backends.md index c952d1106..71b317be3 100644 --- a/docs/multiplexer-backends.md +++ b/docs/multiplexer-backends.md @@ -101,7 +101,10 @@ session-qualified (`session:@N`). **Refused option values.** An `@` option value must read back verbatim. A value that is empty, contains a double quote or a line break, or has leading or trailing whitespace is -refused with a stderr warning, and the option reads as unset. Ordinary +refused with a stderr warning, and the option reads as unset. So are three `-`-leading +shapes that psmux's client rewrites before parsing: `-X=value` (one +letter, `=`, then at least one character), any value starting with `-t`, and `--`. Other +`-`-leading values (`-u`, `-1=x`, `--x=y`) pass. Ordinary Windows paths (spaced, UNC, apostrophed, trailing separator) pass. The project ownership tag is a hex digest of the project path, so it always passes. diff --git a/src/bmad_loop/adapters/psmux_backend.py b/src/bmad_loop/adapters/psmux_backend.py index f955c5829..a6bf72d16 100644 --- a/src/bmad_loop/adapters/psmux_backend.py +++ b/src/bmad_loop/adapters/psmux_backend.py @@ -644,6 +644,18 @@ def new_session( ) } env["PSMUX_ALLOW_NESTING"] = "1" + # psmux's warm pool (spare shells per server) and its hidden `__warm__` + # standby server serve only default-shell creates: every window this + # backend opens carries a command and every session passes `-c`, so + # neither is ever claimed, yet together they cost about 0.75 GiB of + # summed working sets per idle project (#903). The server reads this at + # boot, and this call is the one that boots it; scoped to the spawn, so + # the operator's own psmux keeps its pool. Ceilings: the server's panes + # inherit it, so a psmux session started from inside one starts cold; + # and a psmux config that says `set -g warm on` re-enables the pool at + # load, which stays the operator's call (forcing `warm off` at runtime + # would also kill the registry's shared standby). + env["PSMUX_NO_WARM"] = "1" # The initial window is a window launch too: the same PowerShell floor # as `_shell_wrap`, checked before psmux creates anything. self._require_pwsh_floor() @@ -947,10 +959,21 @@ def _transportable(value: str) -> bool: # the wire now carries them — a value that reads back different is the # same silently-unprunable window whichever hop mangled it. An empty # value is a silent server-side no-op (`-u` is the verb for that). + # + # A few `-`-leading shapes still lose the WIRE half, to the client's + # argv normalizers that run before `set-option` parses (psmux/psmux#795, + # cli.rs at v3.4.0): `normalize_flag_equals` splits `-X=value` (one + # ASCII letter, a nonempty suffix) into `-X value` and the write + # succeeds silently; `normalize_attached_target_flag` splits `-t…` into + # a target flag, and `--` ends the options, so those two fail rc 1 and + # would leave a stale value in place. Other `-` values (`-u`, `-1=x`, + # `--x=y`, `-x=`) are stored verbatim since #583. if not value: return False if '"' in value or value != value.strip(): return False + if value == "--" or value.startswith("-t") or re.match(r"-[A-Za-z]=.", value): + return False return value.splitlines() == [value] def set_session_option(self, name: str, option: str, value: str) -> None: diff --git a/tests/test_psmux_backend.py b/tests/test_psmux_backend.py index cc4202e2e..f3fadb902 100644 --- a/tests/test_psmux_backend.py +++ b/tests/test_psmux_backend.py @@ -361,6 +361,9 @@ def test_new_session_bypasses_nesting_guard(rec, monkeypatch, tmp_path): # the no-op belt: create is verified by a has-session probe afterwards assert rec.argv == ["psmux", "has-session", "-t", "=s"] assert create_kwargs["env"]["PSMUX_ALLOW_NESTING"] == "1" + # the server this call boots keeps no warm pool and spawns no `__warm__` + # standby: neither serves a `-c` session or a command-bearing window (#903) + assert create_kwargs["env"]["PSMUX_NO_WARM"] == "1" # the claude session vars are scrubbed from the create env (the psmux server # this call may cold-start would otherwise hand them to every window) assert "CLAUDE_CODE_SSE_PORT" not in create_kwargs["env"] @@ -1681,6 +1684,15 @@ def test_kill_window_unroutable_target_skips_cleanup_but_kills(monkeypatch): assert [c[0][1] for c in rec_.calls] == ["kill-window"] +# Shapes psmux's client argv normalizers rewrite or refuse before `set-option` +# parses its value (psmux/psmux#795): `-X=value` reads back as `-X value` with +# rc 0, and `-t…`/`--` fail rc 1, leaving a prior value in place. +_PSMUX_ARGV_SHAPES = ["-x=foo", "-a=b c", "-Z=1", "-t", "-tx", "-t=foo", "-target", "--"] +# Their measured neighbours, which psmux 3.4.0 stores verbatim: the refusal must +# not widen to them. +_DASH_SAFE_CONTROLS = ["-1=x", "--x=y", "-x=", "-a="] + + @pytest.mark.parametrize( "value", [ @@ -1691,6 +1703,7 @@ def test_kill_window_unroutable_target_skips_cleanup_but_kills(monkeypatch): "", # empty write is a silent server-side no-op; unset exists for this " C:\\p x", # leading space survives the wire but this backend's reads strip "C:\\p x ", # trailing space, same round-trip loss + *_PSMUX_ARGV_SHAPES, ], ) def test_set_window_option_refuses_untransportable_values(monkeypatch, capsys, value): @@ -1721,6 +1734,7 @@ def test_set_window_option_refuses_untransportable_values(monkeypatch, capsys, v "x\u00a0y", # non-ASCII whitespace no longer splits server-side (#536) "-u", # a `-`-leading value is stored, not read as a flag (psmux/psmux#583) "-flag", + *_DASH_SAFE_CONTROLS, ], ) def test_set_window_option_accepts_wire_safe_values(monkeypatch, value): @@ -1747,6 +1761,7 @@ def test_set_window_option_write_failure_warns(monkeypatch, capsys): 'C:\\a"b\\proj', # `"` cannot survive the listing's one-quote-pair strip "bad\nline", # the listing is parsed line-by-line "", # an empty write is a silent server-side no-op + *_PSMUX_ARGV_SHAPES, ], ) def test_set_session_option_refuses_untransportable_values(monkeypatch, capsys, value): @@ -1775,6 +1790,7 @@ def test_set_session_option_refuses_untransportable_values(monkeypatch, capsys, "\\\\srv\\share name\\proj", # spaced UNC — `\\` no longer collapses (#547) "C:\\Users\\O'Brien\\proj", # unspaced `'` no longer opens a quote "-u", # a `-`-leading value is stored, not read as a flag (psmux/psmux#583) + *_DASH_SAFE_CONTROLS, ], ) def test_set_session_option_accepts_transportable_values(monkeypatch, value): diff --git a/tests/test_psmux_live.py b/tests/test_psmux_live.py index 3e81d3778..bd186880c 100644 --- a/tests/test_psmux_live.py +++ b/tests/test_psmux_live.py @@ -968,6 +968,37 @@ def test_adopted_a_dash_leading_option_value_is_stored(probe): ) +@pytest.mark.parametrize( + ("value", "outcome"), + [ + ("-x=foo", (0, "-x foo")), # rewritten, and the write reports success + ("-tx", (1, "seed")), # read as a target flag: refused, the seed stays + ("--", (1, "seed")), # read as the end of options: same + ], +) +def test_premise_an_argv_flag_shaped_option_value_is_not_stored(probe, value, outcome): + # psmux's client argv normalizers rewrite `-X=value` to `-X value` and read + # `-t…`/`--` as flags before `set-option` sees its value (psmux/psmux#795). + # One row per family, so each retires on its own: red means psmux handles + # that shape differently now, and if it reads back verbatim, + # _transportable's matching refusal is droppable behind a floor bump. The + # write's rc is part of the premise, so a probe whose write never ran + # cannot pass on the seed alone. + mux, session, _ = probe + seeded = mux._run(["set-option", "-t", session, "@k", "seed"], check=False) + assert seeded.returncode == 0, f"probe setup: seeding @k failed: {seeded.stderr.strip()!r}" + written = mux._run(["set-option", "-t", session, "@k", value], check=False) + got = mux._run(["show-options", "-qv", "-t", session, "@k"], check=False) + assert got.returncode == 0, f"probe setup: reading @k back failed: {got.stderr.strip()!r}" + observed = (written.returncode, got.stdout.strip()) + assert observed == outcome, ( + f"psmux now answers the option value {value!r} with (rc, read back) {observed!r}, " + f"not {outcome!r} (psmux/psmux#795) — if it reads back verbatim, " + "psmux_backend._transportable's refusal of that shape is droppable behind a " + "floor bump" + ) + + def test_adopted_a_pane_inherits_the_servers_start_path(tmp_path): """A pane inherits the PATH of the process that started its server, and a bare `psmux` in the pane resolves through it (psmux/psmux#773); 3.3.8 @@ -1365,6 +1396,61 @@ def test_premise_a_port_and_key_pair_is_answered_by_whichever_server_it_names( (root / f"{forged}.{suffix}").unlink(missing_ok=True) +def test_adopted_a_backend_session_spawns_no_warm_standby(tmp_path, monkeypatch): + """A session the backend starts boots its server with ``PSMUX_NO_WARM=1``, + so no hidden ``__warm__`` standby joins the registry (#903): a booting + server with warm enabled spawns one and publishes ``__warm__.port`` beside + its own. Red means the create env lost the variable, or psmux stopped + honoring it at boot, and every idle project carries the standby again. + + Its own root, not the module fixture's: the raw probe sessions there boot + with warm on, so their standby would answer for this one. And an empty + config of its own: an inherited `set -g warm off` would suppress the + standby whatever the env says. + """ + mux = PsmuxMultiplexer() + if not mux.available(): + pytest.skip("psmux present but not an admitted version") + root = tmp_path / "registry" + root.mkdir() + monkeypatch.setenv("PSMUX_DATA_DIR", str(root)) + monkeypatch.delenv("PSMUX_NO_WARM", raising=False) # the backend must supply it + monkeypatch.delenv("PSMUX_WARM_POOL_SIZE", raising=False) + config = tmp_path / "empty.conf" + config.write_text("", encoding="utf-8") + monkeypatch.setenv("PSMUX_CONFIG_FILE", str(config)) + env = _new_session_env() + session = f"bmad-loop-nowarm-probe-{uuid.uuid4().hex[:8]}" + standby = root / "__warm__.port" + try: + mux.new_session(session, tmp_path) + assert (root / f"{session}.port").is_file(), "probe setup: the session has no port file" + # The standby is spawned during the server's boot, and a server may take + # psmux's whole readiness window to register on a loaded box; reading + # absence any sooner could pass while a late standby is still coming. + deadline = time.monotonic() + psmux_teardown.PSMUX_READY_DEADLINE_S + while time.monotonic() < deadline and not standby.exists(): + time.sleep(0.25) + assert not standby.exists(), ( + "a session the psmux backend started spawned a `__warm__` standby — " + "new_session's PSMUX_NO_WARM no longer reaches the server it boots" + ) + finally: + _teardown_probe_session(mux, session, env) + # A standby that registered late is a server of its own, which the + # session teardown never reaches. The root is this test's private + # tmp dir, so a kill-server scoped to it cannot touch anyone else's. + if standby.exists(): + assert str(root).startswith(str(tmp_path)) + subprocess.run( + ["psmux", "kill-server"], + env={**os.environ, "PSMUX_DATA_DIR": str(root)}, + capture_output=True, + timeout=tmux_base.TMUX_TIMEOUT_S, + check=False, + ) + + def test_adopted_a_missing_registry_root_self_heals(tmp_path): """The derived root is a path under the state root that nothing creates in advance, so the design leans on psmux ``create_dir_all``-ing it itself