From 2816f0da4e061d7f1a84e53966455937e38d9912 Mon Sep 17 00:00:00 2001 From: memtomem <118508877+memtomem@users.noreply.github.com> Date: Thu, 5 Nov 2026 08:36:55 +0900 Subject: [PATCH] fix(start): suggest gate commands only from exactly named files (#249, #251) discover_gate_commands opened its well-known files by name, so on a case-insensitive filesystem opening "makefile" found "Makefile" and the suggestion named a file that is not on disk (and "PACKAGE.JSON" was read as package.json). It now reads a file only when a directory entry has exactly its name, so the source is true to the disk and the result is the same on every filesystem. This also fixes the test that failed on macOS. The TUI Details test (#251) depended on git's error being one line: on a runner where git stops at a filesystem boundary the error is two lines, Details escapes the newline, and the raw string no longer matched. The test now pins the source-check failure (one-line and two-line) and compares the escaped form Details shows. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 +++++ src/syncmill/start.py | 27 +++++++++++++++++----- tests/test_start.py | 53 +++++++++++++++++++++++++++++++++++++++++++ tests/test_tui.py | 22 ++++++++++++++++-- 4 files changed, 100 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f97f9b8..c0ff6a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,12 @@ All notable changes to SyncMill are documented here. ### Fixed +- `syncmill start` suggests verification commands only from files whose + name matches exactly, so each suggestion names a file that is on disk. On a + case-insensitive filesystem (the macOS default) a repository with only a + `Makefile` showed `make test` as coming from `makefile`, and a file such as + `PACKAGE.JSON` was read as `package.json`; it is no longer suggested from, + as on Linux (#249). - Attaching to an isolated session right after creating it no longer fails with "session cwd does not match its isolated workspace". tmux reports the server's own directory until the new pane has moved into its worktree, so diff --git a/src/syncmill/start.py b/src/syncmill/start.py index c6241f0..e3f0a27 100644 --- a/src/syncmill/start.py +++ b/src/syncmill/start.py @@ -10,6 +10,7 @@ from __future__ import annotations import json +import os import re from dataclasses import dataclass from pathlib import Path @@ -42,17 +43,31 @@ def discover_gate_commands(repo: Path) -> list[GateSuggestion]: Read-only: files are read, never executed, so discovery cannot run a repository script. The order is the table's, and a command appears once. + + A file counts only when a directory entry has exactly its name. Opening by + name would not do: on a case-insensitive filesystem opening "makefile" + finds "Makefile" (or "package.json" finds "PACKAGE.JSON"), and the + suggestion would name a file that is not there (#249). Matching the + entries keeps the source true to the disk and the result the same on + every filesystem. """ found: list[GateSuggestion] = [] + try: + listed = {entry.name for entry in os.scandir(repo)} + except OSError: + listed = set() + + def read(name: str) -> str | None: + return _read(repo / name) if name in listed else None def add(command: str, source: str) -> None: if all(s.command != command for s in found): found.append(GateSuggestion(command, source)) - pyproject = _read(repo / "pyproject.toml") + pyproject = read("pyproject.toml") if pyproject is not None and "pytest" in pyproject: add("python -m pytest -q", "pyproject.toml") - package = _read(repo / "package.json") + package = read("package.json") if package is not None: try: scripts = json.loads(package).get("scripts") or {} @@ -61,14 +76,14 @@ def add(command: str, source: str) -> None: test = scripts.get("test") if isinstance(scripts, dict) else None if isinstance(test, str) and test.strip() and _NPM_PLACEHOLDER not in test: add("npm test", "package.json") - for name in ("GNUmakefile", "makefile", "Makefile"): - text = _read(repo / name) + for name in ("GNUmakefile", "makefile", "Makefile"): # GNU make's order + text = read(name) if text is not None and _MAKE_TEST_TARGET.search(text): add("make test", name) break - if _read(repo / "Cargo.toml") is not None: + if read("Cargo.toml") is not None: add("cargo test", "Cargo.toml") - if _read(repo / "go.mod") is not None: + if read("go.mod") is not None: add("go test ./...", "go.mod") return found diff --git a/tests/test_start.py b/tests/test_start.py index 7584648..78dbe2a 100644 --- a/tests/test_start.py +++ b/tests/test_start.py @@ -236,6 +236,59 @@ def test_discovery_reads_known_files_and_never_runs_them(tmp_path): assert not canary.exists() +@pytest.mark.parametrize("name", ["Makefile", "makefile", "GNUmakefile"]) +def test_discovery_names_the_makefile_as_it_is_stored(tmp_path, monkeypatch, name): + """#249: on a case-insensitive filesystem opening "makefile" finds + "Makefile". Opening is made case-insensitive here so the check holds on + every filesystem: the source must still be the name on disk.""" + import syncmill.start as start_module + + (tmp_path / name).write_text("test:\n\ttrue\n") + real = start_module._read + + def case_insensitive(path): + for entry in path.parent.iterdir(): + if entry.name.lower() == path.name.lower(): + return real(entry) + return None + + monkeypatch.setattr(start_module, "_read", case_insensitive) + assert [(s.command, s.source) for s in discover_gate_commands(tmp_path)] == [ + ("make test", name) + ] + + +@pytest.mark.parametrize( + "stored, body", + [ + ("PYPROJECT.TOML", '[project]\ndependencies = ["pytest"]\n'), + ("Package.json", json.dumps({"scripts": {"test": "jest"}})), + ("MAKEFILE", "test:\n\ttrue\n"), + ("cargo.toml", "[package]\n"), + ("GO.MOD", "module x\n"), + ], +) +def test_discovery_ignores_a_file_whose_name_differs_only_in_case( + tmp_path, monkeypatch, stored, body +): + """A well-known file stored under another capitalization is not that file: + opening by name would find it on a case-insensitive filesystem only, and + the suggestion would name a file that is not on disk (#249).""" + import syncmill.start as start_module + + (tmp_path / stored).write_text(body) + real = start_module._read + + def case_insensitive(path): + for entry in path.parent.iterdir(): + if entry.name.lower() == path.name.lower(): + return real(entry) + return None + + monkeypatch.setattr(start_module, "_read", case_insensitive) + assert discover_gate_commands(tmp_path) == [] + + def test_discovery_skips_placeholders_and_symlinks(tmp_path): (tmp_path / "package.json").write_text(json.dumps( {"scripts": {"test": 'echo "Error: no test specified" && exit 1'}} diff --git a/tests/test_tui.py b/tests/test_tui.py index 4f4e046..4ba921d 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -3814,11 +3814,27 @@ async def scenario(): room.store.close() -def test_details_and_compare_share_wording_with_the_cli_json(tmp_path): +@pytest.mark.parametrize( + "reason", + [ + "git rev-parse --verify HEAD failed: not a git repository", + # git's error when it stops at a filesystem boundary is two lines (#251) + "git rev-parse --verify HEAD failed:\nfatal: not a git repository\n" + "Stopping at filesystem boundary", + ], +) +def test_details_and_compare_share_wording_with_the_cli_json(tmp_path, monkeypatch, reason): + from syncmill.artifacts import printable_name from syncmill.room_compare import comparison_to_dict, gate_label, render_round_table from syncmill.tui import build_room_evidence from syncmill.tui_collaboration import render_candidate_details + def unavailable(repo_root): + raise RuntimeError(reason) + + # The source check's failure text is git's own stderr; pin it rather than + # depend on how the runner's temp directory is mounted. + monkeypatch.setattr("syncmill.tui.repository_fingerprint", unavailable) room, *_ = _seeded_room(tmp_path) try: comparison, _, _ = build_room_evidence( @@ -3837,8 +3853,10 @@ def test_details_and_compare_share_wording_with_the_cli_json(tmp_path): assert f"Gate: {label}" in details assert data[row.candidate_id]["gate"]["verdict"] == row.gate.verdict assert data[row.candidate_id]["apply_blockers"] == list(row.apply_blockers) + assert f"source check unavailable: {reason}" in row.apply_blockers for blocker in row.apply_blockers: - assert blocker in details + # Details shows each blocker on one escaped line; the JSON keeps it raw. + assert f" - {printable_name(blocker)}" in details.splitlines() assert row.gate.verdict.replace("_", " ").split()[0] in table