From 1a11291f4526c03f682bef93ddf922172e6876f4 Mon Sep 17 00:00:00 2001 From: mgrani Date: Tue, 04 Aug 2026 11:29:41 +0000 Subject: [PATCH] fix(remote): matching no datasets is a failure, not a quiet success `remote pull` returned success=True and exit 0 when discovery matched nothing, which is indistinguishable from "the scope is already in sync". That is how the --push-to source bug survived: an OWI mirror ran nightly against a specifier that could never match, recorded success every time, and moved nothing for weeks. Shipping the --push-to fix unblocks that mirror but leaves the next failure of this shape equally invisible. Zero *discovered* datasets now returns success=False with ErrorType.DATA and ExitCode.DATA_ERROR, naming the specifier -- "no datasets" without it sends the reader looking in the wrong place. The HTTP job layer maps success=False to a failed job, so API callers see it too. Deliberately narrow. "Nothing to transfer" stays a success and stays exit 0: that case has datasets and simply finds their files already present, and conflating the two would make an idle mirror look broken every night. Both are now pinned by tests, including the counter-case. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019nPNpEG2QRnmjxxn5LeEb7 --- owilix/core/tasks/remote.py | 26 ++++++++++++++++++++++++++ tests/owilix/core/tasks/test_remote.py | 40 ++++++++++++++++++++++++++++++++++++++-- 2 file(s) changed, 64 insertion(s)(+), 2 deletion(s)(-) diff --git a/owilix/core/tasks/remote.py b/owilix/core/tasks/remote.py --- a/owilix/core/tasks/remote.py +++ b/owilix/core/tasks/remote.py @@ -284,6 +284,32 @@ total_files = sum(d.metadata.get('fileCount', 0) or 0 for d in datasets) console.print(f"\n[bold]Summary:[/bold] {len(datasets)} datasets found, {total_size/1e9:.1f} GB, {total_files:,} files") + # Matching nothing is not success. + # + # This used to fall through to the success path below, so a specifier that + # selected no datasets at all was indistinguishable from one whose datasets + # were already in sync -- both printed a summary and exited 0. That is how + # the `--push-to` source bug survived: an OWI mirror ran nightly against a + # specifier that could never match, recorded success every time, and moved + # nothing for weeks. + # + # "Nothing to transfer" is still success and still exits 0; that case has + # datasets and simply finds their files present. What is reported here is + # narrower: discovery matched no dataset, so nothing was even considered. + if not datasets: + from owilix.core.types import ErrorType, ExitCode + return CommandResult( + success=False, + object={"datasets": [], "specifier": specifier}, + msg=( + f"No datasets matched specifier '{specifier}'" + + (f" (push target {push_to_remote})" if push_to_remote else "") + ), + error_type=ErrorType.DATA, + exit_code=ExitCode.DATA_ERROR, + command="remote pull", + ) + _add = "" if push_to_remote is None else f" and push to {push_to_remote}" failed_files: List[tuple] = [] # (file_path, error_message) diff --git a/tests/owilix/core/tasks/test_remote.py b/tests/owilix/core/tasks/test_remote.py --- a/tests/owilix/core/tasks/test_remote.py +++ b/tests/owilix/core/tasks/test_remote.py @@ -745,11 +745,47 @@ args, _ = manager.remote_data.list.call_args assert args[0] == "lexis" - def test_no_datasets_found(self): + def test_no_datasets_found_is_a_failure_not_a_quiet_success(self): + """Matching nothing must be distinguishable from being up to date. + + This asserted `success is True` until 2026-08-04. That is precisely how + the `--push-to` source bug stayed hidden: an OWI mirror ran nightly + against a specifier that could never match, took this path every time, + exited 0, and moved nothing for weeks. A caller had no way to tell + "already in sync" from "selected nothing at all". + """ + from owilix.core.types import ErrorType, ExitCode + manager = _make_mock_manager(datasets=[]) result = remote_pull(manager, specifier="dc1/public", auto_yes=True, console=MagicMock()) + + assert result.success is False + assert result.exit_code == ExitCode.DATA_ERROR + assert result.error_type == ErrorType.DATA + # The specifier is in the message, because "no datasets" without it + # sends the reader looking in the wrong place. + assert "dc1/public" in result.msg + + @patch("owilix.core.tasks.remote.currentItemProgress") + @patch("owilix.core.tasks.remote.ask_yes_no", return_value=True) + def test_datasets_found_but_nothing_to_transfer_is_still_success( + self, mock_ask, mock_progress, tmp_path + ): + """The case that must NOT be swept up: discovery worked, files are present. + + "Nothing to do" and "nothing found" are different states and only the + second is a fault; conflating them would make an idle mirror look broken + every night. + """ + ds = _make_mock_dataset() + manager = _make_mock_manager(datasets=[ds]) + manager.remote_data.files.return_value = [] + + result = remote_pull( + manager, specifier="dc1/public", files="['**/*']", console=MagicMock() + ) + assert result.success is True - assert "0 datasets" in result.msg @patch("owilix.core.tasks.remote.ask_yes_no", return_value=False) def test_user_declines(self, mock_ask): -- tangled.sh