From 296f24c59e6bfefc043747c29ba62f68ac589e0d Mon Sep 17 00:00:00 2001 From: sid Date: Thu, 30 Jul 2026 23:10:57 -0600 Subject: [PATCH] fix: preflight installs and preserve unrelated hooks --- install | 96 +++++++++++++++++++++++++------------------ tests/test_install.py | 32 +++++++++++++++ 2 files changed, 88 insertions(+), 40 deletions(-) diff --git a/install b/install index 936ffe7..74ee748 100755 --- a/install +++ b/install @@ -94,8 +94,17 @@ def read_json(path: Path) -> dict[str, Any]: def is_coord_hook(item: object) -> bool: - return isinstance(item, dict) and isinstance(item.get("command"), str) and ( - ".coord/" in item["command"] or ".claude/coord/" in item["command"] + if not isinstance(item, dict) or not isinstance(item.get("command"), str): + return False + command = item["command"] + return any( + endpoint in command + for endpoint in ( + ".coord/hook.py", + ".coord/session.py", + ".claude/coord/hook.py", + ".claude/coord/session.py", + ) ) @@ -170,7 +179,12 @@ def write_if_changed( temporary = path.with_name(f".{path.name}.coord-new-{secrets.token_hex(6)}") fd = os.open(temporary, os.O_WRONLY | os.O_CREAT | os.O_EXCL, expected_mode) try: - os.write(fd, content) + remaining = memoryview(content) + while remaining: + written = os.write(fd, remaining) + if written <= 0: + raise OSError(f"short write while creating {temporary}") + remaining = remaining[written:] os.fsync(fd) finally: os.close(fd) @@ -209,49 +223,35 @@ def install(target: Path, *, requested_id: str, check: bool) -> list[str]: if not target.is_dir(): raise ValueError(f"target is not a directory: {target}") selected_id = project_id(target, requested_id, check=check) - drift: list[str] = [] destination = target / ".coord" + planned: list[tuple[Path, bytes, bool]] = [] for name in CORE_FILES: - write_if_changed( - destination / name, - (HERE / name).read_bytes(), - executable=name in EXECUTABLES, - check=check, - drift=drift, + planned.append( + ( + destination / name, + (HERE / name).read_bytes(), + name in EXECUTABLES, + ) ) - write_if_changed( - destination / "project-id", - f"{selected_id}\n".encode(), - check=check, - drift=drift, + planned.append( + (destination / "project-id", f"{selected_id}\n".encode(), False) ) legacy = target / ".claude" / "coord" for name in ("coord", "hook.py", "session.py", "store.py", "workspace.py"): - write_if_changed( - legacy / name, - legacy_wrapper(name).encode(), - executable=name == "coord", - check=check, - drift=drift, + planned.append( + (legacy / name, legacy_wrapper(name).encode(), name == "coord") + ) + planned.extend( + ( + (legacy / "project-id", f"{selected_id}\n".encode(), False), + ( + legacy / "DESIGN.md", + b"Canonical design: [`../../.coord/DESIGN.md`](../../.coord/DESIGN.md).\n", + False, + ), + (legacy / ".gitignore", b"state/\n__pycache__/\n", False), ) - write_if_changed( - legacy / "project-id", - f"{selected_id}\n".encode(), - check=check, - drift=drift, - ) - write_if_changed( - legacy / "DESIGN.md", - b"Canonical design: [`../../.coord/DESIGN.md`](../../.coord/DESIGN.md).\\n", - check=check, - drift=drift, - ) - write_if_changed( - legacy / ".gitignore", - b"state/\\n__pycache__/\\n", - check=check, - drift=drift, ) settings = target / ".claude" / "settings.json" @@ -263,13 +263,29 @@ def install(target: Path, *, requested_id: str, check: bool) -> list[str]: ) + "\n" ).encode() - write_if_changed(settings, expected_claude, check=check, drift=drift) codex = target / ".codex" / "hooks.json" expected_codex = ( json.dumps(codex_hooks(read_json(codex)), indent=2, sort_keys=True) + "\n" ).encode() - write_if_changed(codex, expected_codex, check=check, drift=drift) + planned.extend( + ( + (settings, expected_claude, False), + (codex, expected_codex, False), + ) + ) + + # All source reads, identity checks, wrapper generation and hook JSON + # validation complete before the first target replacement. + drift: list[str] = [] + for path, content, executable in planned: + write_if_changed( + path, + content, + executable=executable, + check=check, + drift=drift, + ) return drift diff --git a/tests/test_install.py b/tests/test_install.py index b1aad01..0b586f6 100755 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -64,6 +64,11 @@ class InstallBehavior(unittest.TestCase): "command": "printf unrelated", "timeout": 2, }, + { + "type": "command", + "command": "python3 .coord/custom-security.py", + "timeout": 3, + }, { "type": "command", "command": "python3 .claude/coord/hook.py", @@ -115,10 +120,20 @@ class InstallBehavior(unittest.TestCase): for hook in group["hooks"] ] self.assertIn("printf unrelated", commands) + self.assertIn("python3 .coord/custom-security.py", commands) self.assertEqual( sum("/.coord/hook.py" in command for command in commands), 1 ) self.assertFalse(any(".claude/coord/" in command for command in commands)) + self.assertEqual( + (target / ".claude/coord/.gitignore").read_text(), + "state/\n__pycache__/\n", + ) + self.assertTrue( + (target / ".claude/coord/DESIGN.md") + .read_text() + .endswith("DESIGN.md).\n") + ) canonical = subprocess.run( [str(target / ".coord/coord"), "--help"], @@ -244,6 +259,23 @@ class InstallBehavior(unittest.TestCase): self.assertEqual(record["task"], "smoke-work") self.assertTrue(Path(record["actual_path"]).is_dir()) + def test_invalid_hook_json_refuses_before_any_target_replacement(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + target = Path(temporary) + legacy = target / ".claude/coord" + legacy.mkdir(parents=True) + (legacy / "project-id").write_text("preflight-project\n") + sentinel = legacy / "coord" + sentinel.write_text("legacy-sentinel\n") + (target / ".claude/settings.json").write_text("{ invalid") + + refused = self.run_install(target) + self.assertEqual(refused.returncode, 2) + self.assertIn("invalid JSON", refused.stderr) + self.assertEqual(sentinel.read_text(), "legacy-sentinel\n") + self.assertFalse((target / ".coord").exists()) + self.assertFalse((target / ".codex").exists()) + if __name__ == "__main__": unittest.main(verbosity=2)