fix(worker): identify a staged input the same way on every OS (#2005)

A staged task input's artifact id was built with os.path.join, so a Windows
control plane produced `inputs\<sha256>.wav`. That id is not a local path. It
is persisted into remote_tasks.params_json, shipped to remote workers over
gRPC as the identifier for the input they must fetch, and compared against a
later disk sweep to decide whether a staged file is still referenced.

So a Windows host hands a Linux worker `inputs\abc.wav`, where the backslash is
an ordinary filename character and no such file exists. Remote GPU workers are
a shipped feature; this broke them for every Windows control plane. The same
ids also stop matching when an omnivoice_data/ directory moves between
operating systems.

artifact_id_for() makes it canonical POSIX — resolve_within already treats both
separators as structural, so resolution is unchanged. normalize_artifact_id()
covers the upgrade: rows written by the old code carry a backslash, and the
sweeper decides "unreferenced" by comparing ids, so without it an upgraded
install reads every legacy row as garbage and deletes inputs that surviving
tasks still point at.

Two other tests in this run asserted POSIX-only behaviour rather than product
behaviour, and are corrected here too:

  - the durability-barrier test required a directory fsync, which
    _fsync_parent_directory deliberately skips without os.O_DIRECTORY. It now
    gates on that same attribute rather than on the OS name, so the test and
    the code it checks cannot drift apart.
  - the read-only-cache test built its scenario with chmod(0o500), which on
    Windows only toggles a read-only FILE attribute and does not stop a file
    being created inside the directory. It verifies its premise by probing and
    skips when the host writes anyway — which also covers root and anything
    holding CAP_DAC_OVERRIDE, replacing a geteuid check that named only one of
    them.

Then the reason none of this was visible: CI runs tests/ on Linux only. The two
worker suites join the existing Windows step in the smoke matrix. They need no
ffmpeg, so they cost seconds. Verified green on Windows first — 244 tests
across the four suites in that step.

Fails before, passes after, both directions: a staged id containing a
backslash, and a legacy-id input deleted by the sweeper.

.github/workflows/ci.yml

@@ -459,9 +459,21 @@ jobs:
 
       # Artifact commits depend on native Windows rename/replace semantics;
       # Linux emulation cannot exercise sharing rules or path parsing.
+      # test_worker_task_store and test_worker_inbound_transport joined this
+      # step after a Windows run found a real portability bug the Linux-only
+      # `test` job could not see: a staged input's artifact id was built with
+      # os.path.join, so a Windows control plane persisted and shipped
+      # `inputs\<sha>.wav` — which a Linux worker cannot resolve. These suites
+      # need no ffmpeg, so they cost seconds here.
       - name: Remote-worker artifact paths (Windows)
         if: runner.os == 'Windows' && matrix.backend_supported
-        run: uv run --no-sync pytest tests/test_worker_upload_server.py tests/test_worker_server_integrity.py -q --tb=short
+        run: >-
+          uv run --no-sync pytest
+          tests/test_worker_upload_server.py
+          tests/test_worker_server_integrity.py
+          tests/test_worker_task_store.py
+          tests/test_worker_inbound_transport.py
+          -q --tb=short
         env:
           HF_HUB_OFFLINE: "1"
           HF_HUB_CACHE: ${{ runner.temp }}/worker-artifact-empty-hf-cache

CHANGELOG.md

@@ -95,6 +95,8 @@ the frozen-backend fallback mirror it for their toolchains.
 
 ### Fixed
 
+- Remote GPU workers work when the machine running VoiceStudio is on Windows: a staged input is now identified the same way on every operating system, instead of with a path only Windows can read (#2005)
+
 - The pronunciation list badges an IPA or CMU entry as not applied yet, so you can see it without running a test (#1949) — thanks @utkarsha741!
 
 - A remote-worker test no longer fails at random on Windows CI: it waited for a background thread by spinning the event loop that thread's work needed (#1990)

backend/worker/task_store.py

@@ -134,6 +134,32 @@ INPUT_PARAM_KEYS: tuple[str, ...] = (
 # task records what was staged for it. The record is what makes the purge
 # exact: an input is deletable only when no surviving task still refers to it.
 INPUTS_DIRNAME = "inputs"
+
+
+def artifact_id_for(name: str) -> str:
+    """The id a staged input is known by, everywhere.
+
+    This is a PROTOCOL identifier, not a local path: it is persisted in
+    ``params_json``, handed to remote workers over gRPC, and matched against
+    what a later sweep finds on disk. ``os.path.join`` made it OS-specific, so
+    a Windows control plane stored and shipped ``inputs\\<sha>.wav`` — which a
+    Linux worker cannot resolve, and which stops matching the moment the same
+    data directory is opened on another OS. Always ``/``; ``resolve_within``
+    already treats both separators as structural, so resolution is unaffected.
+    """
+    return f"{INPUTS_DIRNAME}/{name}"
+
+
+def normalize_artifact_id(artifact_id: str) -> str:
+    """Compare ids written by any host on equal terms.
+
+    Rows staged by a Windows control plane before this was canonicalised carry
+    a backslash. The sweeper decides whether a file on disk is still
+    referenced by comparing ids, so without this an upgraded install would
+    read every legacy row as unreferenced and delete inputs that surviving
+    tasks still point at.
+    """
+    return (artifact_id or "").replace("\\", "/")
 INPUTS_PARAM_KEY = "inputs"
 
 _HASH_CHUNK_BYTES = 1024 * 1024
@@ -292,7 +318,7 @@ def stage_input(
             f"Could not read the task input {source!r}: {exc}"
         ) from exc
 
-    artifact_id = os.path.join(INPUTS_DIRNAME, f"{digest}{_extension(source)}")
+    artifact_id = artifact_id_for(f"{digest}{_extension(source)}")
     try:
         destination = resolve_within(base, artifact_id)
     except UnsafePath as exc:  # pragma: no cover — the id is ours, hex only
@@ -473,7 +499,7 @@ def _referenced_artifacts(conn) -> set[str]:
             continue
         for entry in entries:
             if isinstance(entry, dict) and entry.get("artifact_id"):
-                referenced.add(str(entry["artifact_id"]))
+                referenced.add(normalize_artifact_id(str(entry["artifact_id"])))
     return referenced
 
 
@@ -560,8 +586,7 @@ def purge_artifacts(
     except OSError:
         return removed
     for name in names:
-        artifact_id = os.path.join(INPUTS_DIRNAME, name)
-        if artifact_id in referenced:
+        if artifact_id_for(name) in referenced:
             continue
         path = os.path.join(inputs_dir, name)
         try:
@@ -893,6 +918,8 @@ def purge_finished(
 
 __all__ = [
     "INPUTS_DIRNAME",
+    "artifact_id_for",
+    "normalize_artifact_id",
     "INPUTS_PARAM_KEY",
     "INPUT_PARAM_KEYS",
     "InputStagingError",

tests/test_recurrence_hardening.py

@@ -194,11 +194,25 @@ def test_generate_timeout_env_floor_respected(monkeypatch):
 def test_user_env_drops_read_only_path(tmp_path, monkeypatch):
     """An existing directory on a read-only mount passes isdir but fails on
     first real use — validation must probe actual write capability."""
-    if hasattr(os, "geteuid") and os.geteuid() == 0:
-        pytest.skip("root writes anywhere; the probe cannot fail")
     ro = tmp_path / "readonly-cache"
     ro.mkdir()
     ro.chmod(0o500)
+    # Verify the premise instead of assuming it. `chmod(0o500)` makes a
+    # directory unwritable on POSIX; on Windows it only toggles a read-only
+    # FILE attribute and does not stop a file being created inside, so the
+    # scenario cannot be built there at all and the probe correctly reports
+    # the directory as usable. Root (and anything holding CAP_DAC_OVERRIDE)
+    # writes through the mode bits for the same reason. Probing for it covers
+    # every such host, including ones no explicit check would name.
+    try:
+        probe = ro / ".writable-probe"
+        probe.touch()
+        probe.unlink()
+    except OSError:
+        pass  # genuinely unwritable — the test can do its work
+    else:
+        ro.chmod(0o700)
+        pytest.skip("this host writes into a mode-0500 directory; no read-only path to test")
     env_file = tmp_path / "env"
     env_file.write_text(f"OMNIVOICE_CACHE_DIR={ro}\n")
     monkeypatch.delenv("OMNIVOICE_CACHE_DIR", raising=False)

tests/test_worker_inbound_transport.py

@@ -3048,7 +3048,12 @@ async def test_adopted_input_final_is_durable_and_not_swept_as_an_orphan(
     )
     assert final not in store._orphaned_paths
     file_fsync = durability_events.index("file")
-    assert "directory" in durability_events[file_fsync + 1 :]
+    # The directory half of the barrier exists only where the platform has one.
+    # `_fsync_parent_directory` returns immediately without `os.O_DIRECTORY`,
+    # which Windows does not define — so gate on the same condition the product
+    # uses rather than on the OS name, and the two cannot drift apart.
+    if hasattr(os, "O_DIRECTORY"):
+        assert "directory" in durability_events[file_fsync + 1 :]
 
     # Any later write runs the orphan sweep. The adopted final must no longer
     # be a deletion candidate once its durability barrier has succeeded.

tests/test_worker_task_store.py

@@ -7,6 +7,8 @@ while the desktop app is closed. Recovery, not burial.
 """
 from __future__ import annotations
 
+import json
+import os
 import sqlite3
 
 import pytest
@@ -82,6 +84,65 @@ def test_persisted_input_params_do_not_contain_user_home_paths(db, tmp_path, mon
     assert "inputs/" in stored
 
 
+def test_a_staged_input_id_is_posix_on_every_host(tmp_path, monkeypatch):
+    """The artifact id crosses machines, so it cannot carry an OS separator.
+
+    It is persisted in params_json, shipped to remote workers over gRPC, and
+    matched against a later disk sweep. os.path.join made it host-specific: a
+    Windows control plane produced ``inputs\<sha>.wav``, which a Linux worker
+    cannot resolve and which stops matching the moment the same data directory
+    is opened on another OS.
+    """
+    root = tmp_path / "artifacts"
+    (root / task_store.INPUTS_DIRNAME).mkdir(parents=True)
+    monkeypatch.setattr(task_store, "artifact_root", lambda **_kw: str(root))
+    source = tmp_path / "voice.wav"
+    source.write_bytes(b"voice")
+
+    record = task_store.stage_input(str(source), root=str(root))
+
+    assert "\\" not in record["artifact_id"], record["artifact_id"]
+    assert record["artifact_id"].startswith(f"{task_store.INPUTS_DIRNAME}/")
+    # And it still resolves to the file that was actually written.
+    assert (root / record["artifact_id"]).is_file()
+
+
+def test_a_legacy_windows_id_still_protects_its_input(db, tmp_path, monkeypatch):
+    """An upgraded install must not delete inputs its tasks still point at.
+
+    Rows staged before the id was canonicalised carry a backslash. The sweeper
+    decides "unreferenced" by comparing ids, so matching a legacy row against a
+    freshly built posix id would read every one of them as garbage and delete
+    the file a surviving task depends on.
+    """
+    root = tmp_path / "artifacts"
+    (root / task_store.INPUTS_DIRNAME).mkdir(parents=True)
+    monkeypatch.setattr(task_store, "artifact_root", lambda **_kw: str(root))
+    source = tmp_path / "voice.wav"
+    source.write_bytes(b"voice")
+    record = task_store.stage_input(str(source), root=str(root))
+    staged = root / record["artifact_id"]
+    os.utime(staged, (0, 0))  # older than any cutoff
+
+    # The row exactly as a pre-fix Windows control plane wrote it. Inserted
+    # directly: `create` validates against today's rules, and the point is a
+    # row that predates them.
+    legacy = dict(record, artifact_id=record["artifact_id"].replace("/", "\\"))
+    with sqlite3.connect(db) as conn:
+        conn.execute(
+            "INSERT INTO remote_tasks"
+            " (id, operation, params_json, state, created_at, updated_at)"
+            " VALUES (?, 'tts', ?, 'queued', 1000.0, 1000.0)",
+            ("legacy-1", json.dumps({task_store.INPUTS_PARAM_KEY: [legacy]})),
+        )
+
+    with task_store.db_conn() as conn:
+        referenced = task_store._referenced_artifacts(conn)
+    task_store.purge_artifacts((), referenced, cutoff=1e12, root=str(root))
+
+    assert staged.is_file(), "a legacy-id input that a task still references was deleted"
+
+
 def test_attempts_round_trip(db):
     task = _task()
     task_store.create(task, now=1000.0)