ci: run the test suite on Windows as well as Linux

.github/workflows/ci.yml

@@ -7,7 +7,8 @@
 # Jobs:
 # - lint: Runs ruff linter and formatter check
 # - typecheck: Runs ty (Red Knot) type checker
-# - test: Runs pytest with coverage on Python 3.12 and 3.13
+# - test: Runs pytest with coverage on Python 3.12 + 3.13 (Ubuntu) and 3.13
+#         (Windows, where the platform-specific code actually lives)
 # - validate-examples: Validates all example workflow YAML files
 # - build: Verifies the package builds correctly
 # - frontend: Type-checks, tests (Vitest), and builds the web dashboard
@@ -81,12 +82,27 @@ jobs:
         run: uv run ty check src
 
   test:
-    name: Test (Python ${{ matrix.python-version }})
-    runs-on: ubuntu-latest
+    name: Test (Python ${{ matrix.python-version }}, ${{ matrix.os }})
+    runs-on: ${{ matrix.os }}
     needs: [lint, typecheck]
     strategy:
       matrix:
+        os: [ubuntu-latest]
         python-version: ["3.12", "3.13"]
+        # Windows carries a substantial amount of platform-specific code —
+        # cli/pid.py is an entire OpenProcess/GetExitCodeProcess ctypes
+        # implementation, cli/bg_runner.py handles process groups and job-object
+        # breakaway — and none of it was executed anywhere. The existing tests
+        # cover that logic on Linux by monkey-patching a `_kernel32` mock, which
+        # is good design but means nothing validates the real ctypes signatures.
+        #
+        # Deliberately ONE Python version on Windows rather than a full 2x2: the
+        # platform is the variable here, not the interpreter, and the
+        # install-scripts job already provisions a Windows runner on every PR so
+        # this is incremental cost rather than a new platform commitment.
+        include:
+          - os: windows-latest
+            python-version: "3.13"
       fail-fast: false
 
     steps:
@@ -109,11 +125,33 @@ jobs:
         run: uv sync --group dev --extra claude-agent-sdk
 
       - name: Remove bundled Copilot CLI binary
+        # `shell: bash` so this runs identically on the Windows runner (Git Bash
+        # is preinstalled). Without it the step fails on Windows, where a bare
+        # `find` resolves to the unrelated DOS search tool in System32.
+        shell: bash
         run: |
+          set -euo pipefail
           # The github-copilot-sdk >=0.1.23 bundles a CLI binary that tries to
           # authenticate with GitHub on startup. Remove it so tests that invoke
           # the real CLI path fail fast instead of hanging on auth.
-          find .venv -path '*/copilot/bin/copilot*' -delete 2>/dev/null || true
+          #
+          # The binary ships inside the package (copilot/bin/), not in the venv's
+          # scripts directory — there is no console-script entry point — so one
+          # pattern covers both platforms: the leading */ absorbs lib/pythonX.Y/
+          # or Lib/, and the trailing * absorbs the .exe suffix on Windows.
+          #
+          # Checked rather than best-effort: `find` exits 0 when it matches
+          # nothing, so a silent no-op reinstates the auth hang this step exists
+          # to prevent, and it surfaces ten minutes later as a bare job timeout
+          # naming neither the CLI nor this step.
+          removed=$(find .venv -path '*/copilot/bin/copilot*' -print -delete)
+          if [ -z "$removed" ]; then
+            echo "::error::No bundled Copilot CLI matched '*/copilot/bin/copilot*'."
+            echo "The github-copilot-sdk layout changed — update this pattern, or"
+            echo "tests taking the real CLI path will hang until the job timeout."
+            exit 1
+          fi
+          echo "Removed: $removed"
 
       - name: Run tests with coverage
         timeout-minutes: 10
@@ -128,7 +166,9 @@ jobs:
 
       - name: Upload coverage reports
         uses: codecov/codecov-action@v4
-        if: matrix.python-version == '3.12'
+        # One upload per commit. Without the os guard a second runner would
+        # upload a partial report for the same commit and skew the totals.
+        if: matrix.python-version == '3.12' && matrix.os == 'ubuntu-latest'
         with:
           file: ./coverage.xml
           fail_ci_if_error: false

CHANGELOG.md

@@ -183,6 +183,49 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Plugin checkouts from a `file://` source no longer land outside the plugin
+  cache on Windows.** The cache key is derived from the URL's path segments, but
+  the splitter only knew `/`, so a native Windows path arrived as a single
+  segment with its backslashes intact — and the key kept them, putting the
+  checkout at a drive-absolute location rather than under the cache root, which
+  is the same escape the `..` check exists to prevent. Two further problems sat
+  behind it: a drive colon made an owner of `C:_src` read as a drive (or, in the
+  middle of a name, as an NTFS alternate data stream), and flattening a deep
+  path into one segment produced a directory name long enough that `git` refused
+  to create `.git` inside it. Separators are now folded, the characters that
+  change a path's meaning on Windows are substituted, and an over-long segment
+  is replaced by a digest of itself — on every platform, so one workflow file
+  resolves to the same cache layout wherever it runs.
+- **Two sources resolving to the same commit no longer fail the whole fetch on
+  Windows.** Publishing a completed checkout tolerates losing the race to a
+  concurrent fetch, but recognised only the POSIX errnos for "destination
+  already exists"; Windows reports that as `ERROR_ACCESS_DENIED`, so the
+  tolerance never applied and the second source raised. Safe to accept because
+  the readiness sentinel is written after publishing: a winner that died
+  mid-clone leaves no sentinel, so the tree is re-fetched rather than read
+  half-written.
+- **A local path is recognised the same way on every platform** — `_is_local_path`
+  asked `pathlib.Path`, which is the *running* platform's flavour, so a POSIX
+  absolute path such as `/srv/plugins` was refused as an unrecognised source on
+  Windows. Both conventions are now consulted.
+- **Registry names are validated before they can corrupt the config** — a name
+  containing a quote, a space, `=` or `#` was accepted, written into
+  `registries.toml` as an unescaped table key, and then failed to parse. Since
+  `registry add`, `remove` and `get` all load the config first, the user could
+  not remove the entry that broke it and every unrelated registry went down
+  with it. Names are now restricted to letters, digits, `.`, `_` and `-`, which
+  also keeps them legal as cache directory names on Windows, and the table key
+  is quoted so a dotted name stays one registry instead of becoming a nested
+  table.
+- **`conductor doctor` no longer reports a missing Claude CLI on Windows** —
+  the CLI probe dropped five `~`-anchored fallback locations on Windows,
+  including `~/.claude/local/claude` where Claude Code's own installer puts it,
+  so `validate_connection()` returned False for a CLI the SDK would find and
+  run. Only `/usr/local/bin/claude` is now skipped there: it is rooted but
+  driveless, so it resolves against the current drive, which any unprivileged
+  local user can write to.
+- **Registry TOML values are escaped** — a registry whose source or type
+  contained a quote or a backslash produced a file that could not be re-read.
 - **Dashboard context-window bar no longer reports cumulative input tokens as
   a false red at >100% of the cap** (#412). The bar reused
   `AgentOutput.input_tokens` — a *billing* total summed across every API call

src/conductor/plugins/fetch.py

@@ -60,6 +60,7 @@
 import re
 import shutil
 import subprocess
+import sys
 import tempfile
 import time
 from collections.abc import Callable, Sequence
@@ -235,9 +236,20 @@ def _run_git(arguments: Sequence[str], *, timeout: int, context: str) -> str:
         "SSH_ASKPASS": "",
         "GCM_INTERACTIVE": "never",
     }
+    # ``core.longpaths`` is git-for-Windows' opt-in to the Win32 extended-length
+    # API; without it git refuses any path over 260 characters. The plugin cache
+    # is inherently deep -- cache root, then host, owner, repo-digest, a staging
+    # directory, and then whatever the repository itself nests -- so a user whose
+    # cache lives under a long home directory could not clone at all, failing on
+    # git's own `.git/hooks/*.sample` before any project file was written.
+    #
+    # Set per-invocation rather than asking users to configure it globally: this
+    # is a property of the paths Conductor generates, not a preference of theirs.
+    # A no-op on POSIX, where the limit does not exist.
+    git_config = ["-c", "protocol.ext.allow=never", "-c", "core.longpaths=true"]
     try:
         completed = subprocess.run(  # noqa: S603
-            ["git", "-c", "protocol.ext.allow=never", *arguments],  # noqa: S607
+            ["git", *git_config, *arguments],  # noqa: S607
             capture_output=True,
             text=True,
             timeout=timeout,
@@ -483,7 +495,19 @@ def _publish(temporary: Path, destination: Path) -> None:
         # Only the lost-race errnos. Treating EACCES or ENOSPC as "someone
         # else got there first" would report a broken checkout as a
         # successful one, on the strength of the directory merely existing.
-        if exc.errno in (errno.ENOTEMPTY, errno.EEXIST) and destination.is_dir():
+        #
+        # Windows is the exception, and needs naming rather than adding
+        # EACCES globally: replacing a directory that already exists raises
+        # ERROR_ACCESS_DENIED (WinError 5, surfaced as EACCES) instead of
+        # ENOTEMPTY, so the POSIX-only list never fired and a second source
+        # resolving to the same SHA failed the whole fetch. Safe because the
+        # readiness sentinel is written *after* this returns: a winner that
+        # died mid-clone leaves no sentinel, `is_cached` reports a miss, and
+        # the tree is re-fetched rather than read half-written.
+        lost_race = exc.errno in (errno.ENOTEMPTY, errno.EEXIST) or (
+            sys.platform == "win32" and getattr(exc, "winerror", None) == 5
+        )
+        if lost_race and destination.is_dir():
             shutil.rmtree(temporary, ignore_errors=True)
             return
         raise

src/conductor/plugins/sources.py

@@ -34,7 +34,7 @@
 import hashlib
 import re
 from dataclasses import dataclass
-from pathlib import Path, PurePosixPath
+from pathlib import Path, PurePosixPath, PureWindowsPath
 
 from conductor.plugins.errors import PluginSourceError
 
@@ -77,6 +77,28 @@
 # with a real host. Not a valid DNS name, deliberately.
 _LOCAL_HOST = "_local"
 
+# Characters that are legal in a URL path segment but change what a path
+# *means* on Windows, so a cache key built from them would not name the
+# directory it appears to. ``:`` is the one that actually occurs: a
+# ``file://C:/src/repo`` source derives the owner ``C:_src``, and Windows
+# reads ``C:`` as a drive (or, mid-component, as an NTFS alternate data
+# stream) rather than as part of the name. The rest are included because
+# they fail the same way and cost nothing to cover.
+#
+# Substituted rather than refused, unlike ``..``: a Windows path is a
+# legitimate source, so refusing it would make ``file://`` unusable there.
+# Substituted on *every* platform so one workflow file resolves to the same
+# cache layout everywhere.
+_PATH_UNSAFE = str.maketrans(dict.fromkeys(':<>"|?*', "_"))
+
+# Bound on one cache-key segment. Windows caps a path at 260 characters
+# unless long-path support is enabled, and the segment is only one part of a
+# path that also carries the cache root, the host, the leaf, a staging
+# directory, and whatever the repository itself nests. Sized against the
+# worst case seen in CI -- a pytest temporary root, which at 48 came to 264
+# characters and at 24 comes to 240.
+_MAX_SEGMENT = 24
+
 
 def redact_credentials(text: str) -> str:
     """Remove any URL-embedded credential from ``text``.
@@ -243,10 +265,14 @@ def _is_local_path(location: str) -> bool:
     """
     if location.startswith(("~", ".")):
         return True
-    # Absolute on either platform. A bare Windows drive root ("C:\\") is
-    # caught by ntpath's isabs via PureWindowsPath, which PurePosixPath
-    # would call relative.
-    return Path(location).is_absolute() or bool(re.match(r"\A[A-Za-z]:[\\/]", location))
+    # Absolute in *either* convention, deliberately independent of the host
+    # OS. A plugin source is a string in a workflow file, so the same file
+    # must classify it the same way everywhere -- ``Path`` is the running
+    # platform's flavour, so on Windows it called "/srv/p" relative and the
+    # source was refused as unrecognised. ``PureWindowsPath`` also covers a
+    # bare drive root ("C:\\") and UNC paths, which the posix flavour calls
+    # relative; a drive-relative "C:" is absolute in neither, correctly.
+    return PurePosixPath(location).is_absolute() or PureWindowsPath(location).is_absolute()
 
 
 def _strip_git_suffix(name: str) -> str:
@@ -288,7 +314,38 @@ def _key_from_parts(raw: str, host: str, path: str) -> tuple[str, str, str]:
             f"Source {raw!r} contains a '.' or '..' path component, which would "
             "escape the plugin cache directory."
         )
-    return resolved
+    # Substitution happens after the '..' check, so a hostile segment is
+    # still refused rather than quietly renamed into a harmless one.
+    return tuple(_safe_segment(segment) for segment in resolved)  # type: ignore[return-value]
+
+
+def _safe_segment(segment: str) -> str:
+    """Make one cache-key segment safe to use as a directory name anywhere.
+
+    Two problems, both only reachable via a local ``file://`` source, and
+    both of which put the checkout somewhere other than where the key says.
+
+    Characters are substituted because ``:`` changes what a path *means* on
+    Windows: an owner of ``C:_src`` reads as a drive, or mid-component as an
+    NTFS alternate data stream. The rest of the set fails the same way.
+
+    Length is bounded because a local source flattens its whole directory
+    path into the owner segment, and Windows still caps a path at 260
+    characters by default -- a source under a deep directory produced a name
+    long enough that ``git`` refused to create ``.git`` inside it. Replacing
+    an over-long segment with a digest of itself costs nothing: the cache
+    key's leaf already carries a digest of the full location, so the owner
+    disambiguates nothing on its own.
+
+    The *tail* is kept rather than the head. What survives is then the
+    directory nearest the repository, which is the part someone browsing the
+    cache can recognise; the head is a drive letter and ``Users``.
+    """
+    segment = segment.translate(_PATH_UNSAFE)
+    if len(segment) > _MAX_SEGMENT:
+        digest = hashlib.sha256(segment.encode("utf-8")).hexdigest()[:12]
+        return f"{segment[-(_MAX_SEGMENT - len(digest) - 1) :]}-{digest}"
+    return segment
 
 
 def _parse_url(raw: str, location: str, ref: str | None) -> PluginSource:
@@ -298,7 +355,14 @@ def _parse_url(raw: str, location: str, ref: str | None) -> PluginSource:
         # A file:// URL has no host worth keying on, and its path is
         # already absolute. Treated as a remote (git can clone it) but
         # keyed under the local host so it cannot collide with a forge.
-        host, owner, repo = _key_from_parts(raw, _LOCAL_HOST, remainder)
+        #
+        # Backslashes are folded to '/' first: this is the one URL form
+        # that carries a native Windows path, and the splitter below only
+        # knows '/'. Without this the whole of "C:\src\repo" arrives as a
+        # single segment, so the cache key kept its separators and the
+        # checkout landed at a drive-absolute path outside the plugin
+        # cache entirely -- the same escape the '..' check exists to stop.
+        host, owner, repo = _key_from_parts(raw, _LOCAL_HOST, remainder.replace("\\", "/"))
         return PluginSource(raw=raw, location=location, ref=ref, host=host, owner=owner, repo=repo)
 
     authority, _, path = remainder.partition("/")

src/conductor/providers/claude_agent_sdk.py

@@ -7,6 +7,7 @@
 import json
 import logging
 import os
+import sys
 import tempfile
 import time
 from pathlib import Path
@@ -965,12 +966,17 @@ async def validate_connection(self) -> bool:
         import shutil
         from pathlib import Path
 
-        # Bundled CLI takes precedence (matches the SDK's own resolution).
+        is_windows = sys.platform == "win32"
+
+        # Bundled CLI takes precedence (matches the SDK's own resolution). The SDK names
+        # the bundled binary per-platform — see _find_bundled_cli — so probing only
+        # "claude" reports "no CLI" on Windows even when the bundled one is present.
         try:
             import claude_agent_sdk  # ty: ignore[unresolved-import]
 
             sdk_dir = Path(claude_agent_sdk.__file__).parent
-            for candidate in (sdk_dir / "_bundled" / "claude",):
+            bundled_name = "claude.exe" if is_windows else "claude"
+            for candidate in (sdk_dir / "_bundled" / bundled_name,):
                 if candidate.exists() and candidate.is_file():
                     return True
         except Exception:
@@ -981,14 +987,30 @@ async def validate_connection(self) -> bool:
 
         # SDK's hardcoded fallback locations — keep in sync with
         # claude_agent_sdk._internal.transport.subprocess_cli._find_cli.
-        for path in (
+        #
+        # Audited against claude-agent-sdk 0.2.87, the version uv.lock pins. That
+        # version has *no* platform branch in _find_cli: it probes all six of these
+        # on every OS, Windows included. So this narrows conductor's *report* only —
+        # the SDK will still spawn a planted binary even when this returns False.
+        # Later SDKs (>= 0.2.13x) refuse the driveless entry; this matches that
+        # behaviour ahead of the pin.
+        #
+        # Only "/usr/local/bin/claude" is driveless, so only it is dropped on
+        # Windows: a rooted but driveless path resolves against the current drive
+        # (C:\usr\local\bin\claude), which any unprivileged local user can create.
+        # The other five are Path.home()-anchored and carry no such risk — dropping
+        # those would report "no CLI" for a Windows user whose CLI sits at
+        # ~/.claude/local/claude, where Claude Code's own local installer puts it,
+        # and the SDK would find and run it.
+        fallbacks: tuple[Path, ...] = (
             Path.home() / ".npm-global/bin/claude",
-            Path("/usr/local/bin/claude"),
+            *(() if is_windows else (Path("/usr/local/bin/claude"),)),
             Path.home() / ".local/bin/claude",
             Path.home() / "node_modules/.bin/claude",
             Path.home() / ".yarn/bin/claude",
             Path.home() / ".claude/local/claude",
-        ):
+        )
+        for path in fallbacks:
             if path.exists() and path.is_file():
                 return True