From e66945983788f103fad7a5963b3a70ac12081a2a Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 11 Sep 2026 00:06:19 +0200 Subject: [PATCH] fix(memory): stop the curator calling a working mirror broken The curator logged "brain git diff failed" every cycle that actually had something to mirror. Nothing was wrong: `git diff --cached --quiet` exits 1 to say staged changes exist, which is the reason to commit, but the brain helper treated every non-zero exit as a fault. So the log announced a broken mirror at precisely the moments it was doing its job, and went quiet when there was genuinely nothing to do. Anyone debugging the memory wiki read it backwards. --- stacklets/memory/bot/curator.py | 8 +++-- tests/stacklets/test_memory_curator_sync.py | 37 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/stacklets/memory/bot/curator.py b/stacklets/memory/bot/curator.py index 59f26ca6..1378b646 100644 --- a/stacklets/memory/bot/curator.py +++ b/stacklets/memory/bot/curator.py @@ -685,8 +685,12 @@ def _commit_push(self, message: str) -> bool: True when a commit was made and pushed (or there was nothing to commit, which is still a success).""" self._run("add", "-A") - # `diff --cached --quiet` exits 1 when staged changes exist. - code, _, _ = self._run("diff", "--cached", "--quiet") + # `diff --cached --quiet` exits 1 when staged changes exist, so its + # exit code is the answer to "is there anything to commit?" rather + # than a fault. It calls git directly to stay out of `_run`, which + # reports every non-zero exit as a failure and would therefore + # announce a broken mirror on every cycle that had work to do. + code, _, _ = run_git(self.path, "diff", "--cached", "--quiet", env=self._env) if code != 0: rc, _, _ = self._run( "-c", f"user.name={_BRAIN_AUTHOR_NAME}", diff --git a/tests/stacklets/test_memory_curator_sync.py b/tests/stacklets/test_memory_curator_sync.py index 72e63882..96d33f65 100644 --- a/tests/stacklets/test_memory_curator_sync.py +++ b/tests/stacklets/test_memory_curator_sync.py @@ -319,6 +319,21 @@ def warnings_logged(): logger.remove(sink_id) +@pytest.fixture +def debug_logged(): + """Everything the curator says, DEBUG included. + + `warnings_logged` deliberately starts at WARNING. The line pinned + below only ever appears at DEBUG, so it needs its own sink. + """ + from loguru import logger + + lines: list[str] = [] + sink_id = logger.add(lines.append, level="DEBUG") + yield lines + logger.remove(sink_id) + + class TestVaultSync: """The curator's vault sync: source policy, one auth retry, loud.""" @@ -414,3 +429,25 @@ async def test_a_push_that_cannot_land_is_reported_not_forced( assert any("not reaching Forgejo" in line for line in warnings_logged) # The commit is still here for the next cycle to deliver. assert "brain: project" in _log_subjects(local) + + async def test_a_routine_commit_does_not_report_git_as_failing( + self, git_healthy_clone, tmp_path, debug_logged, + ): + """`git diff --cached --quiet` answers with its exit code, and 1 + means staged changes exist — the reason to commit, not a fault. + Reporting it as "brain git diff failed" told anyone reading the + curator's log that the mirror was broken at exactly the moments + it was working, which is worse than silence. + """ + from curator import CURATOR_REMOTE, Brain + + local = git_healthy_clone.local + _git(local, "remote", "add", CURATOR_REMOTE, + str(git_healthy_clone.remote)) + (local / "index.md").write_text("generated\n", encoding="utf-8") + + pushed = await Brain(local, tmp_path / "source").commit_push("brain: project") + + assert pushed is True + assert "brain: project" in _log_subjects(local) + assert [ln for ln in debug_logged if "git diff failed" in ln] == []