From a0e9c8dbe883c241b2171a6cf54ca97eaca9c009 Mon Sep 17 00:00:00 2001
From: woolcoxm <woolcoxm@hotmail.com>
Date: Tue, 14 Jul 2026 07:49:17 -0400
Subject: [PATCH] =?UTF-8?q?doctor:=20platform-aware=20engine.binary=20chec?=
 =?UTF-8?q?k=20=E2=80=94=20Windows=20has=20no=20X=5FOK=20bit=20(#178,=20fi?=
 =?UTF-8?q?xes=20#141)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On Windows, os.access(path, os.X_OK) always returns True for any existing
file (NTFS has no execute bit; executability is governed by file extension,
not mode bits). So the test_non_executable_engine test — which chmods the
engine to 0o644 and expects 'fail' — could never pass on Windows.

Fixed doctor.py to use a platform-aware check: on Windows, any existing
file is executable; on POSIX, honor the mode bits via os.access(X_OK).

Fixed the test to assert the correct per-platform expectation: 'pass' on
Windows (chmod is a no-op for executability), 'fail' on POSIX.

Co-authored-by: woolcoxm <13604288+woolcoxm@users.noreply.github.com>
---
 c/doctor.py            | 12 +++++++++++-
 c/tests/test_doctor.py |  9 ++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/c/doctor.py b/c/doctor.py
index 0fb74af..da64704 100644
--- a/c/doctor.py
+++ b/c/doctor.py
@@ -2,6 +2,7 @@
 """Read-only installation diagnostics for colibri."""
 
 import os
+import sys
 import json
 import subprocess
 from pathlib import Path
@@ -63,7 +64,16 @@ def run_doctor(model, ram_gb=0, context=4096, gpu_indices=None, vram_gb=0, *,
         checks.append(_check("storage.persistence", "skip", "persistence requires a model directory"))
 
     engine = Path(engine_path)
-    if engine.is_file() and os.access(engine, os.X_OK):
+    # On Windows, os.access(X_OK) always returns True for any existing file
+    # (NTFS has no execute bit; executability is governed by file extension).
+    # So a chmod(0o644) "non-executable" scenario can't be detected via X_OK
+    # on Windows. Use a platform-aware check: on POSIX, honor the mode bits;
+    # on Windows, any existing file is treated as executable. (#141)
+    if sys.platform == "win32":
+        engine_ok = engine.is_file()
+    else:
+        engine_ok = engine.is_file() and os.access(engine, os.X_OK)
+    if engine_ok:
         checks.append(_check("engine.binary", "pass", "engine executable is ready", path=str(engine)))
     elif engine.is_file():
         checks.append(_check("engine.binary", "fail", "engine exists but is not executable", path=str(engine)))
diff --git a/c/tests/test_doctor.py b/c/tests/test_doctor.py
index 1647a0d..8950147 100644
--- a/c/tests/test_doctor.py
+++ b/c/tests/test_doctor.py
@@ -104,7 +104,14 @@ class DoctorTest(unittest.TestCase):
         report = self.report(ram_gb=40)
         checks = self.checks_by_id(report)
 
-        self.assertEqual(checks["engine.binary"]["status"], "fail")
+        # On Windows chmod(0o644) does not remove executability (NTFS has no
+        # execute bit; os.access(X_OK) is always True for existing files), so
+        # the engine.binary check stays "pass" there. The excessive-RAM check
+        # (memory.ram) is platform-independent and must still fail. (#141)
+        if sys.platform == "win32":
+            self.assertEqual(checks["engine.binary"]["status"], "pass")
+        else:
+            self.assertEqual(checks["engine.binary"]["status"], "fail")
         self.assertEqual(checks["memory.ram"]["status"], "fail")
         self.assertEqual(report["status"], "error")