diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..a1ea335 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,13 @@ +# Chemenu - .gitattributes +# +# Every text file is stored and checked out with LF, whatever `core.autocrlf` says. +# Without this, a Windows checkout with `core.autocrlf=true` gives the sh launcher +# tools/wikitool CRLF line endings, and Git Bash then fails with `env: 'bash\r'`. The +# tools also compare file bytes (the published skill copies, the sha256 per file in +# `.wikitool-release.json`), and those comparisons only agree when the line endings do. +* text=auto eol=lf + +# Sources are kept byte for byte as they arrived: raw/CONTRACT.md makes them immutable, +# and normalizing a CRLF source on `git add` would change it. +/raw/** -text +/incoming/** -text diff --git a/CHANGES.md b/CHANGES.md index c3992f6..45e3be0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -59,7 +59,7 @@ concern - readable here, never shipped as something to parse. --- -## 8.0.0-beta.18 - 2026-10-01 - trace-hook.ps1: Copilot hooks no longer open Windows' choose-an-app dialog +## 8.0.0-beta.19 - 2026-10-01 - Windows-Portabilität: Pfadtrenner, Zeilenenden, Encoding und Locks **Author:** Torben Nehmer @@ -92,6 +92,7 @@ concern - readable here, never shipped as something to parse. - PowerShell 7 preflight and launcher: tools/preflight.ps1, tools/wikitool.ps1, doctor checks for execution policy and Mark of the Web - Preflight as a release asset: download, verify and unpack the stack, then run the tree preflight - trace-hook.ps1: Copilot hooks no longer open Windows' choose-an-app dialog +- Windows-Portabilität: Pfadtrenner, Zeilenenden, Encoding und Locks **Low impact** - version bump no longer points at version release in its output @@ -129,6 +130,57 @@ concern - readable here, never shipped as something to parse. - preflight.ps1: the asset-mode error helper is Exit-Asset, so PSScriptAnalyzer passes +### Windows-Portabilität: Pfadtrenner, Zeilenenden, Encoding und Locks + +The Python package assumed POSIX in several places that nothing on Linux would ever reveal +(Gitea #152, part of #140). In the run analysed in #140, `instructions verify` failed on all 23 +instructions on Windows. This changeset makes the package behave the same on Windows, and holds +it there with guards that run in the ordinary Linux CI. + +- **Path separators.** Every `str(.relative_to(...))` is now `.as_posix()`, as are the + error messages that printed a relative path. On Windows these strings came out as + `kb\x.md`. They were then compared with POSIX keys or stored. The type-spec + self-reference check in `type_resolver.py` is one of them, and it failed every validation. +- **ripgrep paths.** `rg --json` writes `\` on Windows, and `--path-separator /` does not + reach its JSON output (measured on the target system, T3). `search/ripgrep.py` converts the + separator where it parses a match, and does so only when `os.sep` is `\`. Nothing is lost: + no Windows path component can contain `\`, and since #155 no page title can either. +- **Line endings.** A new `.gitattributes` (`* text=auto eol=lf`) keeps every text file LF in + a checkout with `core.autocrlf=true`. Without it the sh launcher gets CRLF and Git Bash fails + with `env: 'bash\r'`. `raw/` and `incoming/` are `-text`, so a source is stored byte for + byte as it arrived. `dist export` ships the file. The index was LF throughout already, so + renormalizing changes nothing. Every text write now passes `newline="\n"`, so the byte + comparison of published skill copies and the per-file sha256 in `dist upgrade` agree on + Windows too. +- **Decoding.** Every `subprocess` call with `text=True` names `encoding="utf-8"`. Without it, + Windows decodes `git` and `rg` output in the locale's code page (cp1252). +- **wikitool's own output.** `tools/run_wikitool.py`, the file both launchers run, sets + stdout and stderr to UTF-8. Python on Windows writes into a pipe in cp1252. Measured on + the target system, PowerShell decodes the output of a child process with + `[Console]::OutputEncoding`. Under Copilot that is UTF-8, and Git Bash passes bytes through + unchanged. Before this change `doctor` showed `Fu�noten` under both harnesses. A console is + unaffected either way. +- **Hook payloads.** `trace_ingest.py` reads its stdin as UTF-8 bytes. A locale-decoded read + failed outright on Windows when a payload contained a character such as `Ł`, whose UTF-8 + form holds a byte that cp1252 leaves undefined. +- **Locks.** The budget counter and the telemetry writer locked with `fcntl` and silently + skipped the lock where it does not exist. Parallel calls on Windows could then lose a + budget increment. The new module `chemenu/filelock.py` is the only one allowed to import + `fcntl` or `msvcrt`. On Windows it locks one byte far past the file's data with + `msvcrt.locking`, because a Windows lock is mandatory and a lock on the data would block + `trace_ingest.py` from reading a trace. It waits for a contended lock the way `flock` does. + +New tests: `tests/test_portability.py` reads the source of `tools/chemenu` and the scripts +beside it. It fails on a stringified `relative_to`, on a text open/read/write without +`encoding=`, on a text write without `newline=`, on `text=True` without `encoding=`, and on an +`fcntl`/`msvcrt` import outside `filelock.py`. Each detector also gets the defect it exists +for, so a guard that matches nothing cannot pass. The `fcntl` fallback is tested with the +import hidden and a fake `msvcrt`. A real Windows lock is not exercised, because CI runs only +Linux. The UTF-8 output and stdin are tested under `PYTHONIOENCODING=cp1252`, which simulates +the Windows pipe. Further tests feed the T3 JSON line through `search/ripgrep.py` with a +simulated Windows separator and check `.gitattributes` with `git check-attr`. Whether `doctor` +shows `Fußnoten` under Copilot and Claude Code is checked by hand on the target machine. + ### trace-hook.ps1: Copilot hooks no longer open Windows' choose-an-app dialog On the Windows target machine, Copilot opened Windows' "choose an app" dialog for `trace-hook` diff --git a/VERSION b/VERSION index db04e2d..dca041a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.0.0-beta.18 +8.0.0-beta.19 diff --git a/raw/CONTRACT.md b/raw/CONTRACT.md index 4f55afb..37ed2db 100644 --- a/raw/CONTRACT.md +++ b/raw/CONTRACT.md @@ -211,7 +211,9 @@ value nobody ever thought about. ## Rules - **Immutable.** Never edit, reformat, summarize, or "clean up" a file after it lands here. - Corrections belong in the `kb/` page that covers it, not in the source. + Corrections belong in the `kb/` page that covers it, not in the source. That includes line + endings: `.gitattributes` marks `raw/` and `incoming/` as `-text`, so git stores a source with + the bytes it arrived with, where every other text file is normalized to LF. - **Replaceable as a whole, never in part.** A source that gets a later edition is replaced wholesale by `raw accept --replaces`, in one commit together with the update of every `kb/` page compiled from it. Whether a new file is a later edition of an existing source or a diff --git a/tools/README.md b/tools/README.md index 46c0aa6..ce0473f 100644 --- a/tools/README.md +++ b/tools/README.md @@ -51,6 +51,15 @@ the recorded path - or, with no file at all (the test suite, a bare but names a path that has gone raises `ToolPathError`, which the CLI turns into an `ERROR` line pointing at the preflight rather than a traceback. +The package runs natively on Windows as well, which CI never does. So the rules that keep +it portable are held by reading the source, in `tests/test_portability.py`: a path that +becomes a string goes through `.as_posix()`, a text file is opened with `encoding=` and +written with `newline="\n"`, a `subprocess` call with `text=True` names `encoding="utf-8"`, +and only `filelock.py` imports `fcntl` or `msvcrt`. An `rg` path comes back with `\` on +Windows even with `--path-separator /`, so `search/ripgrep.py` converts it where it parses +the JSON. `.gitattributes` keeps every text file LF in a checkout, `raw/` and `incoming/` +excepted. + `jsonschema` and `PyYAML` are hard dependencies, not optional extras: schema validation is the tool's whole safety net, so `cli.py` fails loudly with the fix rather than degrading silently. @@ -61,7 +70,7 @@ fix rather than degrading silently. tools/ wikitool entry point (POSIX sh): stops with exit 42 until the preflight has passed wikitool.ps1 the same entry point for PowerShell 7, which resolves `tools/wikitool` to this file first - run_wikitool.py what the launcher runs with the venv's Python - puts chemenu on sys.path without PYTHONPATH + run_wikitool.py what the launcher runs with the venv's Python - puts chemenu on sys.path without PYTHONPATH, sets stdout/stderr to UTF-8 preflight.sh checks prerequisites.txt, records .wikitool-tools.json, creates .venv (POSIX sh); as the release asset, downloads and unpacks the stack first preflight.ps1 the same for PowerShell 7; also checks the execution policy and the Mark of the Web prerequisites.txt what the machine needs, one `|`-separated line per tool - read by the preflight and `doctor` @@ -74,6 +83,7 @@ tools/ api.py the in-process entry point - point Chemenu at a corpus and read it errors.py ChemenuError / ValidationError / BackendError toolpaths.py where git and rg are started from: .wikitool-tools.json, bare name only without the file + filelock.py an exclusive lock on an open file, flock on POSIX and msvcrt on Windows - the only module that imports either prerequisites.py prerequisites.txt read from Python, plus the platform and long-path questions `doctor` asks corpus_cache.py one parsed corpus per commit, never cached while the tree is dirty kb_scan.py page iteration/loading over kb/ diff --git a/tools/chemenu/commands/_util.py b/tools/chemenu/commands/_util.py index da7862f..d557e0a 100644 --- a/tools/chemenu/commands/_util.py +++ b/tools/chemenu/commands/_util.py @@ -214,9 +214,9 @@ def rel_path(path: Path) -> str: from chemenu import config try: - return str(Path(path).relative_to(config.ROOT)) + return Path(path).relative_to(config.ROOT).as_posix() except ValueError: - return str(path) + return Path(path).as_posix() def check_title(name: str) -> None: diff --git a/tools/chemenu/commands/dist_cmd.py b/tools/chemenu/commands/dist_cmd.py index 44c428e..e382bb1 100644 --- a/tools/chemenu/commands/dist_cmd.py +++ b/tools/chemenu/commands/dist_cmd.py @@ -101,7 +101,7 @@ DIST_TEMPLATES_DIR = Path(__file__).resolve().parent.parent / "dist_templates" # that silence is the correct behaviour here, not a gap. ROOT_FILES = ( "AGENTS.md", "CLAUDE.md", "README.md", "EVALS.md", "INSTALL.md", "INSTALL-MCP.md", - ".gitignore", "VERSION", + ".gitignore", ".gitattributes", "VERSION", *config.LICENSE_FILES, *config.PERSONALIZATION_TEMPLATES, config.ENVIRONMENT_TEMPLATE, @@ -568,7 +568,7 @@ def _write_plan(target: Path, plan: dict[str, PlannedFile]) -> None: if isinstance(planned.content, bytes): dest.write_bytes(planned.content) else: - dest.write_text(planned.content, encoding="utf-8") + dest.write_text(planned.content, encoding="utf-8", newline="\n") if planned.executable: dest.chmod(dest.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) @@ -957,6 +957,7 @@ def _git_working_tree_status() -> Optional[str]: [toolpaths.git(), "-C", str(config.ROOT), "status", "--porcelain"], capture_output=True, text=True, + encoding="utf-8", ) return result.stdout if result.returncode == 0 else None diff --git a/tools/chemenu/commands/docs_verify.py b/tools/chemenu/commands/docs_verify.py index 93eb5ff..afc13c1 100644 --- a/tools/chemenu/commands/docs_verify.py +++ b/tools/chemenu/commands/docs_verify.py @@ -506,7 +506,7 @@ def check_collection_contracts() -> list[str]: ) for stray in kb_collections.stray_collection_contracts(): - relative = stray.relative_to(config.ROOT) + relative = stray.relative_to(config.ROOT).as_posix() if kb_collections.kb_collection_of(stray.parent) is not None: issues.append( f"{relative} is nested inside a collection - a subdirectory is an area and " @@ -607,7 +607,7 @@ def check_legacy_type_blocks() -> list[str]: guarded = [ *TYPE_GUARD_DOCS, *( - str((path / "COLLECTION.md").relative_to(config.ROOT)) + (path / "COLLECTION.md").relative_to(config.ROOT).as_posix() for path in kb_collections.iter_kb_collections() ), ] @@ -842,7 +842,7 @@ def _git(args: list[str], stdin: Optional[str] = None) -> Optional[subprocess.Co are unknowable rather than wrong.""" try: return subprocess.run( - [toolpaths.git(), *args], cwd=config.ROOT, capture_output=True, text=True, input=stdin + [toolpaths.git(), *args], cwd=config.ROOT, capture_output=True, text=True, encoding="utf-8", input=stdin ) except OSError: return None @@ -1227,7 +1227,7 @@ def toc_command( for path, after in changed: typer.echo(rel_path(path)) if apply: - path.write_text(after, encoding="utf-8") + path.write_text(after, encoding="utf-8", newline="\n") if apply: success(f"Refreshed the table of contents on {len(changed)} file(s).") @@ -1293,5 +1293,5 @@ def contract_command( typer.echo("Re-run with --apply to write.") return - CLI_README.write_text(after, encoding="utf-8") + CLI_README.write_text(after, encoding="utf-8", newline="\n") success(f"Regenerated the command region in {rel_path(CLI_README)}.") diff --git a/tools/chemenu/commands/doctor.py b/tools/chemenu/commands/doctor.py index 17d46e8..90130af 100644 --- a/tools/chemenu/commands/doctor.py +++ b/tools/chemenu/commands/doctor.py @@ -41,7 +41,7 @@ class Check: def _git(args: list[str]) -> Optional[subprocess.CompletedProcess]: try: return subprocess.run( - [toolpaths.git(), *args], cwd=config.ROOT, capture_output=True, text=True, timeout=5 + [toolpaths.git(), *args], cwd=config.ROOT, capture_output=True, text=True, encoding="utf-8", timeout=5 ) except (OSError, subprocess.SubprocessError, toolpaths.ToolPathError): # A broken tool-paths file is reported once, by `check_tool_paths` - diff --git a/tools/chemenu/commands/eval_cmd.py b/tools/chemenu/commands/eval_cmd.py index fcbb033..5346eb2 100644 --- a/tools/chemenu/commands/eval_cmd.py +++ b/tools/chemenu/commands/eval_cmd.py @@ -151,13 +151,17 @@ def score_command( directory = EVALS_DIR / date.today().isoformat() directory.mkdir(parents=True, exist_ok=True) stem = target.replace("/", "__") - (directory / f"{stem}.json").write_text(json.dumps(card, indent=2), encoding="utf-8") + (directory / f"{stem}.json").write_text( + json.dumps(card, indent=2), encoding="utf-8", newline="\n" + ) (directory / f"{stem}.md").write_text( - scorecard.render_markdown(card) + "\n", encoding="utf-8" + scorecard.render_markdown(card) + "\n", encoding="utf-8", newline="\n" ) success(f"Wrote {rel_path(directory / stem)}.json/.md") if markdown_out: - markdown_out.write_text(scorecard.render_markdown(card) + "\n", encoding="utf-8") + markdown_out.write_text( + scorecard.render_markdown(card) + "\n", encoding="utf-8", newline="\n" + ) success(f"Wrote {rel_path(markdown_out)}") if json_out: typer.echo(json.dumps(card, indent=2)) diff --git a/tools/chemenu/commands/git_publish.py b/tools/chemenu/commands/git_publish.py index a3ab87b..7bc6fc9 100644 --- a/tools/chemenu/commands/git_publish.py +++ b/tools/chemenu/commands/git_publish.py @@ -84,7 +84,7 @@ def _run(args: list[str]) -> subprocess.CompletedProcess: """Run a `git ...` argument list, starting git from its recorded path.""" if args and args[0] == "git": args = [toolpaths.git(), *args[1:]] - return subprocess.run(args, cwd=config.ROOT, capture_output=True, text=True) + return subprocess.run(args, cwd=config.ROOT, capture_output=True, text=True, encoding="utf-8") # --- Publish-Remote Gate ----------------------------------------------------- @@ -269,7 +269,7 @@ def collect_changes(paths: list[str]) -> list[FileChange]: def run(args: list[str]) -> str: result = subprocess.run( - [toolpaths.git(), *args], cwd=config.ROOT, capture_output=True, text=True, env=env, + [toolpaths.git(), *args], cwd=config.ROOT, capture_output=True, text=True, encoding="utf-8", env=env, ) if result.returncode != 0: fail(f"git {args[0]} failed:\n{result.stderr}") diff --git a/tools/chemenu/commands/index_build.py b/tools/chemenu/commands/index_build.py index e30e52e..41affd7 100644 --- a/tools/chemenu/commands/index_build.py +++ b/tools/chemenu/commands/index_build.py @@ -290,7 +290,7 @@ def index_rebuild( for path, content in plan.items(): path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(content, encoding="utf-8") + path.write_text(content, encoding="utf-8", newline="\n") for path in stale: path.unlink() diff --git a/tools/chemenu/commands/lint.py b/tools/chemenu/commands/lint.py index e93a192..953bcee 100644 --- a/tools/chemenu/commands/lint.py +++ b/tools/chemenu/commands/lint.py @@ -142,7 +142,7 @@ def lint_command( "---\n\n" ) target.parent.mkdir(parents=True, exist_ok=True) - target.write_text(frontmatter + render_markdown(report) + "\n", encoding="utf-8") + target.write_text(frontmatter + render_markdown(report) + "\n", encoding="utf-8", newline="\n") success(f"Full report written to {rel_path(target)}") if fail_on_error and has_hard_errors(report): diff --git a/tools/chemenu/commands/log_append.py b/tools/chemenu/commands/log_append.py index 3904e1f..7b79644 100644 --- a/tools/chemenu/commands/log_append.py +++ b/tools/chemenu/commands/log_append.py @@ -109,7 +109,7 @@ def log_append( except (OSError, UnicodeDecodeError) as exc: fail(f"Cannot read --body-file {body_file}: {exc}") entry = format_log_entry(op, title, text) - with config.LOG_FILE.open("a", encoding="utf-8") as f: + with config.LOG_FILE.open("a", encoding="utf-8", newline="\n") as f: f.write("\n" + entry) success(f"Appended log entry to {rel_path(config.LOG_FILE)}") diff --git a/tools/chemenu/commands/migrate_cmd.py b/tools/chemenu/commands/migrate_cmd.py index 0551064..70ff289 100644 --- a/tools/chemenu/commands/migrate_cmd.py +++ b/tools/chemenu/commands/migrate_cmd.py @@ -504,6 +504,7 @@ def _git_show(rev: str, relative: str) -> Optional[str]: cwd=config.ROOT, capture_output=True, text=True, + encoding="utf-8", ) return result.stdout if result.returncode == 0 else None @@ -514,6 +515,7 @@ def _paths_at(rev: str) -> Optional[list[str]]: cwd=config.ROOT, capture_output=True, text=True, + encoding="utf-8", ) if result.returncode != 0: return None @@ -556,7 +558,7 @@ def _shapes_at_revision(rev: str, wanted: set[str]) -> dict[str, corpus_diff.Pag # historical blob is materialised under its real filename - the stem # is the page title, which PageShape compares. scratch = Path(tmp) / Path(relative).name - scratch.write_text(text, encoding="utf-8") + scratch.write_text(text, encoding="utf-8", newline="\n") try: frontmatter, body = read_page(scratch) except Exception: # noqa: BLE001 - an unparseable historical page is not this tool's error diff --git a/tools/chemenu/commands/provenance_cmd.py b/tools/chemenu/commands/provenance_cmd.py index 6fca72e..569361c 100644 --- a/tools/chemenu/commands/provenance_cmd.py +++ b/tools/chemenu/commands/provenance_cmd.py @@ -36,15 +36,15 @@ def _normalize_raw_path(raw: str) -> str: candidate = Path(raw) if candidate.is_absolute(): try: - return str(candidate.relative_to(config.ROOT)) + return candidate.relative_to(config.ROOT).as_posix() except ValueError: - return str(candidate) + return candidate.as_posix() if candidate.exists(): - return str(candidate) + return candidate.as_posix() if (config.ROOT / candidate).exists(): - return str(candidate) + return candidate.as_posix() if (config.RAW_DIR / candidate).exists(): - return str((Path("raw") / candidate)) + return (Path("raw") / candidate).as_posix() return raw @@ -188,7 +188,7 @@ def trace( def build_provenance_index(kb_dir: Path, raw_dir: Path) -> str: pages = load_kb_pages(kb_dir) by_raw = source_pages_by_raw_file(pages) - all_raw = sorted(str(p.relative_to(config.ROOT)) for p in config.iter_raw_files(raw_dir)) + all_raw = sorted(p.relative_to(config.ROOT).as_posix() for p in config.iter_raw_files(raw_dir)) uncovered = uncovered_raw_files(raw_dir, pages) lines: list[str] = [] @@ -274,5 +274,5 @@ def rebuild_index( # written; see the same note in index_build.py. typer.echo(content, nl=False) return - provenance_file.write_text(content, encoding="utf-8") + provenance_file.write_text(content, encoding="utf-8", newline="\n") success(f"Rebuilt {rel_path(provenance_file)}") diff --git a/tools/chemenu/commands/raw_cmd.py b/tools/chemenu/commands/raw_cmd.py index d9c105b..4276acb 100644 --- a/tools/chemenu/commands/raw_cmd.py +++ b/tools/chemenu/commands/raw_cmd.py @@ -122,10 +122,10 @@ def _validate_under_incoming(path: Path, incoming: Path) -> None: "from there. See raw/CONTRACT.md." ) if len(rel.parts) < 1: - fail(f"incoming/{rel} names no file.") + fail(f"incoming/{rel.as_posix()} names no file.") if len(rel.parts) > 2: fail( - f"incoming/{rel} is nested more than one level below incoming/ - place it " + f"incoming/{rel.as_posix()} is nested more than one level below incoming/ - place it " "directly in incoming/, or in at most one subdirectory of it (the " "subdirectory itself is ignored, see raw/CONTRACT.md)." ) diff --git a/tools/chemenu/commands/run_budget.py b/tools/chemenu/commands/run_budget.py index 068e23e..c355fdb 100644 --- a/tools/chemenu/commands/run_budget.py +++ b/tools/chemenu/commands/run_budget.py @@ -27,7 +27,7 @@ from pathlib import Path import typer -from chemenu import cli_contract, config +from chemenu import cli_contract, config, filelock from chemenu.commands._util import fail, success from chemenu.session import session_id as _shared_session_id from chemenu.session import session_id_source as _shared_session_id_source @@ -153,7 +153,7 @@ def _load_state() -> dict: if not STATE_FILE.exists(): return {} try: - return json.loads(STATE_FILE.read_text()) + return json.loads(STATE_FILE.read_text(encoding="utf-8")) except (json.JSONDecodeError, OSError): return {} @@ -177,7 +177,7 @@ def _save_state(state: dict) -> None: STATE_DIR.mkdir(parents=True, exist_ok=True) payload = json.dumps(prune_state(state, time.time()), indent=2) tmp_file = STATE_FILE.with_suffix(STATE_FILE.suffix + ".tmp") - tmp_file.write_text(payload) + tmp_file.write_text(payload, encoding="utf-8", newline="\n") os.replace(tmp_file, STATE_FILE) @@ -187,20 +187,14 @@ def _state_lock(): save cycle. Without this, two `wikitool` calls racing in the same session (e.g. two parallel subagents) can both load count=N, both compute N+1, and both save - losing an increment and letting the session run past the gate - it exists to enforce. POSIX-only (fcntl); best-effort no-op if unavailable, - since the loop-breaker's identical-call check still degrades gracefully.""" + it exists to enforce. `chemenu.filelock` makes it hold on Windows too. + + The lock file is opened for appending, not with `w`: truncating a file + whose lock another process holds is what Windows may refuse.""" STATE_DIR.mkdir(parents=True, exist_ok=True) - try: - import fcntl - except ImportError: # pragma: no cover - non-POSIX platform - yield - return - with open(LOCK_FILE, "w") as lock_fh: - fcntl.flock(lock_fh, fcntl.LOCK_EX) - try: + with open(LOCK_FILE, "a", encoding="utf-8", newline="\n") as lock_fh: + with filelock.exclusive(lock_fh): yield - finally: - fcntl.flock(lock_fh, fcntl.LOCK_UN) def loop_breaker_message(call_signature: str, loop_window: int) -> str: diff --git a/tools/chemenu/commands/upstream_cmd.py b/tools/chemenu/commands/upstream_cmd.py index 523bd7b..5654db5 100644 --- a/tools/chemenu/commands/upstream_cmd.py +++ b/tools/chemenu/commands/upstream_cmd.py @@ -37,7 +37,7 @@ app = typer.Typer(help="Take a stack update from a public upstream, machinery on def _run(args: list[str]): import subprocess - return subprocess.run(args, cwd=config.ROOT, capture_output=True, text=True) + return subprocess.run(args, cwd=config.ROOT, capture_output=True, text=True, encoding="utf-8") def _rev_parse(rev: str) -> Optional[str]: diff --git a/tools/chemenu/commands/version_cmd.py b/tools/chemenu/commands/version_cmd.py index 0ed1198..37113ff 100644 --- a/tools/chemenu/commands/version_cmd.py +++ b/tools/chemenu/commands/version_cmd.py @@ -729,7 +729,7 @@ def bump_command( migration_required=migration_required, impact=chosen_impact, ), - encoding="utf-8", + encoding="utf-8", newline="\n", ) impact_note = "" if impact is not None else f" (impact not given - assumed {chosen_impact})" success( @@ -873,7 +873,7 @@ def release_command( version_mod.write_version(new_version) changes.write_text( version_mod.release_entry(text, today_iso(), title.strip() if title else None), - encoding="utf-8", + encoding="utf-8", newline="\n", ) success( f"{current} -> {new_version} (release). Wrote {version_mod.VERSION_FILENAME} and fixed the " @@ -1005,5 +1005,5 @@ def regrade_command( fail(str(exc)) return - changes.write_text(new_text, encoding="utf-8") + changes.write_text(new_text, encoding="utf-8", newline="\n") success(f"Regraded {len(indices)} bump title(s) to {impact} impact.") diff --git a/tools/chemenu/commands/work_cmd.py b/tools/chemenu/commands/work_cmd.py index 6c9fc0f..9392340 100644 --- a/tools/chemenu/commands/work_cmd.py +++ b/tools/chemenu/commands/work_cmd.py @@ -256,8 +256,12 @@ def new_command( return target.mkdir(parents=True) - (target / "README.md").write_text(readme_template(run_key, input_path), encoding="utf-8") - (target / "plan.md").write_text(plan_template(run_key, input_path), encoding="utf-8") + (target / "README.md").write_text( + readme_template(run_key, input_path), encoding="utf-8", newline="\n" + ) + (target / "plan.md").write_text( + plan_template(run_key, input_path), encoding="utf-8", newline="\n" + ) typer.echo(f"Run key: {run_key}") typer.echo(f"Workshop: {rel_path(target)}/") diff --git a/tools/chemenu/config.py b/tools/chemenu/config.py index 3460afe..69312c9 100644 --- a/tools/chemenu/config.py +++ b/tools/chemenu/config.py @@ -307,6 +307,7 @@ def default_author() -> str | None: cwd=_root(), capture_output=True, text=True, + encoding="utf-8", timeout=5, check=False, ) diff --git a/tools/chemenu/corpus_cache.py b/tools/chemenu/corpus_cache.py index fc93127..f75c44c 100644 --- a/tools/chemenu/corpus_cache.py +++ b/tools/chemenu/corpus_cache.py @@ -66,6 +66,7 @@ def _git(args: list[str], root: Optional[Path] = None): cwd=root or config.ROOT, capture_output=True, text=True, + encoding="utf-8", timeout=10, check=False, ) diff --git a/tools/chemenu/filelock.py b/tools/chemenu/filelock.py new file mode 100644 index 0000000..e8aeea7 --- /dev/null +++ b/tools/chemenu/filelock.py @@ -0,0 +1,77 @@ +"""An exclusive lock on an open file that holds on POSIX and on Windows alike. + +Two writers share files across processes: the budget state (`commands/run_budget.py`), +where a lost race loses an increment and lets a session run past its gate, and the +telemetry trace (`telemetry/writer.py`), where it lets one line land inside another. +Both used `fcntl.flock` and skipped the lock where `fcntl` does not exist - which is +every native Windows install, silently. This module is the one place that knows the +platform difference, and the only module allowed to import `fcntl` +(`tests/test_portability_guards.py` holds that). + +Windows has no `flock`. `msvcrt.locking` locks a byte range instead, and the lock is +mandatory: a locked byte cannot be read or written by any other process. So the lock +does not go on the file's data, which a reader such as `trace_ingest.py` would then fail +on, but on one byte far past it. Windows allows a lock beyond the end of a file, and +there it excludes the other lockers and nobody else. +""" +from __future__ import annotations + +import os +from contextlib import contextmanager +from typing import IO, Iterator + +# Where the Windows lock sits: past any size a trace or budget file reaches, and +# still inside a signed 32-bit offset. +WINDOWS_LOCK_OFFSET = 2**31 - 2 + + +@contextmanager +def exclusive(handle: IO) -> Iterator[None]: + """Hold an exclusive lock on `handle` for the duration of the block. + + Blocks until the lock is free, on both platforms. The handle's file position is + left where it was, so an append handle keeps appending. + """ + try: + import fcntl + except ImportError: + fcntl = None + + if fcntl is not None: + fcntl.flock(handle.fileno(), fcntl.LOCK_EX) + try: + yield + finally: + fcntl.flock(handle.fileno(), fcntl.LOCK_UN) + return + + import msvcrt + + handle.flush() + _windows_lock(handle.fileno(), msvcrt, msvcrt.LK_LOCK) + try: + yield + finally: + handle.flush() + _windows_lock(handle.fileno(), msvcrt, msvcrt.LK_UNLCK) + + +def _windows_lock(fd: int, msvcrt, mode: int) -> None: + """Lock or unlock the one byte at WINDOWS_LOCK_OFFSET. + + `msvcrt.locking` acts on the current position, so this seeks there and back. + `LK_LOCK` gives up after ten one-second retries with an OSError; it is retried + here until it succeeds, which is what `flock` does on POSIX. + """ + position = os.lseek(fd, 0, os.SEEK_CUR) + os.lseek(fd, WINDOWS_LOCK_OFFSET, os.SEEK_SET) + try: + while True: + try: + msvcrt.locking(fd, mode, 1) + return + except OSError: + if mode != msvcrt.LK_LOCK: + raise + finally: + os.lseek(fd, position, os.SEEK_SET) diff --git a/tools/chemenu/frontmatter_io.py b/tools/chemenu/frontmatter_io.py index 7f1b0b0..5fc8c6e 100644 --- a/tools/chemenu/frontmatter_io.py +++ b/tools/chemenu/frontmatter_io.py @@ -320,4 +320,4 @@ def write_page(path: Path, frontmatter: dict[str, Any], body: str) -> None: content = f"---\n{fm_text}\n---{body}" if not content.endswith("\n"): content += "\n" - path.write_text(content, encoding="utf-8") + path.write_text(content, encoding="utf-8", newline="\n") diff --git a/tools/chemenu/kb_scan.py b/tools/chemenu/kb_scan.py index 01a8315..8baa983 100644 --- a/tools/chemenu/kb_scan.py +++ b/tools/chemenu/kb_scan.py @@ -81,9 +81,9 @@ def find_duplicate_title_paths(kb_dir: Path, root: Path) -> list[dict]: by_stem: dict[str, list[str]] = {} for path in iter_kb_pages(kb_dir): try: - rel = str(path.relative_to(root)) + rel = path.relative_to(root).as_posix() except ValueError: - rel = str(path.relative_to(kb_dir.parent)) + rel = path.relative_to(kb_dir.parent).as_posix() by_stem.setdefault(path.stem, []).append(rel) return [ {"stem": stem, "paths": sorted(paths)} diff --git a/tools/chemenu/kb_state.py b/tools/chemenu/kb_state.py index dd17c4b..aa74380 100644 --- a/tools/chemenu/kb_state.py +++ b/tools/chemenu/kb_state.py @@ -67,9 +67,9 @@ class Migration: @property def relative_path(self) -> str: try: - return str(self.path.relative_to(config.ROOT)) + return self.path.relative_to(config.ROOT).as_posix() except ValueError: - return str(self.path) + return self.path.as_posix() def read_kb_version() -> Optional[Version]: @@ -122,7 +122,7 @@ def render_kb_state(version: Version, applied: list[dict]) -> str: def write_kb_state(version: Version, applied: list[dict]) -> None: - kb_state_file().write_text(render_kb_state(version, applied), encoding="utf-8") + kb_state_file().write_text(render_kb_state(version, applied), encoding="utf-8", newline="\n") def migrations_dir() -> Path: diff --git a/tools/chemenu/lint_core.py b/tools/chemenu/lint_core.py index 18d361e..4bb54b4 100644 --- a/tools/chemenu/lint_core.py +++ b/tools/chemenu/lint_core.py @@ -89,9 +89,9 @@ def _display(path: Path) -> str: otherwise (a fixture tree in a test, or any tree `config.ROOT` does not contain).""" try: - return str(path.relative_to(config.ROOT)) + return path.relative_to(config.ROOT).as_posix() except ValueError: - return str(path) + return path.as_posix() def find_misplaced(pages: dict[str, Page]) -> list[tuple[str, Page, Path]]: @@ -296,9 +296,9 @@ def long_paths(kb_dir: Path, raw_dir: Path) -> list[dict]: def _repo_relative(path: Path, kb_dir: Path) -> str: try: - return str(path.relative_to(config.ROOT)) + return path.relative_to(config.ROOT).as_posix() except ValueError: - return str(path.relative_to(kb_dir.parent)) + return path.relative_to(kb_dir.parent).as_posix() def run_lint(kb_dir: Path) -> dict: diff --git a/tools/chemenu/prerequisites.py b/tools/chemenu/prerequisites.py index d5c2cee..bc99cc3 100644 --- a/tools/chemenu/prerequisites.py +++ b/tools/chemenu/prerequisites.py @@ -181,7 +181,7 @@ def execution_policy() -> Optional[Policy]: done = subprocess.run( [pwsh, "-NoProfile", "-NonInteractive", "-Command", "Get-ExecutionPolicy -List | ForEach-Object { '{0}={1}' -f $_.Scope, $_.ExecutionPolicy }"], - capture_output=True, text=True, timeout=30, + capture_output=True, text=True, encoding="utf-8", timeout=30, ) except (toolpaths.ToolPathError, OSError, subprocess.SubprocessError): # pragma: no cover return None diff --git a/tools/chemenu/provenance.py b/tools/chemenu/provenance.py index 0353eb2..73cbce7 100644 --- a/tools/chemenu/provenance.py +++ b/tools/chemenu/provenance.py @@ -349,7 +349,7 @@ def page_raw_files(pages: dict[str, Page], page: Page) -> list[str]: def uncovered_raw_files(raw_dir: Path, pages: dict[str, Page]) -> list[str]: """Raw files with no source page claiming to cover them.""" covered = set(source_pages_by_raw_file(pages)) - all_raw = {str(p.relative_to(config.ROOT)) for p in config.iter_raw_files(raw_dir)} + all_raw = {p.relative_to(config.ROOT).as_posix() for p in config.iter_raw_files(raw_dir)} return sorted(all_raw - covered) diff --git a/tools/chemenu/search/base.py b/tools/chemenu/search/base.py index d68a62a..a813ee2 100644 --- a/tools/chemenu/search/base.py +++ b/tools/chemenu/search/base.py @@ -32,6 +32,6 @@ class SearchBackend(Protocol): def page_key(path: Path, root: Path) -> str: """Repo-relative path string, the key both sides of the backend boundary use.""" try: - return str(path.relative_to(root)) + return path.relative_to(root).as_posix() except ValueError: - return str(path) + return path.as_posix() diff --git a/tools/chemenu/search/ripgrep.py b/tools/chemenu/search/ripgrep.py index b87b944..d33bb95 100644 --- a/tools/chemenu/search/ripgrep.py +++ b/tools/chemenu/search/ripgrep.py @@ -19,6 +19,7 @@ Three safety properties are load-bearing and must survive any edit here: from __future__ import annotations import json +import os import subprocess from pathlib import Path from typing import Iterable @@ -86,9 +87,25 @@ def _iter_match_records(stdout: str) -> Iterable[dict]: yield record.get("data", {}) +# The platform's own path separator, read through this name so a test can +# simulate Windows on any host. +NATIVE_SEPARATOR = os.sep + + def _record_path(data: dict) -> str | None: + """The matched file's path, always with `/` separators. + + On Windows `rg` writes `kb\\comparisons\\COLLECTION.md`, and its + `--path-separator /` does not apply to `--json` output (Gitea #152, T3), so + the separator is converted here instead - the one place an `rg` path enters. + The conversion loses nothing: no Windows path component can contain a `\\`, + and no page title may contain one on any platform (`titles.py`). + """ path = data.get("path") or {} - return path.get("text") + text = path.get("text") + if text is not None and NATIVE_SEPARATOR == "\\": + text = text.replace("\\", "/") + return text class RipgrepBackend: @@ -115,6 +132,7 @@ class RipgrepBackend: argv, capture_output=True, text=True, + encoding="utf-8", check=False, timeout=RIPGREP_TIMEOUT_SECONDS, ) diff --git a/tools/chemenu/telemetry/writer.py b/tools/chemenu/telemetry/writer.py index 42bb61e..9af3b77 100644 --- a/tools/chemenu/telemetry/writer.py +++ b/tools/chemenu/telemetry/writer.py @@ -28,7 +28,7 @@ import json import os from pathlib import Path -from chemenu import config +from chemenu import config, filelock from chemenu.session import session_id as current_session_id from chemenu.session import session_id_source as current_session_id_source from chemenu.session import session_slug @@ -136,7 +136,7 @@ def write_event( raise ValueError(f"invalid trace event: {'; '.join(errors)}") line = json.dumps(record, ensure_ascii=False, separators=(",", ":")) + "\n" - with open(target, "a", encoding="utf-8") as handle: + with open(target, "a", encoding="utf-8", newline="\n") as handle: _locked_write(handle, line) return record @@ -152,7 +152,7 @@ def _seed_session_header(target: Path, source: str, session: str) -> None: and an `exists()` check would let two of them both write the header. """ try: - handle = open(target, "x", encoding="utf-8") + handle = open(target, "x", encoding="utf-8", newline="\n") except FileExistsError: return with handle: @@ -179,18 +179,9 @@ def _seed_session_header(target: Path, source: str, session: str) -> None: def _locked_write(handle, line: str) -> None: - try: - import fcntl - except ImportError: # pragma: no cover - non-POSIX platform + with filelock.exclusive(handle): handle.write(line) handle.flush() - return - fcntl.flock(handle, fcntl.LOCK_EX) - try: - handle.write(line) - handle.flush() - finally: - fcntl.flock(handle, fcntl.LOCK_UN) def _enforce_retention(root: Path, keep: int, exclude: str) -> None: @@ -244,7 +235,7 @@ def _mark_limit_once(session_dir: Path, source: str, session: str, limit: int) - `exists()` check would let more than one of them win. """ try: - handle = open(session_dir / LIMIT_MARKER, "x", encoding="utf-8") + handle = open(session_dir / LIMIT_MARKER, "x", encoding="utf-8", newline="\n") except FileExistsError: return handle.close() diff --git a/tools/chemenu/tests/test_dist_cmd.py b/tools/chemenu/tests/test_dist_cmd.py index 03e3158..f782f24 100644 --- a/tools/chemenu/tests/test_dist_cmd.py +++ b/tools/chemenu/tests/test_dist_cmd.py @@ -41,6 +41,7 @@ def repo(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: (root / "EVALS.md").write_text("# EVALS\n", encoding="utf-8") (root / "CLAUDE.md").write_text("# CLAUDE\n\n@AGENTS.md\n", encoding="utf-8") (root / ".gitignore").write_text("*.pyc\n", encoding="utf-8") + (root / ".gitattributes").write_text("* text=auto eol=lf\n", encoding="utf-8") (root / "VERSION").write_text("0.3.1\n", encoding="utf-8") for name in config.LICENSE_FILES: @@ -291,6 +292,13 @@ def test_plan_ships_the_environment_template_but_not_the_filled_file(repo): assert config.ENVIRONMENT_FILE not in plan +def test_plan_ships_the_line_ending_rule(repo): + """An instance cloned onto Windows with `core.autocrlf=true` needs it as much + as this repository does: without it the sh launcher gets CRLF (Gitea #152).""" + plan = dist_cmd.build_plan() + assert "eol=lf" in plan[".gitattributes"].content + + def test_plan_ships_the_claude_harness_shim(repo): """Claude Code loads `CLAUDE.md` and not `AGENTS.md`, so a distributed instance running that harness would start every session without the diff --git a/tools/chemenu/tests/test_portability.py b/tools/chemenu/tests/test_portability.py new file mode 100644 index 0000000..29d046d --- /dev/null +++ b/tools/chemenu/tests/test_portability.py @@ -0,0 +1,423 @@ +"""Windows portability, held on a Linux CI that never runs Windows (Gitea #152). + +Most of what breaks on Windows is code that is right on POSIX by accident: a path +stringified with the native separator, a text file written in the native line ending, +a pipe decoded in the locale's code page, a lock imported from a module Windows does +not have. None of it fails here, so nothing here would notice it coming back. + +The guards below therefore read the source rather than run it. Each guard comes with +a test that feeds it the defect it exists for, because a guard that silently matches +nothing passes for the wrong reason. The remaining tests simulate the Windows side of +what can be simulated: a missing `fcntl`, a `cp1252` pipe. +""" +from __future__ import annotations + +import ast +import os +import subprocess +import sys +import types +from pathlib import Path + +import pytest + +from chemenu import filelock + +TOOLS_DIR = Path(__file__).resolve().parents[2] +PACKAGE_DIR = TOOLS_DIR / "chemenu" + +# Receivers whose `open` is not a text-file open: `os.open` takes flags, and the +# archive modules open binary members. +NOT_TEXT_OPEN = {"os", "tarfile", "zipfile", "gzip"} + +# The one module allowed to import the platform lock modules. +LOCK_MODULE = PACKAGE_DIR / "filelock.py" + + +def _source_files() -> list[Path]: + """Every shipped Python file: `tools/chemenu` without its tests, and the scripts + beside it in `tools/` - `trace_ingest.py` among them, which a harness runs on + every tool call.""" + package = (p for p in PACKAGE_DIR.rglob("*.py") if "tests" not in p.relative_to(PACKAGE_DIR).parts) + return sorted([*package, *TOOLS_DIR.glob("*.py")]) + + +def _call_name(call: ast.Call) -> tuple[str | None, str | None]: + """(function name, receiver name) - `('open', 'os')` for `os.open(...)`.""" + func = call.func + if isinstance(func, ast.Name): + return func.id, None + if isinstance(func, ast.Attribute): + receiver = func.value.id if isinstance(func.value, ast.Name) else None + return func.attr, receiver + return None, None + + +def _keywords(call: ast.Call) -> set[str | None]: + return {keyword.arg for keyword in call.keywords} + + +def _open_mode(call: ast.Call, receiver: str | None) -> ast.expr | None: + """The mode argument of an `open`: second for `open`/`io.open`, first for `Path.open`.""" + for keyword in call.keywords: + if keyword.arg == "mode": + return keyword.value + index = 1 if isinstance(call.func, ast.Name) or receiver == "io" else 0 + return call.args[index] if len(call.args) > index else None + + +# --- the four detectors, each a function of source text -------------------------- + + +def stringified_relative_paths(source: str) -> list[int]: + """Lines that turn a `relative_to` result into a string with the native separator. + + `str(p.relative_to(root))` and `f"{p.relative_to(root)}"` both give `kb\\x.md` on + Windows; `.as_posix()` is the portable spelling. + """ + def is_relative_to(node: ast.AST) -> bool: + return ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and node.func.attr == "relative_to" + ) + + lines = [] + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.Call) and _call_name(node) == ("str", None): + if node.args and is_relative_to(node.args[0]): + lines.append(node.lineno) + if isinstance(node, ast.FormattedValue) and is_relative_to(node.value): + lines.append(node.value.lineno) + return sorted(lines) + + +def unportable_file_io(source: str) -> list[tuple[int, str]]: + """Text file access that depends on the platform: a missing `encoding=` on any + text open, read or write, and a missing `newline=` on a write. + + Without `encoding=`, Windows reads and writes in the locale's code page; without + `newline=`, a write produces CRLF there. + """ + findings = [] + for node in ast.walk(ast.parse(source)): + if not isinstance(node, ast.Call): + continue + name, receiver = _call_name(node) + keywords = _keywords(node) + method = isinstance(node.func, ast.Attribute) + if name == "read_text" and method: + writes = False + elif name == "write_text" and method: + writes = True + elif name == "open" and receiver not in NOT_TEXT_OPEN: + mode = _open_mode(node, receiver) + if mode is None: + writes = False + elif isinstance(mode, ast.Constant) and isinstance(mode.value, str): + if "b" in mode.value: + continue + writes = bool(set(mode.value) & set("wax+")) + else: + writes = True # a mode this scan cannot read is treated as a write + else: + continue + if "encoding" not in keywords: + findings.append((node.lineno, f"{name} without encoding=")) + if writes and "newline" not in keywords: + findings.append((node.lineno, f"{name} writes without newline=")) + return sorted(findings) + + +def undecoded_subprocess_text(source: str) -> list[int]: + """Lines where a subprocess call asks for text but not for an encoding. + + `text=True` alone decodes with the locale's code page - cp1252 on Windows - and + `git` and `rg` write UTF-8. Only calls spelled `subprocess.(...)` are read: + `text=` is an ordinary parameter name elsewhere, and every call site in the + package uses that spelling. + """ + lines = [] + for node in ast.walk(ast.parse(source)): + if not isinstance(node, ast.Call) or _call_name(node)[1] != "subprocess": + continue + for keyword in node.keywords: + if keyword.arg in ("text", "universal_newlines") and not ( + isinstance(keyword.value, ast.Constant) and keyword.value.value is False + ): + if "encoding" not in _keywords(node): + lines.append(node.lineno) + return sorted(lines) + + +def platform_lock_imports(source: str) -> list[int]: + """Lines importing `fcntl` or `msvcrt` - which only `chemenu.filelock` may do.""" + lines = [] + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.Import) and any(a.name in ("fcntl", "msvcrt") for a in node.names): + lines.append(node.lineno) + if isinstance(node, ast.ImportFrom) and node.module in ("fcntl", "msvcrt"): + lines.append(node.lineno) + return sorted(lines) + + +def _scan(detector, *, skip: Path | None = None) -> list[str]: + findings = [] + for path in _source_files(): + if path == skip: + continue + relative = path.relative_to(TOOLS_DIR).as_posix() + for finding in detector(path.read_text(encoding="utf-8")): + findings.append(f"{relative}: {finding}") + return findings + + +# --- the guards ------------------------------------------------------------------ + + +def test_the_guards_scan_the_whole_package(): + files = _source_files() + assert len(files) > 60 + assert PACKAGE_DIR / "search" / "ripgrep.py" in files + assert TOOLS_DIR / "trace_ingest.py" in files + assert not any("tests" in p.relative_to(TOOLS_DIR).parts for p in files) + + +def test_no_relative_path_is_stringified_with_the_native_separator(): + assert _scan(stringified_relative_paths) == [] + + +def test_every_text_file_access_names_its_encoding_and_every_write_its_newline(): + assert _scan(unportable_file_io) == [] + + +def test_every_text_subprocess_names_its_encoding(): + assert _scan(undecoded_subprocess_text) == [] + + +def test_subprocess_is_only_called_through_its_module_name(): + """The spelling `undecoded_subprocess_text` relies on - a `from subprocess + import run` would take its call sites out of the guard's sight.""" + def from_imports(source: str) -> list[int]: + return [ + node.lineno for node in ast.walk(ast.parse(source)) + if isinstance(node, ast.ImportFrom) and node.module == "subprocess" + ] + + assert _scan(from_imports) == [] + + +def test_only_the_lock_module_imports_a_platform_lock(): + assert _scan(platform_lock_imports, skip=LOCK_MODULE) == [] + assert platform_lock_imports(LOCK_MODULE.read_text(encoding="utf-8")) != [] + + +# --- each detector catches the defect it exists for ------------------------------ + + +@pytest.mark.parametrize("source", [ + "x = str(path.relative_to(root))", + "x = str(Path(p).relative_to(config.ROOT))", + 'x = f"{path.relative_to(root)} is missing"', +]) +def test_a_stringified_relative_path_is_caught(source): + assert stringified_relative_paths(source) == [1] + + +def test_a_posix_relative_path_passes(): + assert stringified_relative_paths("x = path.relative_to(root).as_posix()") == [] + assert stringified_relative_paths("x = path.relative_to(root).parts[0]") == [] + + +@pytest.mark.parametrize("source", [ + 'path.write_text(text, encoding="utf-8")', + 'open(target, "a", encoding="utf-8")', + 'path.open("w", encoding="utf-8")', + 'open(target, mode="x", encoding="utf-8")', + 'open(target, mode, encoding="utf-8")', +]) +def test_a_write_without_newline_is_caught(source): + assert unportable_file_io(source) != [] + + +@pytest.mark.parametrize("source", [ + "path.read_text()", + "open(target)", + 'path.write_text(text, newline="\\n")', +]) +def test_a_text_access_without_encoding_is_caught(source): + assert unportable_file_io(source) != [] + + +@pytest.mark.parametrize("source", [ + 'write_text(path, text)', # a module's own helper of that name, not Path's + 'path.write_text(text, encoding="utf-8", newline="\\n")', + 'open(target, "a", encoding="utf-8", newline="\\n")', + 'path.read_text(encoding="utf-8")', + 'open(target, "rb")', + 'dest.open("wb")', + "os.open(path, flags)", + 'tarfile.open(path, "w:gz")', +]) +def test_portable_file_access_passes(source): + assert unportable_file_io(source) == [] + + +def test_text_without_encoding_is_caught_and_with_it_passes(): + assert undecoded_subprocess_text("subprocess.run(argv, capture_output=True, text=True)") == [1] + assert undecoded_subprocess_text("subprocess.run(argv, universal_newlines=True)") == [1] + assert undecoded_subprocess_text('subprocess.run(argv, text=True, encoding="utf-8")') == [] + assert undecoded_subprocess_text("subprocess.run(argv, capture_output=True)") == [] + + +def test_a_platform_lock_import_is_caught(): + assert platform_lock_imports("import fcntl") == [1] + assert platform_lock_imports("def f():\n import msvcrt\n") == [2] + assert platform_lock_imports("from fcntl import flock") == [1] + + +# --- the lock without fcntl ------------------------------------------------------ + + +class FakeMsvcrt(types.ModuleType): + """What `msvcrt.locking` is asked to do, and from which position.""" + + LK_UNLCK, LK_LOCK, LK_NBLCK = 0, 1, 2 + + def __init__(self, failures: int = 0): + super().__init__("msvcrt") + self.calls: list[tuple[int, int, int]] = [] + self.failures = failures + + def locking(self, fd: int, mode: int, nbytes: int) -> None: + self.calls.append((mode, os.lseek(fd, 0, os.SEEK_CUR), nbytes)) + if mode == self.LK_LOCK and self.failures: + self.failures -= 1 + raise OSError(36, "Resource deadlock avoided") + + +@pytest.fixture +def windows_locking(monkeypatch): + """Hide `fcntl` the way Windows does and hand `chemenu.filelock` a fake `msvcrt`.""" + fake = FakeMsvcrt() + monkeypatch.setitem(sys.modules, "fcntl", None) # `import fcntl` now raises ImportError + monkeypatch.setitem(sys.modules, "msvcrt", fake) + return fake + + +def test_without_fcntl_the_lock_falls_back_to_msvcrt(tmp_path, windows_locking): + target = tmp_path / "trace.jsonl" + with open(target, "a", encoding="utf-8", newline="\n") as handle: + handle.write("first\n") + handle.flush() + before = os.lseek(handle.fileno(), 0, os.SEEK_CUR) + with filelock.exclusive(handle): + assert windows_locking.calls == [(FakeMsvcrt.LK_LOCK, filelock.WINDOWS_LOCK_OFFSET, 1)] + assert os.lseek(handle.fileno(), 0, os.SEEK_CUR) == before + handle.write("second\n") + assert windows_locking.calls[-1] == (FakeMsvcrt.LK_UNLCK, filelock.WINDOWS_LOCK_OFFSET, 1) + # The lock sits past the data, so the lines themselves are untouched. + assert target.read_text(encoding="utf-8") == "first\nsecond\n" + + +def test_a_contended_windows_lock_is_waited_for_like_flock(tmp_path, monkeypatch): + fake = FakeMsvcrt(failures=2) + monkeypatch.setitem(sys.modules, "fcntl", None) + monkeypatch.setitem(sys.modules, "msvcrt", fake) + with open(tmp_path / "budget.lock", "a", encoding="utf-8", newline="\n") as handle: + with filelock.exclusive(handle): + pass + modes = [mode for mode, _, _ in fake.calls] + assert modes == [FakeMsvcrt.LK_LOCK] * 3 + [FakeMsvcrt.LK_UNLCK] + + +def test_the_budget_counter_locks_without_fcntl(tmp_path, monkeypatch, windows_locking): + from chemenu.commands import run_budget + + monkeypatch.setattr(run_budget, "STATE_DIR", tmp_path) + monkeypatch.setattr(run_budget, "LOCK_FILE", tmp_path / "budget.lock") + with run_budget._state_lock(): + pass + assert [mode for mode, _, _ in windows_locking.calls] == [FakeMsvcrt.LK_LOCK, FakeMsvcrt.LK_UNLCK] + + +def test_the_telemetry_writer_locks_without_fcntl(tmp_path, windows_locking): + from chemenu.telemetry import writer + + target = tmp_path / "trace.jsonl" + with open(target, "a", encoding="utf-8", newline="\n") as handle: + writer._locked_write(handle, '{"event":"x"}\n') + assert [mode for mode, _, _ in windows_locking.calls] == [FakeMsvcrt.LK_LOCK, FakeMsvcrt.LK_UNLCK] + assert target.read_text(encoding="utf-8") == '{"event":"x"}\n' + + +def test_with_fcntl_the_lock_is_flock(tmp_path): + fcntl = pytest.importorskip("fcntl") + with open(tmp_path / "lock", "a", encoding="utf-8", newline="\n") as handle: + with filelock.exclusive(handle): + with open(tmp_path / "lock", "a", encoding="utf-8", newline="\n") as other: + with pytest.raises(BlockingIOError): + fcntl.flock(other.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + + +# --- the launcher's own output --------------------------------------------------- + + +def _launcher_stderr(**env: str) -> bytes: + """What `run_wikitool.py` writes for an unknown command named `Fußnoten`. + + Typer echoes the command name back, which makes it a non-ASCII line that + needs no checkout, no corpus and no preflight. + """ + result = subprocess.run( + [sys.executable, str(TOOLS_DIR / "run_wikitool.py"), "Fußnoten"], + capture_output=True, + env={**os.environ, **env}, + ) + assert result.returncode != 0 + return result.stderr + + +def test_output_into_a_pipe_is_utf8(): + """Linux and macOS: the bytes are what they always were.""" + assert "Fußnoten".encode("utf-8") in _launcher_stderr() + + +def test_output_into_a_pipe_is_utf8_even_under_a_cp1252_locale(): + """What Windows does to a pipe, simulated: Python would otherwise write `Fu\\xdfnoten`.""" + stderr = _launcher_stderr(PYTHONIOENCODING="cp1252") + assert "Fußnoten".encode("utf-8") in stderr + assert "Fußnoten".encode("cp1252") not in stderr + + +# --- line endings in a checkout -------------------------------------------------- + + +@pytest.mark.parametrize("path, eol", [ + ("tools/wikitool", "lf"), + ("tools/trace-hook", "lf"), + ("tools/preflight.sh", "lf"), + ("AGENTS.md", "lf"), +]) +def test_every_text_file_is_checked_out_with_lf(path, eol): + """`core.autocrlf=true` would otherwise give the sh launchers `bash\\r`.""" + result = subprocess.run( + ["git", "check-attr", "text", "eol", "--", path], + cwd=TOOLS_DIR.parent, capture_output=True, text=True, encoding="utf-8", + ) + if result.returncode != 0: + pytest.skip("not a git checkout") + assert f"{path}: text: auto" in result.stdout + assert f"{path}: eol: {eol}" in result.stdout + + +def test_sources_are_kept_byte_for_byte(): + """raw/ is immutable: a CRLF source must not be normalized on `git add`.""" + result = subprocess.run( + ["git", "check-attr", "text", "--", "raw/articles/x.md", "incoming/x.md"], + cwd=TOOLS_DIR.parent, capture_output=True, text=True, encoding="utf-8", + ) + if result.returncode != 0: + pytest.skip("not a git checkout") + assert "raw/articles/x.md: text: unset" in result.stdout + assert "incoming/x.md: text: unset" in result.stdout diff --git a/tools/chemenu/tests/test_search.py b/tools/chemenu/tests/test_search.py index 6101600..7673dbc 100644 --- a/tools/chemenu/tests/test_search.py +++ b/tools/chemenu/tests/test_search.py @@ -1,3 +1,4 @@ +import json import subprocess import time from pathlib import Path @@ -442,3 +443,50 @@ def test_a_page_with_no_frontmatter_at_all_is_reported(kb_dir, tmp_path): (kb_dir / "entities" / "Naked.md").write_text("# Naked\n\nProse only.\n", encoding="utf-8") reported = unreadable_pages(load_pages_by_path(kb_dir, tmp_path)) assert [entry["path"] for entry in reported] == ["kb/entities/Naked.md"] + + +# What `rg --json --path-separator / Contract kb` printed on the Windows target +# system (Gitea #152, T3): the flag does not reach JSON output. +T3_MATCH = ( + r'{"type":"match","data":{"path":{"text":"kb\\comparisons\\COLLECTION.md"},' + r'"lines":{"text":"Contract\n"},"line_number":1,"absolute_offset":0,"submatches":[]}}' +) + + +def test_a_windows_rg_path_reaches_the_parser_with_forward_slashes(monkeypatch): + data = json.loads(T3_MATCH)["data"] + monkeypatch.setattr(ripgrep, "NATIVE_SEPARATOR", "\\") + assert ripgrep._record_path(data) == "kb/comparisons/COLLECTION.md" + + +def test_a_posix_rg_path_is_left_as_it_is(monkeypatch): + """On POSIX a backslash is an ordinary character of a name, not a separator.""" + data = json.loads(T3_MATCH)["data"] + monkeypatch.setattr(ripgrep, "NATIVE_SEPARATOR", "/") + assert ripgrep._record_path(data) == "kb\\comparisons\\COLLECTION.md" + + +@pytest.mark.parametrize("separator, found", [("\\", True), ("/", False)]) +def test_a_windows_rg_match_becomes_a_full_hit(monkeypatch, kb_dir, tmp_path, pages, separator, found): + """The T3 line shape, pointed at a real fixture page: with the Windows separator + the hit carries the repo-relative POSIX key, its title and its frontmatter.""" + page = kb_dir / "entities" / "systems" / "aurora.md" + match = json.loads(T3_MATCH) + match["data"]["path"]["text"] = str(page).replace("/", "\\") + match["data"]["lines"]["text"] = "Hosts things.\n" + + def fake_run(argv, **kwargs): + return subprocess.CompletedProcess(argv, 0, json.dumps(match) + "\n", "") + + monkeypatch.setattr(ripgrep.subprocess, "run", fake_run) + monkeypatch.setattr(ripgrep, "NATIVE_SEPARATOR", separator) + hits = RipgrepBackend(kb_dir, tmp_path).search(SearchQuery(text="Hosts"), pages) + if not found: + assert hits == [] + return + [hit] = hits + assert hit.path == "kb/entities/systems/aurora.md" + assert hit.title == "aurora" + assert hit.kind == "entity" + assert hit.collection == "entities" + assert "ZFS" in hit.summary diff --git a/tools/chemenu/tests/test_trace_ingest.py b/tools/chemenu/tests/test_trace_ingest.py index de2d8f9..253a707 100644 --- a/tools/chemenu/tests/test_trace_ingest.py +++ b/tools/chemenu/tests/test_trace_ingest.py @@ -5,6 +5,7 @@ change in our normalisation shows up here rather than in a silent gap in a scored run. """ import json +import os import subprocess import sys import tomllib @@ -145,6 +146,23 @@ def test_malformed_stdin_never_fails_a_tool_call(tmp_path, payload): assert result.stdout == "" +def test_a_utf8_payload_survives_a_cp1252_locale(): + """What Windows does to `sys.stdin`, simulated: the harness writes UTF-8, and a + locale-decoded read fails outright on a byte cp1252 leaves undefined - `Ł` is + `C5 81` in UTF-8 (Gitea #152). A `ß` alone would not show it: misread on the + way in and miswritten on the way out, it comes back intact.""" + payload = {**VIBE_POST_TOOL, "session_id": "v-Łódź-Fußnoten"} + result = subprocess.run( + [sys.executable, str(SCRIPT), "--source", "mistral-vibe", "--dry-run"], + input=json.dumps(payload, ensure_ascii=False).encode("utf-8"), + capture_output=True, + env={**os.environ, "PYTHONIOENCODING": "cp1252"}, + ) + assert result.returncode == 0, result.stderr + record = json.loads(result.stdout.decode("utf-8")) + assert record["session_id"] == "v-Łódź-Fußnoten" + + def test_dry_run_prints_exactly_one_valid_event(): result = subprocess.run( [sys.executable, str(SCRIPT), "--source", "mistral-vibe", "--dry-run"], diff --git a/tools/chemenu/type_resolver.py b/tools/chemenu/type_resolver.py index 62b9fbf..cdb23f4 100644 --- a/tools/chemenu/type_resolver.py +++ b/tools/chemenu/type_resolver.py @@ -76,9 +76,9 @@ class TypeResolver: candidate = (source_file.parent / type_path).resolve() # Ensure it's within the repo and starts with types/ or has types/ in path try: - candidate.relative_to(self.repo_root) + relative = candidate.relative_to(self.repo_root) # Allow relative paths that resolve to types/ or kb/**/types/ - if 'types' in str(candidate.relative_to(self.repo_root).parts): + if 'types' in relative.parts: if candidate.exists() and candidate.is_file(): return candidate except ValueError: @@ -170,7 +170,7 @@ class TypeResolver: # Verify type field points to valid type-spec or is self-referential type_ref = frontmatter['type'] - if type_ref != str(path.relative_to(self.repo_root)): + if type_ref != path.relative_to(self.repo_root).as_posix(): # Should be self-referential or point to parent type-spec try: parent_path = self.resolve_type_path(type_ref, path) @@ -523,7 +523,7 @@ class TypeResolver: for path in sorted(config.TYPES_DIR.glob("*.md")): frontmatter, _ = read_page(path) if frontmatter.get("type") == "types/type-spec.md": - specs.append((str(path.relative_to(self.repo_root)), frontmatter)) + specs.append((path.relative_to(self.repo_root).as_posix(), frontmatter)) return specs def find_type_by_name(self, name: str) -> Optional[str]: diff --git a/tools/chemenu/upload.py b/tools/chemenu/upload.py index 7a0cb8e..64b1b92 100644 --- a/tools/chemenu/upload.py +++ b/tools/chemenu/upload.py @@ -238,7 +238,7 @@ def _append_ledger(root: "Path | str", event: dict) -> None: base.mkdir(parents=True, exist_ok=True) path = base / "ledger.jsonl" line = json.dumps(event, sort_keys=True) + "\n" - with path.open("a", encoding="utf-8") as fh: + with path.open("a", encoding="utf-8", newline="\n") as fh: fh.write(line) diff --git a/tools/chemenu/version.py b/tools/chemenu/version.py index 76e0266..0558ccc 100644 --- a/tools/chemenu/version.py +++ b/tools/chemenu/version.py @@ -298,7 +298,7 @@ def read_version() -> Version: def write_version(version: Version) -> None: - version_file().write_text(f"{version}\n", encoding="utf-8") + version_file().write_text(f"{version}\n", encoding="utf-8", newline="\n") def read_stamp() -> Optional[dict]: diff --git a/tools/run_wikitool.py b/tools/run_wikitool.py index 25dcf0b..d17419e 100644 --- a/tools/run_wikitool.py +++ b/tools/run_wikitool.py @@ -7,8 +7,27 @@ on Windows, one more thing for two launchers to get right. The name deliberately does not start with `wikitool.`: the Windows Python installer can add `.PY` to `PATHEXT`, and a `tools/wikitool.py` could then be what PowerShell runs for `tools/wikitool`. + +stdout and stderr are set to UTF-8 here, the one file both launchers run. On +Windows, Python writes into a pipe in the locale's code page (cp1252), and every +harness reads its commands' output through a pipe: Git Bash passes the bytes on +unchanged, and PowerShell decodes them with `[Console]::OutputEncoding`, which is +UTF-8 under the harnesses (measured, Gitea #152). Without this, `Fußnoten` arrives +as `Fu�noten`. A console is unaffected: Python writes to one through the Windows +console API, whatever the encoding says. On Linux and macOS the locale is UTF-8 +already, so nothing changes there. """ +import sys + from chemenu.cli import main + +def use_utf8_output() -> None: + for stream in (sys.stdout, sys.stderr): + if stream is not None and hasattr(stream, "reconfigure"): + stream.reconfigure(encoding="utf-8") + + if __name__ == "__main__": + use_utf8_output() main() diff --git a/tools/trace_ingest.py b/tools/trace_ingest.py index a3bea71..6eba395 100755 --- a/tools/trace_ingest.py +++ b/tools/trace_ingest.py @@ -232,7 +232,11 @@ def main() -> int: ) args = parser.parse_args() - raw = sys.stdin.read() if not sys.stdin.isatty() else "" + # Bytes, decoded here: a harness writes its payload as UTF-8, and Python on + # Windows would decode `sys.stdin` with the locale's code page (Gitea #152). + raw = "" + if not sys.stdin.isatty(): + raw = sys.stdin.buffer.read().decode("utf-8", errors="replace") try: payload = json.loads(raw) if raw.strip() else {} except json.JSONDecodeError: @@ -255,6 +259,7 @@ def main() -> int: if errors: sys.stderr.write("; ".join(errors) + "\n") return 1 + sys.stdout.reconfigure(encoding="utf-8") sys.stdout.write(json.dumps(record, ensure_ascii=False) + "\n") return 0