fix(repo): keep the log's history readable when the data is a folder of a bigger repo (0.4.10)

README.md

@@ -63,7 +63,7 @@ Only one process may poll a bot token. `sct say` does not poll, so it is safe be
 
 | variable | meaning |
 |---|---|
-| `SCT_DATA` | the data directory (default `/data`) |
+| `SCT_DATA` | the data directory (default `/data`): a checkout, or a folder inside one (mount the whole checkout) |
 | `TELEGRAM_BOT_TOKEN` | the bot token |
 | `SCT_GIT_TOKEN` | a token that can push to the data repository, sent as an `Authorization` header |
 | `TZ` | the container timezone; dates follow `profile.timezone` |

docs/DESIGN.md

@@ -1220,7 +1220,7 @@ README.md  AGENTS.md
 
 | variable | meaning |
 |---|---|
-| `SCT_DATA` | the data directory (default `/data`), a git checkout of the private data repository |
+| `SCT_DATA` | the data directory (default `/data`), a git checkout of the private data repository, or a folder inside one (mount the whole checkout) |
 | `TELEGRAM_BOT_TOKEN` | the bot token |
 | `SCT_GIT_TOKEN` | a token with write access to the data repository, sent as an `Authorization` header on pull and push |
 | `TZ` | the container timezone; the app uses `profile.timezone` for every date |

supercycletracker/__init__.py

@@ -1,3 +1,3 @@
 """SuperCycleTracker: a menstrual cycle tracker that lives in a Telegram group."""
 
-__version__ = "0.4.9"
+__version__ = "0.4.10"

supercycletracker/repo.py

@@ -364,7 +364,7 @@ def _run(self, *args: str, code: str) -> str:
 
     def _show(self, rev: str) -> str | None:
         """The log's text at `rev`, or None when that revision has no log."""
-        r = self._git("show", f"{rev}:{LOG_FILE}")
+        r = self._git("show", f"{rev}:./{LOG_FILE}")  # `./`: the data may be a folder of the repo
         return r.stdout if r.returncode == 0 else None
 
     def _dirty(self) -> bool:
@@ -527,8 +527,10 @@ def write_profile(self, fn: Callable[[str], str], message: str) -> str:
 
     def undo(self) -> Change | None:
         """Undo the last check-in. While the log is dirty, restore it from HEAD (the whole
-        uncommitted batch goes, nothing is committed). When it is clean and HEAD is the bot's
-        `log:` commit, restore it from HEAD~1 and commit `undo:`. Otherwise None.
+        uncommitted batch goes, nothing is committed). When it is clean and the last commit is
+        the bot's `log:` commit, restore it from that commit's parent and commit `undo:`.
+        Otherwise None. With the data in a folder of a bigger repo, the last commit is the last
+        one that touched the folder, so another folder's commit on top does not hide it.
 
         Returns a `Change` from the current log to the restored one: the dates that went back.
         """
@@ -537,11 +539,12 @@ def undo(self) -> Change | None:
             if self._dirty():
                 rev, message = "HEAD", None
             else:
-                head = self._run("log", "-1", "--format=%ae%x00%s", code="log_failed")
-                email, _, subject = head.rstrip("\n").partition("\x00")
+                scope = () if (self.data / ".git").exists() else ("--", ".")
+                head = self._run("log", "-1", "--format=%H%x00%ae%x00%s", *scope, code="log_failed")
+                sha, email, subject = head.rstrip("\n").split("\x00", 2)
                 if email != BOT_EMAIL or not subject.startswith(LOG_PREFIX):
                     return None
-                rev, message = "HEAD~1", UNDO_PREFIX + subject.removeprefix(LOG_PREFIX)
+                rev, message = f"{sha}~1", UNDO_PREFIX + subject.removeprefix(LOG_PREFIX)
             target = self._show(rev)
             if target is not None:
                 self._run("checkout", rev, "--", LOG_FILE, code="checkout_failed")

tests/conftest.py

@@ -91,6 +91,33 @@ def data_repo(_data_repo_template, tmp_path, monkeypatch) -> DataRepo:
     return DataRepo(remote=root / "remote.git", data=root / "data", human=root / "human")
 
 
+@pytest.fixture
+def subfolder_repo(tmp_path, monkeypatch) -> DataRepo:
+    """The data in a folder of a bigger checkout, `tracker/`, next to `doctor/`, which another
+    bot writes from its own clone (`human` here is that clone's root)."""
+    for key, value in ISOLATED.items():
+        monkeypatch.setenv(key, value)
+    root = tmp_path / "repos"
+    root.mkdir()
+    remote, seed = root / "remote.git", root / "seed"
+    git(root, "init", "--quiet", "--bare", "-b", "main", str(remote))
+    git(root, "init", "--quiet", "-b", "main", str(seed))
+    (seed / "tracker" / "cycle").mkdir(parents=True)
+    records.save(seed / "tracker" / "cycle" / "log.jsonl", seed_log())
+    (seed / "tracker" / "cycle" / "profile.yaml").write_text(
+        "name: Example\nlanguage: es\n", encoding="utf-8"
+    )
+    (seed / "tracker" / ".gitignore").write_text("state/\n", encoding="utf-8")
+    (seed / "doctor").mkdir()
+    (seed / "doctor" / "notes.md").write_text("one\n", encoding="utf-8")
+    git(seed, "add", ".")
+    git(seed, "commit", "--quiet", "-m", "seed")
+    git(seed, "push", "--quiet", str(remote), "main")
+    for name in ("bot", "other"):
+        git(root, "clone", "--quiet", "remote.git", name)
+    return DataRepo(remote=remote, data=root / "bot" / "tracker", human=root / "other")
+
+
 class FakeTelegram:
     """Stands in for `Telegram`: records every call, hands out increasing message ids, and
     raises a scripted `TelegramError` on demand. Safe across threads (the worker sends too).

tests/test_repo.py

@@ -482,6 +482,75 @@ def test_undo_on_an_undo_or_a_persons_commit_has_nothing_to_undo(data_repo):
     assert subject(dr.data) == "undo: 2027-03-04"
 
 
+# --- the data in a folder of a bigger checkout ------------------------------------------------
+
+
+def other_bot_writes(dr, text):
+    """The other bot, in its own clone, commits to its own folder and pushes."""
+    git(dr.human, "pull", "--quiet", "--rebase")
+    (dr.human / "doctor" / "notes.md").write_text(text, encoding="utf-8")
+    git(dr.human, "commit", "--quiet", "-am", "doctor: note")
+    git(dr.human, "push", "--quiet")
+
+
+def test_a_data_folder_commits_and_rebases_on_the_other_folders_push(subfolder_repo):
+    dr = subfolder_repo
+    repo, n0 = Repo(dr.data, clock=Clock()), dr.remote_count()
+    tap(repo, flow="medium")
+    other_bot_writes(dr, "two\n")
+    assert repo.flush(force=True) == "pushed"
+    assert dr.remote_count() == n0 + 2
+    assert subject(dr.remote, "main") == "log: 2027-03-04"
+    assert dr.remote_file("tracker/" + LOG_FILE) == dr.log.read_text(encoding="utf-8")
+    assert dr.remote_file("doctor/notes.md") == "two\n"
+    assert (dr.data.parent / "doctor" / "notes.md").read_text() == "two\n"
+
+
+def test_a_data_folder_undoes_its_last_check_in(subfolder_repo):
+    dr = subfolder_repo
+    repo = Repo(dr.data, clock=Clock())
+    seed = dr.log.read_text(encoding="utf-8")
+    tap(repo, flow="medium", pain=3)
+    assert repo.flush(force=True) == "pushed"
+    change = repo.undo()
+    assert change.dates == (DAY,)
+    assert subject(dr.data.parent) == "undo: 2027-03-04"
+    assert dr.log.read_text(encoding="utf-8") == seed
+
+
+def test_a_data_folder_undoes_a_check_in_under_the_other_folders_commit(subfolder_repo):
+    dr, clock = subfolder_repo, Clock()
+    repo = Repo(dr.data, clock=clock)
+    seed = dr.log.read_text(encoding="utf-8")
+    tap(repo, flow="medium", pain=3)
+    assert repo.flush(force=True) == "pushed"
+    other_bot_writes(dr, "two\n")
+    clock.advance(3600)
+    assert repo.flush() == "pulled"
+    assert subject(dr.data.parent) == "doctor: note"  # HEAD is not the bot's check-in
+    change = repo.undo()
+    assert change.dates == (DAY,)
+    assert dr.log.read_text(encoding="utf-8") == seed
+    assert repo.flush(force=True) == "pushed"
+    assert subject(dr.remote, "main") == "undo: 2027-03-04"
+    assert dr.remote_file("doctor/notes.md") == "two\n"
+    assert repo.undo() is None  # the last commit in the folder is `undo:`
+
+
+def test_a_data_folder_fails_as_a_conflict_when_its_log_is_deleted_on_the_remote(subfolder_repo):
+    dr = subfolder_repo
+    repo = Repo(dr.data, clock=Clock())
+    tap(repo, flow="heavy")
+    git(dr.human, "rm", "--quiet", "tracker/" + LOG_FILE)
+    git(dr.human, "commit", "--quiet", "-m", "remove the log")
+    git(dr.human, "push", "--quiet")
+    assert repo.flush(force=True) == "failed"
+    assert repo.last_error == "pull_conflict"
+    assert not rebasing(dr.data)
+    assert repo.unpushed() == 1
+    assert records.load(dr.log)[DAY].flow == "heavy"
+
+
 def test_commit_messages_carry_dates_only(data_repo):
     dr = data_repo
     repo = Repo(dr.data, clock=Clock())