diff --git a/scripts/check-merge-plan-suite.py b/scripts/check-merge-plan-suite.py index 9531b1f5..38517ac8 100644 --- a/scripts/check-merge-plan-suite.py +++ b/scripts/check-merge-plan-suite.py @@ -215,12 +215,18 @@ already been reported as defects (one measured in a hand-built landing-tree worktree, and both re-measured on a real landing tree the next cycle): it has **no `.venv`**, so `uv run pytest` there reports that no suite ran, and it has **no `node_modules`**, so the -GUI Node suite fails one unrelated spawn-args test (`python=python3 (expected -.venv/bin/python)`) - on that real tree, `daemon_client` was 68 passed / 1 failed without -the links and 69 / 0 with them. Neither is a verdict on the tree, so the note prints the +GUI Node suite fails two files - the spawn-args test (`python=python3 (expected +.venv/bin/python)`) and `test/integration.test.js` (`Cannot find module 'ws'`), because +the GUI's dependencies (`ws` among them) live in `emrg/gui/node_modules`, not in the +repository root's. The node link therefore names the **GUI's own** directory: it was +first written as the repository root's, which is empty, so the line that was supposed to +fix the suite fixed nothing (measured 2026-09-19, `cyc20260919-060712`: on landing tree +`f96d6515c734`, 125 passed / 2 failed with no link, 126 / 1 with the python link alone, +and 126 / 0 / 8 skipped with both - identical with the root link, because it links an +empty directory). Neither failure is a verdict on the tree, so the note prints the two commands that fix it (the main checkout's interpreter with `PYTHONPATH` pointed at -the worktree; the main checkout's `node_modules` and `.venv` linked in), and says to -compare worktree runs with worktree runs. +the worktree; the main checkout's `emrg/gui/node_modules` and `.venv` linked in), and says +to compare worktree runs with worktree runs. What the run leaves behind -------------------------- @@ -1069,10 +1075,15 @@ def _kept_note(path: Path, tree_sha: str | None = None) -> None: print(f" git worktree remove --force {path}") print( " It has no .venv and no node_modules, like any fresh worktree: `uv run pytest`\n" - " there reports that no suite ran, and the GUI Node suite fails one unrelated\n" - " spawn-args test (python=python3, expected .venv/bin/python). Measured on a real\n" - " landing tree (2026-09-17): `daemon_client` is 68 passed / 1 failed without the\n" - " links below and 69 / 0 with them. Both remedies, against this tree:" + " there reports that no suite ran, and the GUI Node suite fails two files - the\n" + " spawn-args test (python=python3, expected .venv/bin/python) and\n" + " test/integration.test.js (Cannot find module 'ws'). Measured on a real landing\n" + " tree (2026-09-19, f96d6515c734): the GUI suite is 125 passed / 2 failed with\n" + " neither link, 126 / 1 with the python link alone, 126 / 0 / 8 skipped with both.\n" + " The node link must point at the GUI's OWN node_modules: the repository root has\n" + " none at all (there is no root package.json either), so `ln -sfn` from it leaves a\n" + " dangling symlink - as indistinguishable from linking nothing as an empty directory\n" + " would be (126 / 1 either way). Both remedies, against this tree:" ) # `cd` before the interpreter, not only `PYTHONPATH`: the harness's own `_suite_verdict` # passes `cwd=str(worktree)` *and* `_suite_env`'s pinned `PYTHONPATH` for the same @@ -1087,7 +1098,7 @@ def _kept_note(path: Path, tree_sha: str | None = None) -> None: f" python: cd {path} && PYTHONPATH={path} " f"{main}/.venv/bin/python -m pytest tests/ -q" ) - print(f" node: ln -sfn {main}/node_modules {path}/emrg/gui/node_modules") + print(f" node: ln -sfn {main}/emrg/gui/node_modules {path}/emrg/gui/node_modules") print(f" ln -sfn {main}/.venv {path}/.venv") print(" Then compare worktree runs with worktree runs, never with main-checkout runs.") diff --git a/tests/test_check_merge_plan_suite.py b/tests/test_check_merge_plan_suite.py index 9e5f4032..295875b0 100644 --- a/tests/test_check_merge_plan_suite.py +++ b/tests/test_check_merge_plan_suite.py @@ -318,6 +318,134 @@ def _worktree_listing(repo: Path) -> str: return _git(repo, "worktree", "list", "--porcelain") +#: The GUI's dependencies live here, not in the repository root's `node_modules`. +_GUI_NODE_MODULES = "emrg/gui/node_modules" + + +#: Where the note's `ln -sfn ` line joins its two operands. The note +#: interpolates them unquoted, so a path may itself contain a space and whitespace is not a +#: splitter for it — but the source always ends with `node_modules`, so the space that +#: separates it from the destination is the line's one unambiguous seam. +_OPERAND_SEAM = "node_modules " + + +def _node_remedy(note: str) -> tuple[str, str]: + """The `(source, destination)` of the node remedy the note prints. + + Read at the seam, never on whitespace. Measured 2026-09-19 (`cyc20260919-065231`), + the previous `line.split()` reader required exactly five fields, so a path carrying a + space made it raise `the note prints no node remedy` while the note printed the remedy + on that very line — a wrong cause, the same class as the `_worktree_listing` backslash + defect one function up (paths must not be compared by splitting them). Both halves are + still read: a line whose source is the repository root's `node_modules` answers False. + + A line the seam cannot be found in is reported as *unmeasurable*, with the line: a + reshaped note (quoted operands, or a source that is not a `node_modules` at all) needs + this reader taught, and must not read as "no remedy printed". + """ + for line in note.splitlines(): + stripped = line.strip() + if not stripped.startswith("node:"): + continue + command = stripped[len("node:") :].strip() + if not command.startswith("ln -sfn "): + continue + operands = command[len("ln -sfn ") :].rstrip() + cut = operands.find(_OPERAND_SEAM) + if cut == -1: + raise AssertionError( + "the note's node line is not `ln -sfn /node_modules `" + f", so its two paths cannot be read apart: {stripped!r}" + ) + source = operands[: cut + len("node_modules")] + return source, operands[cut + len(_OPERAND_SEAM) :].strip() + raise AssertionError(f"the note prints no node remedy:\n{note}") + + +def _node_remedy_links_the_guis_own_deps(note: str) -> bool: + """True when the node remedy links the *GUI's* `node_modules`, not the root's. + + The source is the load-bearing half, and the destination cannot discriminate it: + the root form (`ln -sfn
/node_modules /emrg/gui/node_modules`) contains + the string `emrg/gui/node_modules` too, which is why the assertion that stood here + passed for a link that fixed nothing. Measured 2026-09-19 (`cyc20260919-060712`) on + landing tree `f96d6515c734`: the repository root has no `node_modules` at all — there + is no root `package.json` either, so `ln -sfn` there leaves a *dangling symlink* — + and linking it is indistinguishable from linking nothing: the GUI suite reports 126 + passed / 1 failed (`test/integration.test.js`, `Cannot find module 'ws'`) either way, + and 126 / 0 / 8 skipped once the GUI's own directory is linked. + """ + source, destination = _node_remedy(note) + return source.endswith(_GUI_NODE_MODULES) and destination.endswith(_GUI_NODE_MODULES) + + +def test_the_node_remedy_reader_rejects_the_root_form() -> None: + """The instrument's control: the wrong link source must read as wrong. + + Without this, `_node_remedy_links_the_guis_own_deps` could return True for anything + (the shape the previous assertion effectively had) and the arm it now backs would + still pass. Three shapes, because the two halves fail independently: the right + source with the right destination, the root source, and a destination that is not + the GUI's directory at all. + """ + def note(source: str, destination: str = "/kept/emrg/gui/node_modules") -> str: + return f" node: ln -sfn {source} {destination}\n" + + assert _node_remedy_links_the_guis_own_deps(note("/main/emrg/gui/node_modules")) + assert not _node_remedy_links_the_guis_own_deps(note("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/main/node_modules")) + assert not _node_remedy_links_the_guis_own_deps( + note("/main/emrg/gui/node_modules", "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/kept/node_modules") + ) + + +def test_the_node_remedy_reader_reads_paths_that_carry_a_space() -> None: + """The note's paths are unquoted, so a space in one is ordinary, not malformed. + + Measured 2026-09-19 (`cyc20260919-065231`) against the `line.split()` reader this one + replaced, on the two paths a host really has: `
` = `/Users/John Smith/main`, and + a kept directory named `kept dir`. Both made it raise `the note prints no node remedy` + — the remedy was printed on that line, so the message named the wrong cause and the + arm guarding the remedy would have redded on such a host while passing here (pytest's + `tmp_path` carries no space). It answers `True` for a spacey source, a spacey + destination, and for both at once, and still `False` for the root form spelled with a + spacey main checkout. + """ + def note(source: str, destination: str) -> str: + return f" node: ln -sfn {source} {destination}\n" + + spacey_main = "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/Users/John Smith/main" + spacey_kept = "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/Users/John Smith/tmp/kept dir" + assert _node_remedy_links_the_guis_own_deps( + note(f"{spacey_main}/emrg/gui/node_modules", f"{spacey_kept}/emrg/gui/node_modules") + ) + assert _node_remedy_links_the_guis_own_deps( + note("/main/emrg/gui/node_modules", f"{spacey_kept}/emrg/gui/node_modules") + ) + assert not _node_remedy_links_the_guis_own_deps( + note(f"{spacey_main}/node_modules", "/kept/emrg/gui/node_modules") + ) + + +def test_the_node_remedy_reader_reports_a_reshaped_line_as_unmeasurable() -> None: + """Two different failures must not share one message (`cyc20260919-065231`). + + A line this reader cannot take apart — quoted operands are the shape an outside review + measured, where the old reader answered a silent `False` (the quote landing in the + `endswith` comparison) — is a *failure to measure*, so it raises with the line. "The + note prints no node remedy" is reserved for a note that prints none, which is the + distinction that keeps a format change from being read as a missing remedy. + """ + quoted = ( + ' node: ln -sfn "/main/emrg/gui/node_modules" "/kept/emrg/gui/node_modules"\n' + ) + with pytest.raises(AssertionError, match="cannot be read apart"): + _node_remedy(quoted) + + no_remedy = "kept /tmp/kept (tree abc)\n git worktree remove --force /tmp/kept\n" + with pytest.raises(AssertionError, match="prints no node remedy"): + _node_remedy(no_remedy) + + def _worktree_listing_names(listing: str, path: Path) -> bool: """Does this listing name `path` as one of its worktrees? @@ -387,7 +515,8 @@ def test_a_kept_worktree_is_the_tree_the_run_measured( # The note names the path, the tree, and the two traps every fresh worktree has - # no `.venv` (so `uv run pytest` there reports that no suite ran) and no - # `node_modules` (so one unrelated GUI spawn-args test reds). Both were reported + # `node_modules` (so the GUI suite reds the spawn-args test and + # `test/integration.test.js`, which cannot resolve `ws`). Both were reported # as defects before, which is why the tool says them out loud - and it prints the # remedy for each, because a warning without one costs the next reader the same # discovery. Shape-matched, not path-matched: the note prints git's spelling of the @@ -396,7 +525,7 @@ def test_a_kept_worktree_is_the_tree_the_run_measured( assert ".venv" in kept.stdout and "node_modules" in kept.stdout assert f"PYTHONPATH={kept_dir}" in kept.stdout assert "-m pytest tests/ -q" in kept.stdout - assert "emrg/gui/node_modules" in kept.stdout + assert _node_remedy_links_the_guis_own_deps(kept.stdout), kept.stdout # The removal line it prints is the one that works. _git(repo, "worktree", "remove", "--force", str(kept_dir)) diff --git a/tests/test_start_window_env_pairing.py b/tests/test_start_window_env_pairing.py new file mode 100644 index 00000000..c990447e --- /dev/null +++ b/tests/test_start_window_env_pairing.py @@ -0,0 +1,108 @@ +"""One variable, two entry points: read the pairing where both sides exist. + +Why this file exists +-------------------- +Issue #1276 item 5 gave the host a lever for a slow cold start — `EMRG_START_TIMEOUT` +— and the point of the lever is that it is *one* variable reaching *both* entry +points: the CLI/TUI (`emrg/client/daemon_manager.py`, #1402) and the GUI +(`emrg/gui/daemon_client.js`, #1404). Each side is pinned on its own: this repo's +tests read `dm._START_WINDOW_ENV`, the GUI's read the literal `EMRG_START_TIMEOUT`. +What nothing pinned, until here, is that the two sides are the *same name* — and the +gap is not hypothetical. Measured 2026-09-19 (`cyc20260919-060712`) by renaming the +Python constant to `EMRG_START_WINDOW_TIMEOUT` and touching nothing else: the Python +suite stayed green (38 passed, because those assertions read the constant and so +*follow* a rename) and the GUI's stayed green (77 passed, because its literal had not +moved). A host's `EMRG_START_TIMEOUT=30` would have stopped reaching the TUI with no +red anywhere — the lever #1402 exists to provide, unhooked in silence. + +How the two sides are read +-------------------------- +The Python side is imported and read as a value. The GUI side is a file in another +language, so its declaration is *parsed* — and a parser that is the instrument is +only as good as its controls, which is what `_JS_SAMPLE_*` below are for. The +extractor is deliberately name-agnostic: it returns whatever name the file declares, +which is what makes the equality assertion a comparison rather than a re-statement of +what it is looking for. + +Named limit +----------- +This pins the *name*, not that either side honours it. That the value reaches the +wait is covered per side (`tests/test_daemon_start_diagnostics.py` drives the loop +end to end; `emrg/gui/test/daemon_client.test.js` feeds both spawn sites). And the +GUI half is only comparable once it exists: before that, this file reports the +comparison as unmeasurable rather than as a pass. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from emrg.client import daemon_manager as dm # noqa: E402 + +REPO_ROOT = Path(__file__).resolve().parent.parent +GUI_CLIENT = REPO_ROOT / "emrg" / "gui" / "daemon_client.js" + +#: The declaration the GUI side would carry. Matched as `const = "name";` so the +#: extractor reports the name the file *has*, never a name it was told to expect. +_JS_DECLARATION = re.compile(r'^const\s+\w*START_WINDOW\w*\s*=\s*"([^"]*)"\s*;', re.MULTILINE) + +_JS_SAMPLE_DECLARED = 'const START_WINDOW_ENV = "EMRG_START_TIMEOUT";\n' +_JS_SAMPLE_DECLARED_DIFFERENTLY = 'const START_WINDOW_ENV = "EMRG_START_WINDOW_TIMEOUT";\n' +_JS_SAMPLE_SILENT = "const SPAWN_WAIT_MS = 5_000;\n" + + +def _js_env_name(text: str) -> str | None: + """The start-window variable name `text` declares, or `None` if it declares none.""" + found = _JS_DECLARATION.search(text) + return found.group(1) if found else None + + +def test_the_extractor_reads_the_name_the_file_has_not_the_one_it_wants(): + """The instrument's control, in both directions that matter. + + A parser that returned nothing unless it saw the expected name would make the + assertion below a tautology, and one that returned the expected name whatever it + saw would make it a rubber stamp. So it must report a *differing* name as that + name (that is the case a rename produces), and nothing at all when no declaration + is present (`None`, not the default). + """ + assert _js_env_name(_JS_SAMPLE_DECLARED) == "EMRG_START_TIMEOUT" + assert _js_env_name(_JS_SAMPLE_DECLARED_DIFFERENTLY) == "EMRG_START_WINDOW_TIMEOUT" + assert _js_env_name(_JS_SAMPLE_SILENT) is None + + +def test_the_python_side_names_the_documented_variable(): + """The TUI's spelling is a contract with the host's shell, so it is pinned literally. + + Every other assertion about this feature reads `dm._START_WINDOW_ENV`, which is the + right instrument for "does the window change" and the wrong one for "is it still + called that": it follows a rename. `DEVELOPMENT.md` documents `EMRG_START_TIMEOUT` + for the host, so the string is asserted here rather than left to whoever edits the + constant next — a rename is a deliberate act, and this is the red that says so. + """ + assert dm._START_WINDOW_ENV == "EMRG_START_TIMEOUT" + + +def test_the_two_entry_points_read_the_same_variable(): + """The pairing itself: whatever the GUI declares must be the name the TUI reads.""" + text = GUI_CLIENT.read_text(encoding="utf-8") + declared = _js_env_name(text) + if declared is None: + # Not a pass: with no declaration there is no pairing to read, and saying that + # out loud is the measurement (issue #1276's GUI half is PR #1404). + assert dm._START_WINDOW_ENV not in text, ( + f"{GUI_CLIENT.name} names {dm._START_WINDOW_ENV} but declares no constant " + "for it — the pairing cannot be read from this file" + ) + pytest.skip(f"{GUI_CLIENT.name} does not read the start-window variable yet") + assert declared == dm._START_WINDOW_ENV, ( + "the GUI and the TUI must read the same start-window variable: the host sets " + f"one value, and renaming either side unhooks it silently (GUI declares " + f"{declared!r}, TUI reads {dm._START_WINDOW_ENV!r})" + )