fix(ingestion): ingest non-existent absolute-looking strings as text [SDK-234] (gh #3887, follow-up to #3892) (#4155)

## Description

Follow-up to #3892 (now merged to `dev`), closing the remaining half of
#3887.

#3892 stops the Windows `ValueError` crash by guarding the absolute-path
branch of `save_data_item_to_storage` with `is_absolute()`. This PR
fixes the other half of the report: a `/`-prefixed string that is
**not** an actual file (e.g. a text note like `/remember to call Bob
about the meeting`) should be ingested as text, not turned into a
`file://` URI pointing at a file that does not exist.

The absolute-path branch converted unconditionally, whereas the
relative-path branch right below it already required the file to exist
(`abs_path.is_file()`). I made the two consistent by reusing that same,
already-computed `abs_path`: an absolute-looking string is only turned
into a `file://` URI when it points at an existing file — otherwise it
falls through to text ingestion, on every platform. The
`ACCEPT_LOCAL_FILE_PATH=false` rejection of *existing* local files is
unchanged.

While tracing the callers I found the dry-run estimator
(`cognee/modules/cognify/estimator.py`) deliberately mirrors this
routing, and it was raising "file does not exist" for missing absolute
paths. Since a real run now treats those as text, I updated
`_path_candidate` to match (missing path → raw text; the gate rejects
only *existing* absolute files) and adjusted its two tests.

Behavior note: because the branch now keys on "existing file"
(`is_file()`), a handful of absolute-looking strings that are **not**
regular files also route to text instead of a `file://` URI — an
existing *directory* path, a broken symlink, and pathological paths that
error during `resolve()` (embedded null / over-length). This is intended
and safe: none of those are ingestible files, and directories are
normally expanded upstream by `resolve_data_directories` before reaching
here. It also repairs a latent downstream break — before this change a
`/`-prefixed note produced a broken `file://` URI that
`get_data_file_path` turned into a non-existent path and the loader then
failed on.

> #3892 has merged to `dev`; this PR is rebased on top of it and now
contains only the follow-up commits.

## Acceptance Criteria

* `save_data_item_to_storage("/remember to call Bob about the meeting")`
is ingested as text — no `file://` URI, no crash — on Windows, macOS and
Linux.
* An existing absolute file path still converts to a `file://` URI; with
`ACCEPT_LOCAL_FILE_PATH=false` it still raises `IngestionError`.
* The dry-run estimate prices a missing absolute path as text instead of
erroring out.
* The regression tests run on the regular (Linux) unit-test CI, not only
the Windows OS matrix. Verified locally: 38 passed, 1 skipped (the
genuine drive-anchored Windows case); `ruff format` + `ruff check`
clean.

## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Code refactoring
- [ ] Other (please specify):

## Pre-submission Checklist
- [x] **I have tested my changes thoroughly before submitting this PR**
(See `CONTRIBUTING.md`)
- [x] **This PR contains minimal changes necessary to address the
issue/feature**
- [x] My code follows the project's coding standards and style
guidelines
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added necessary documentation (if applicable)
- [x] All new and existing tests pass
- [x] I have searched existing PRs to ensure this change hasn't been
submitted already
- [x] I have linked any relevant issues in the description
- [x] My commits have clear and descriptive messages

## DCO Affirmation
I affirm that all code in every commit of this pull request conforms to
the terms of the Topoteretes Developer Certificate of Origin.

Author: Vasilije1990

cognee/modules/cognify/estimator.py

@@ -335,12 +335,14 @@ def _accept_local_file_path() -> bool:
 def _path_candidate(value: str) -> Optional[Path]:
     """The local path this string refers to, or None when it is raw text.
 
-    Mirrors ``save_data_item_to_storage``: remote URLs and missing absolute
-    paths are loud errors, ``file://`` URIs resolve to local paths, everything
-    else that is not an existing file is raw text. Scheme is checked before the
-    newline guard so a trailing-newline URL cannot slip through as text. When
-    ``ACCEPT_LOCAL_FILE_PATH`` is disabled, path references are rejected and
-    relative strings are raw text, exactly as in a real run.
+    Mirrors ``save_data_item_to_storage``: remote URLs are loud errors and
+    ``file://`` URIs resolve to local paths, but a bare string is a file only
+    when it points at one that exists. An absolute-looking string that is not an
+    existing file (e.g. a "/"-prefixed text note) is raw text, exactly as a real
+    run now ingests it. Scheme is checked before the newline guard so a
+    trailing-newline URL cannot slip through as text. When
+    ``ACCEPT_LOCAL_FILE_PATH`` is disabled, an existing *absolute* file path is
+    rejected while relative paths and non-existent strings stay raw text.
     """
     scheme = urlparse(value).scheme.lower()
     if scheme in _UNSUPPORTED_SCHEMES:
@@ -356,20 +358,24 @@ def _path_candidate(value: str) -> Optional[Path]:
     if "\n" in value or "\r" in value or len(value) > 4096:
         return None
 
-    if not _accept_local_file_path():
-        if value.startswith("/"):
-            raise ValueError(f"Local files are not accepted, got {value!r}.")
-        return None
-
     try:
         path = Path(value)
-        if path.exists():
-            return path
+        exists = path.exists()
     except (OSError, ValueError):
         return None
-    if value.startswith("/"):
-        # A real run treats absolute paths as file references and would fail too.
-        raise ValueError(f"dry_run file input does not exist: {value!r}.")
+
+    if exists:
+        if not _accept_local_file_path():
+            # Mirror the ACCEPT_LOCAL_FILE_PATH gate: an existing *absolute* file
+            # path is rejected, while an existing *relative* path falls through to
+            # raw text (save_data_item_to_storage has no reject branch for it).
+            if value.startswith("/"):
+                raise ValueError(f"Local files are not accepted, got {value!r}.")
+            return None
+        return path
+
+    # A non-existent path — absolute or relative — is raw text. A real run no
+    # longer treats a "/"-prefixed string as a file reference unless it exists.
     return None
 
 

cognee/tasks/ingestion/save_data_item_to_storage.py

@@ -72,16 +72,27 @@ async def save_data_item_to_storage(data_item: Union[BinaryIO, str, Any]) -> str
             else:
                 raise IngestionError(message="Local files are not accepted.")
 
-        # data is an absolute file path
+        # data is an absolute file path that points at an existing file
         elif (
-            data_item.startswith("/")
-            or (os.name == "nt" and len(data_item) > 1 and data_item[1] == ":")
-        ) and Path(os.path.normpath(data_item)).is_absolute():
+            (
+                data_item.startswith("/")
+                or (os.name == "nt" and len(data_item) > 1 and data_item[1] == ":")
+            )
+            and Path(os.path.normpath(data_item)).is_absolute()
+            and abs_path.is_file()
+        ):
             # Handle both Unix absolute paths (/path) and Windows absolute paths (C:\path).
-            # The is_absolute() guard matters on Windows: a POSIX-style "/path" (or a
-            # drive-relative "C:path") normalizes to a *drive-relative* WindowsPath, and
-            # Path.as_uri() raises ValueError for it. Such strings are not usable file
-            # paths on this platform and continue to the relative-path/text handling below.
+            #
+            # is_absolute() guard: on Windows a POSIX-style "/path" (or a drive-relative
+            # "C:path") normalizes to a *drive-relative* WindowsPath, and Path.as_uri()
+            # raises ValueError for it. Such strings are not usable file paths on this
+            # platform and continue to the relative-path/text handling below.
+            #
+            # abs_path.is_file() guard: only convert an absolute-looking string to a
+            # file:// URI when it actually points at an existing file, mirroring the
+            # relative-path branch below. A "/"-prefixed string that is not an existing
+            # file (e.g. a plain text note such as "/remember to call Bob") falls through
+            # to text ingestion on every platform instead of becoming a broken file:// URI.
             if settings.accept_local_file_path:
                 # Normalize path separators before creating file URL
                 normalized_path = os.path.normpath(data_item)

cognee/tests/unit/modules/cognify/test_estimator.py

@@ -240,9 +240,10 @@ async def test_directories_are_rejected(tmp_path):
 
 
 @pytest.mark.asyncio
-async def test_missing_absolute_path_is_rejected():
-    with pytest.raises(ValueError, match="does not exist"):
-        await estimator._input_to_texts("/no/such/file.txt")
+async def test_missing_absolute_path_is_raw_text():
+    # A "/"-prefixed string that is not an existing file is ingested as text by
+    # a real run (see #3887), so dry_run must price it as text, not reject it.
+    assert await estimator._input_to_texts("/no/such/file.txt") == ["/no/such/file.txt"]
 
 
 @pytest.mark.asyncio
@@ -254,10 +255,15 @@ async def test_local_paths_are_rejected_when_gate_disabled(tmp_path, monkeypatch
     file_path = tmp_path / "notes.txt"
     file_path.write_text("stored text")
 
+    # An existing local file is rejected, whether named by file:// URI or by an
+    # absolute path — the gate must reject exactly what a real run rejects.
     with pytest.raises(ValueError, match="not accepted"):
         await estimator._input_to_texts(file_path.as_uri())
     with pytest.raises(ValueError, match="not accepted"):
-        await estimator._input_to_texts("/no/such/file.txt")
+        await estimator._input_to_texts(str(file_path))
+    # A non-existent absolute path is raw text even with the gate off: a real run
+    # saves it as text because it is not an existing local file (see #3887).
+    assert await estimator._input_to_texts("/no/such/file.txt") == ["/no/such/file.txt"]
     # A real run treats a relative path to an existing file as raw text when
     # the gate is off.
     monkeypatch.chdir(tmp_path)

cognee/tests/unit/tasks/ingestion/test_save_data_item_to_storage.py

@@ -1,13 +1,22 @@
+import importlib
+import ntpath
 import os
-from unittest.mock import AsyncMock, patch
+from types import SimpleNamespace
+from unittest.mock import AsyncMock
 
 import pytest
 
+from cognee.modules.ingestion.exceptions import IngestionError
 from cognee.tasks.ingestion.save_data_item_to_storage import save_data_item_to_storage
 
+# The package __init__ rebinds the name ``save_data_item_to_storage`` to the
+# function, so ``import ... as mod`` would yield the function, not the module.
+# import_module returns the real module object (for patching its globals).
+mod = importlib.import_module("cognee.tasks.ingestion.save_data_item_to_storage")
+
 
 @pytest.mark.asyncio
-async def test_existing_absolute_path_returns_file_uri(tmp_path):
+async def test_existing_absolute_file_returns_file_uri(tmp_path):
     file_path = tmp_path / "note.txt"
     file_path.write_text("hello", encoding="utf-8")
 
@@ -17,47 +26,108 @@ async def test_existing_absolute_path_returns_file_uri(tmp_path):
 
 
 @pytest.mark.asyncio
-@pytest.mark.skipif(os.name == "nt", reason="POSIX absolute-path semantics")
-async def test_posix_absolute_path_behavior_unchanged_on_posix():
-    # On POSIX, "/"-prefixed strings are genuine absolute paths and convert to
-    # file URIs whether or not the file exists (pre-existing behavior).
-    result = await save_data_item_to_storage("/nonexistent/path/file.txt")
+async def test_nonexistent_absolute_path_is_ingested_as_text(monkeypatch):
+    """A "/"-prefixed string that is not an existing file is ingested as text.
+
+    Regression test for #3887. On every platform, a plain text note that happens
+    to start with "/" (e.g. slash-command-style content) must be saved as text
+    rather than turned into a file:// URI for a file that does not exist. On
+    Windows the pre-fix code additionally crashed here with
+    ``ValueError: relative path can't be expressed as a file URI``.
+    """
+    save_mock = AsyncMock(return_value="text-file-path")
+    monkeypatch.setattr(mod, "save_data_to_file", save_mock)
+
+    note = "/remember to call Bob about the meeting"
+    result = await save_data_item_to_storage(note)
 
-    assert result == "file:///nonexistent/path/file.txt"
+    assert result == "text-file-path"
+    save_mock.assert_awaited_once_with(note)
 
 
 @pytest.mark.asyncio
-@pytest.mark.skipif(os.name != "nt", reason="Windows drive-relative path semantics")
-async def test_posix_style_string_falls_back_to_text_on_windows():
-    """A "/"-prefixed string that is not a usable path on Windows is ingested as text.
-
-    Regression test: os.path.normpath("/x") produces a drive-relative
-    WindowsPath, and Path.as_uri() raised ValueError ("relative paths can't
-    be expressed as file URIs") out of add() for any string starting with
-    "/" — both POSIX-style paths and plain text.
+async def test_windows_style_paths_do_not_crash_and_fall_back_to_text(monkeypatch):
+    """Windows-style path normalization, simulated on any OS.
+
+    ``ntpath.normpath`` turns a "/"-prefixed or drive-relative string into a
+    backslash path, which ``pathlib`` treats as *relative* on POSIX exactly as
+    ``WindowsPath`` does on Windows (``is_absolute()`` is False, ``as_uri()``
+    raises). Combined with the existence guard, such inputs — which do not name
+    an existing file — are ingested as text on every platform instead of raising
+    the Windows ``ValueError: relative path can't be expressed as a file URI``
+    that #3887 reported.
+
+    We replace the module's ``os`` *reference* (not the shared ``os`` singleton),
+    so ``os.name``/``os.path.normpath`` behave like Windows inside the function
+    while ``pathlib`` keeps using the real platform for ``Path``/``Path.cwd()``.
+    The genuine drive-anchored Windows path (``C:\\...``) that points at an
+    existing file is covered by the Windows-only test below and the OS-matrix CI.
     """
-    with patch(
-        "cognee.tasks.ingestion.save_data_item_to_storage.save_data_to_file",
-        new_callable=AsyncMock,
-        return_value="text-file-path",
-    ) as mock_save:
-        result = await save_data_item_to_storage("/remember to call Bob about the meeting")
+    fake_os = SimpleNamespace(name="nt", path=SimpleNamespace(normpath=ntpath.normpath))
+    save_mock = AsyncMock(return_value="text-file-path")
+    monkeypatch.setattr(mod, "save_data_to_file", save_mock)
+    monkeypatch.setattr(mod, "os", fake_os)
+
+    for item in ["/no/such/windows/path.txt", "C:name.txt"]:
+        save_mock.reset_mock()
+        result = await save_data_item_to_storage(item)
+        assert result == "text-file-path"
+        save_mock.assert_awaited_once_with(item)
+
+
+@pytest.mark.asyncio
+async def test_existing_absolute_file_rejected_when_gate_disabled(tmp_path, monkeypatch):
+    # An existing absolute local file is still rejected when
+    # ACCEPT_LOCAL_FILE_PATH is off — the fix must not weaken that gate.
+    file_path = tmp_path / "note.txt"
+    file_path.write_text("hello", encoding="utf-8")
+    monkeypatch.setattr(mod.settings, "accept_local_file_path", False)
+
+    with pytest.raises(IngestionError, match="Local files are not accepted"):
+        await save_data_item_to_storage(str(file_path))
+
+
+@pytest.mark.asyncio
+async def test_nonexistent_absolute_path_is_text_even_when_gate_disabled(monkeypatch):
+    # The ACCEPT_LOCAL_FILE_PATH gate rejects existing *local files*; a
+    # non-existent absolute-looking string is not a local file, so it is still
+    # ingested as text even with the gate off (the reject branch is only reached
+    # once is_file() is true). Mirrors the estimator's gate-off behavior.
+    save_mock = AsyncMock(return_value="text-file-path")
+    monkeypatch.setattr(mod, "save_data_to_file", save_mock)
+    monkeypatch.setattr(mod.settings, "accept_local_file_path", False)
+
+    result = await save_data_item_to_storage("/no/such/file.txt")
 
     assert result == "text-file-path"
-    mock_save.assert_awaited_once_with("/remember to call Bob about the meeting")
+    save_mock.assert_awaited_once_with("/no/such/file.txt")
 
 
 @pytest.mark.asyncio
-@pytest.mark.skipif(os.name != "nt", reason="Windows drive-relative path semantics")
-async def test_drive_relative_string_falls_back_to_text_on_windows():
-    # "C:name.txt" matches the Windows-path arm but is drive-relative, not
-    # absolute; it used to raise the same ValueError from Path.as_uri().
-    with patch(
-        "cognee.tasks.ingestion.save_data_item_to_storage.save_data_to_file",
-        new_callable=AsyncMock,
-        return_value="text-file-path",
-    ) as mock_save:
-        result = await save_data_item_to_storage("C:drive-relative-note.txt")
+async def test_existing_directory_is_ingested_as_text(tmp_path, monkeypatch):
+    # An existing directory is not a file (is_file() is False), so it falls
+    # through to text ingestion rather than becoming a file:// URI. (In the
+    # normal add() flow directories are expanded upstream by
+    # resolve_data_directories.) The dry-run estimator deliberately diverges
+    # here — it rejects directories with a clearer message; see
+    # test_estimator.test_directories_are_rejected.
+    save_mock = AsyncMock(return_value="text-file-path")
+    monkeypatch.setattr(mod, "save_data_to_file", save_mock)
+
+    result = await save_data_item_to_storage(str(tmp_path))
 
     assert result == "text-file-path"
-    mock_save.assert_awaited_once()
+    save_mock.assert_awaited_once_with(str(tmp_path))
+
+
+@pytest.mark.asyncio
+@pytest.mark.skipif(os.name != "nt", reason="Windows absolute-path semantics")
+async def test_genuine_windows_absolute_path_returns_file_uri(tmp_path):
+    # A real Windows absolute path (C:\...) to an existing file still converts
+    # to a file:// URI. Runs only on Windows, where tmp_path is drive-anchored.
+    file_path = tmp_path / "note.txt"
+    file_path.write_text("hello", encoding="utf-8")
+
+    result = await save_data_item_to_storage(str(file_path))
+
+    assert result == file_path.as_uri()