fix(windows): resolve control flow and path quoting in scheduler task execution

2026-07-15 14:54:27 +05:30
parent 79d6117f6f
commit 7c7fdfcd6c
4 changed files with 291 additions and 73 deletions
@@ -6,42 +6,48 @@ set "SCRIPT_DIR=%~dp0"
 :: Strip trailing backslash
 set "SCRIPT_DIR=%SCRIPT_DIR:~0,-1%"

+set "REPO_ROOT="
 if exist "%SCRIPT_DIR%\..\skillopt_sleep" (
    cd /d "%SCRIPT_DIR%\.."
    set "REPO_ROOT=%CD%"
-) else if not "%CLAUDE_PLUGIN_ROOT%"=="" if exist "%CLAUDE_PLUGIN_ROOT%\..\..\skillopt_sleep" (
+    goto root_resolved
+)
+if not "%CLAUDE_PLUGIN_ROOT%"=="" if exist "%CLAUDE_PLUGIN_ROOT%\..\..\skillopt_sleep" (
    cd /d "%CLAUDE_PLUGIN_ROOT%\..\.."
    set "REPO_ROOT=%CD%"
-) else if not "%SKILLOPT_SLEEP_REPO%"=="" if exist "%SKILLOPT_SLEEP_REPO%\skillopt_sleep" (
+    goto root_resolved
+)
+if not "%SKILLOPT_SLEEP_REPO%"=="" if exist "%SKILLOPT_SLEEP_REPO%\skillopt_sleep" (
    set "REPO_ROOT=%SKILLOPT_SLEEP_REPO%"
-) else (
+    goto root_resolved
+)
+
 :: Search upward from current directory
 set "d=%CD%"
-    set "REPO_ROOT="
 :loop
 if exist "!d!\skillopt_sleep" (
    set "REPO_ROOT=!d!"
-        goto found
+    goto root_resolved
 )
 for %%I in ("!d!") do set "parent=%%~dpI"
 :: Strip trailing backslash from parent if it's not root
-    if "!parent!"=="!d!" goto notfound
+if "!parent!"=="!d!" goto root_resolved
 set "parent=!parent:~0,-1!"
-    if "!parent!"=="" goto notfound
+if "!parent!"=="" goto root_resolved
 set "d=!parent!"
 goto loop
-    :notfound
-    echo [sleep] ERROR: could not locate the skillopt_sleep package. Set SKILLOPT_SLEEP_REPO to the repo root. >&2
-    exit /b 1
-    :found
-)

-if not "%REPO_ROOT%"=="" (
-    :: Find python >= 3.10
+:root_resolved
+
+if "%REPO_ROOT%"=="" goto fallback_mode
+
+:: ── Source Checkout Mode ───────────────────────────────────────────
 set "PY="
 if not "%SKILLOPT_SLEEP_PYTHON%"=="" (
    set "PY=%SKILLOPT_SLEEP_PYTHON%"
-    ) else (
+    goto py_found
+)
+
 for %%p in (python3.exe python.exe py.exe) do (
    where %%p >nul 2>nul
    if !errorlevel! equ 0 (
@@ -52,9 +58,8 @@ if not "%REPO_ROOT%"=="" (
        )
    )
 )
-    )
-    :py_found

+:py_found
 if "%PY%"=="" (
    echo [sleep] ERROR: need Python >= 3.10 (found none). >&2
    exit /b 1
@@ -67,21 +72,23 @@ if not "%REPO_ROOT%"=="" (
    "%PY%" -m skillopt_sleep %*
 )
 exit /b !errorlevel!
-)

-:: No source checkout found — fall back to an installed engine.
+
+:: ── Fallback Mode (No Source Checkout) ─────────────────────────────
+:fallback_mode

 :: Fallback 1: skillopt-sleep CLI on PATH (uv tool install / pipx / pip install).
 where skillopt-sleep >nul 2>nul
-if !errorlevel! equ 0 (
+if !errorlevel! neq 0 goto try_fallback_2
+
 if "%~1" == "" (
    skillopt-sleep status
 ) else (
    skillopt-sleep %*
 )
 exit /b !errorlevel!
-)

+:try_fallback_2
 :: Fallback 2: importable as a module (pip install into the active Python).
 set "PY="
 for %%p in (python3.exe python.exe py.exe) do (
@@ -99,15 +106,16 @@ for %%p in (python3.exe python.exe py.exe) do (
 )
 :py_import_found

-if not "%PY%"=="" (
+if "%PY%" == "" goto not_found
+
 if "%~1" == "" (
    "%PY%" -m skillopt_sleep status
 ) else (
    "%PY%" -m skillopt_sleep %*
 )
 exit /b !errorlevel!
-)

+:not_found
 echo [sleep] ERROR: could not locate the skillopt_sleep package. >&2
 echo [sleep] Install it with 'uv tool install skillopt' or 'pip install skillopt', >&2
 echo [sleep] or set SKILLOPT_SLEEP_REPO to a clone of the SkillOpt repo. >&2
@@ -105,10 +105,22 @@ def _runner_cmd(project: str, backend: str, extra: str, python: str) -> str:
    logdir = os.path.join(project, ".skillopt-sleep")
    log = os.path.join(logdir, "cron.log")
    # use absolute python + -m so cron's/scheduler's minimal env still works
-    cmd = (f'{python} -m skillopt_sleep run --project "{project}" '
+    cmd = (f'"{python}" -m skillopt_sleep run --project "{project}" '
           f'--scope invoked --backend {backend} {extra}'.rstrip())
    if sys.platform == "win32":
-        return f'cmd.exe /c "if not exist \\"{logdir}\\" mkdir \\"{logdir}\\" && cd /d \\"{_repo_root()}\\" && {cmd} >> \\"{log}\\" 2>&1"'
+        helper_script = os.path.join(logdir, "run.cmd")
+        try:
+            os.makedirs(logdir, exist_ok=True)
+            content = (
+                "@echo off\n"
+                f'cd /d "{_repo_root()}"\n'
+                f'{cmd} >> "{log}" 2>&1\n'
+            )
+            with open(helper_script, "w", encoding="utf-8") as f:
+                f.write(content)
+        except Exception:
+            pass
+        return f'"{helper_script}"'
    return f'mkdir -p "{logdir}"; cd "{_repo_root()}" && {cmd} >> "{log}" 2>&1'


@@ -181,6 +193,13 @@ def unschedule(project: Optional[str] = None, *, all_projects: bool = False) ->
        elif project:
            tn = _win_task_name(project)
            ok = _delete_win_task(tn)
+            try:
+                logdir = os.path.join(project, ".skillopt-sleep")
+                helper = os.path.join(logdir, "run.cmd")
+                if os.path.exists(helper):
+                    os.remove(helper)
+            except Exception:
+                pass
            return ok, ("Removed." if ok else "Failed to remove scheduled task (does it exist?).")
        return False, "No project specified to unschedule."

@@ -116,3 +116,180 @@ Describe "run-sleep.ps1 runner" {
        }
    }
 }
+
+Describe "run-sleep.cmd runner" {
+    BeforeAll {
+        $script = Join-Path $PSScriptRoot "..\plugins\run-sleep.cmd"
+        $tempDir = [System.IO.Path]::GetTempFileName()
+        Remove-Item $tempDir
+        New-Item -ItemType Directory -Path $tempDir | Out-Null
+
+        # Detect a working python to bypass Windows Store alias issues
+        if (-not $env:SKILLOPT_SLEEP_PYTHON) {
+            if (Test-Path "C:\Python314\python.exe") {
+                $env:SKILLOPT_SLEEP_PYTHON = "C:\Python314\python.exe"
+            } elseif (Test-Path "C:\Python313\python.exe") {
+                $env:SKILLOPT_SLEEP_PYTHON = "C:\Python313\python.exe"
+            } elseif (Test-Path "C:\Python312\python.exe") {
+                $env:SKILLOPT_SLEEP_PYTHON = "C:\Python312\python.exe"
+            } elseif (Test-Path "C:\Python311\python.exe") {
+                $env:SKILLOPT_SLEEP_PYTHON = "C:\Python311\python.exe"
+            } elseif (Test-Path "C:\Python310\python.exe") {
+                $env:SKILLOPT_SLEEP_PYTHON = "C:\Python310\python.exe"
+            }
+        }
+    }
+
+    AfterAll {
+        if (Test-Path $tempDir) {
+            Remove-Item -Recurse -Force $tempDir
+        }
+    }
+
+    It "runs successfully in source checkout mode" {
+        $env:SKILLOPT_SLEEP_REPO = Resolve-Path (Join-Path $PSScriptRoot "..")
+        # Run help
+        $result = cmd.exe /c $script "--help" 2>&1
+        $LASTEXITCODE | Should Be 0
+        $result | Out-String | Should Match "skillopt_sleep"
+    }
+
+    It "falls back to CLI on PATH" {
+        # Create a temp dir for isolated testing
+        $sandbox = Join-Path $tempDir "cmd_cli_fallback"
+        New-Item -ItemType Directory -Path $sandbox | Out-Null
+        $scriptCopy = Join-Path $sandbox "run-sleep.cmd"
+        Copy-Item $script $scriptCopy
+
+        # Create fake CLI
+        $binDir = Join-Path $sandbox "bin"
+        New-Item -ItemType Directory -Path $binDir | Out-Null
+        $fakeCli = Join-Path $binDir "skillopt-sleep.cmd"
+        "@echo off`r`necho fake-cli invoked %*`r`nexit /b 0" | Out-File -FilePath $fakeCli -Encoding ascii
+
+        # Save existing env vars
+        $oldPath = $env:PATH
+        $oldRepo = $env:SKILLOPT_SLEEP_REPO
+        $oldPlugin = $env:CLAUDE_PLUGIN_ROOT
+
+        $env:PATH = "$binDir;$oldPath"
+        $env:SKILLOPT_SLEEP_REPO = $null
+        $env:CLAUDE_PLUGIN_ROOT = $null
+
+        $oldLocation = Get-Location
+        Set-Location $sandbox
+        try {
+            $result = cmd.exe /c $scriptCopy "status" 2>&1
+            $LASTEXITCODE | Should Be 0
+            $result | Out-String | Should Match "fake-cli invoked status"
+        }
+        finally {
+            Set-Location $oldLocation
+            $env:PATH = $oldPath
+            $env:SKILLOPT_SLEEP_REPO = $oldRepo
+            $env:CLAUDE_PLUGIN_ROOT = $oldPlugin
+        }
+    }
+
+    It "falls back to importable module" {
+        $sandbox = Join-Path $tempDir "cmd_module_fallback"
+        New-Item -ItemType Directory -Path $sandbox | Out-Null
+        $scriptCopy = Join-Path $sandbox "run-sleep.cmd"
+        Copy-Item $script $scriptCopy
+
+        $oldRepo = $env:SKILLOPT_SLEEP_REPO
+        $oldPlugin = $env:CLAUDE_PLUGIN_ROOT
+
+        $env:SKILLOPT_SLEEP_REPO = $null
+        $env:CLAUDE_PLUGIN_ROOT = $null
+
+        $oldPythonPath = $env:PYTHONPATH
+        $env:PYTHONPATH = Resolve-Path (Join-Path $PSScriptRoot "..")
+
+        $oldLocation = Get-Location
+        Set-Location $sandbox
+        try {
+            $result = cmd.exe /c $scriptCopy "--help" 2>&1
+            $LASTEXITCODE | Should Be 0
+            $result | Out-String | Should Match "skillopt_sleep"
+        }
+        finally {
+            Set-Location $oldLocation
+            $env:SKILLOPT_SLEEP_REPO = $oldRepo
+            $env:CLAUDE_PLUGIN_ROOT = $oldPlugin
+            $env:PYTHONPATH = $oldPythonPath
+        }
+    }
+
+    It "fails when nothing is found" {
+        $sandbox = Join-Path $tempDir "cmd_failure"
+        New-Item -ItemType Directory -Path $sandbox | Out-Null
+        $scriptCopy = Join-Path $sandbox "run-sleep.cmd"
+        Copy-Item $script $scriptCopy
+
+        $oldRepo = $env:SKILLOPT_SLEEP_REPO
+        $oldPlugin = $env:CLAUDE_PLUGIN_ROOT
+        $oldPath = $env:PATH
+
+        $env:SKILLOPT_SLEEP_REPO = $null
+        $env:CLAUDE_PLUGIN_ROOT = $null
+
+        $emptyBin = Join-Path $sandbox "empty_bin"
+        New-Item -ItemType Directory -Path $emptyBin | Out-Null
+        $env:PATH = $emptyBin
+
+        $oldLocation = Get-Location
+        Set-Location $sandbox
+        try {
+            $result = cmd.exe /c $scriptCopy "status" 2>&1
+            $LASTEXITCODE | Should Not Be 0
+            $result | Out-String | Should Match "could not locate the skillopt_sleep package"
+        }
+        finally {
+            Set-Location $oldLocation
+            $env:SKILLOPT_SLEEP_REPO = $oldRepo
+            $env:CLAUDE_PLUGIN_ROOT = $oldPlugin
+            $env:PATH = $oldPath
+        }
+    }
+
+    It "propagates exit code on failure" {
+        $env:SKILLOPT_SLEEP_REPO = Resolve-Path (Join-Path $PSScriptRoot "..")
+        $result = cmd.exe /c $script "non-existent-subcommand" 2>&1
+        $LASTEXITCODE | Should Not Be 0
+    }
+
+    It "handles paths containing spaces correctly" {
+        $spaceDir = Join-Path $tempDir "cmd path with spaces"
+        New-Item -ItemType Directory -Path $spaceDir | Out-Null
+        $scriptCopy = Join-Path $spaceDir "run-sleep.cmd"
+        Copy-Item $script $scriptCopy
+
+        $binDir = Join-Path $spaceDir "bin"
+        New-Item -ItemType Directory -Path $binDir | Out-Null
+        $fakeCli = Join-Path $binDir "skillopt-sleep.cmd"
+        "@echo off`r`necho fake-cli invoked %*`r`nexit /b 0" | Out-File -FilePath $fakeCli -Encoding ascii
+
+        $oldPath = $env:PATH
+        $oldRepo = $env:SKILLOPT_SLEEP_REPO
+        $oldPlugin = $env:CLAUDE_PLUGIN_ROOT
+
+        $env:PATH = "$binDir;$oldPath"
+        $env:SKILLOPT_SLEEP_REPO = $null
+        $env:CLAUDE_PLUGIN_ROOT = $null
+
+        $oldLocation = Get-Location
+        Set-Location $spaceDir
+        try {
+            $result = cmd.exe /c $scriptCopy "status" 2>&1
+            $LASTEXITCODE | Should Be 0
+            $result | Out-String | Should Match "fake-cli invoked status"
+        }
+        finally {
+            Set-Location $oldLocation
+            $env:PATH = $oldPath
+            $env:SKILLOPT_SLEEP_REPO = $oldRepo
+            $env:CLAUDE_PLUGIN_ROOT = $oldPlugin
+        }
+    }
+}
@@ -18,7 +18,10 @@ class TestSchedulerWindows(unittest.TestCase):
                stderr = ""
            return Proc()
            
-        with mock.patch("subprocess.run", side_effect=fake_run):
+        mock_open = mock.mock_open()
+        with mock.patch("subprocess.run", side_effect=fake_run), \
+             mock.patch("os.makedirs") as mock_makedirs, \
+             mock.patch("builtins.open", mock_open):
            ok, msg = schedule("/p/my project", backend="mock", hour=3, minute=17)
            self.assertTrue(ok)
            self.assertIn("via Windows Task Scheduler", msg)
@@ -30,13 +33,21 @@ class TestSchedulerWindows(unittest.TestCase):
            self.assertTrue(cmd[3].startswith("SkillOpt-Sleep-"))
            self.assertIn("my_project", cmd[3])
            self.assertEqual(cmd[4], "/tr")
-            self.assertIn("cmd.exe", cmd[5])
+            self.assertIn("run.cmd", cmd[5])
            self.assertEqual(cmd[6], "/sc")
            self.assertEqual(cmd[7], "daily")
            self.assertEqual(cmd[8], "/st")
            self.assertEqual(cmd[9], "03:17")
            self.assertEqual(cmd[10], "/f")

+            mock_makedirs.assert_called_once()
+            mock_open.assert_called_once()
+            # Verify the content written to the helper script
+            handle = mock_open()
+            written = "".join(call[0][0] for call in handle.write.call_args_list)
+            self.assertIn("@echo off", written)
+            self.assertIn("run --project", written)
+
    @mock.patch("sys.platform", "win32")
    @mock.patch("shutil.which", return_value="C:\\Windows\\System32\\schtasks.exe")
    def test_unschedule_windows(self, mock_which):
@@ -51,7 +62,9 @@ class TestSchedulerWindows(unittest.TestCase):
                stderr = ""
            return Proc()
            
-        with mock.patch("subprocess.run", side_effect=fake_run):
+        with mock.patch("subprocess.run", side_effect=fake_run), \
+             mock.patch("os.path.exists", return_value=True), \
+             mock.patch("os.remove") as mock_remove:
            ok, msg = unschedule("/p/my project")
            self.assertTrue(ok)
            self.assertEqual(len(calls), 1)
@@ -61,3 +74,4 @@ class TestSchedulerWindows(unittest.TestCase):
            self.assertEqual(cmd[2], "/tn")
            self.assertTrue(cmd[3].startswith("SkillOpt-Sleep-"))
            self.assertEqual(cmd[4], "/f")
+            mock_remove.assert_called_once()