From dec2bfc7b69d333423d852ba2a6b554d5be8842e Mon Sep 17 00:00:00 2001 From: Eric Wagoner Date: Thu, 6 Aug 2026 00:43:35 -0400 Subject: [PATCH] Defuse the clone-and-run trap: tracked pipeline data warns loudly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eric spotted it: the README told people to clone this repo, rm the committed data, and run — which writes THEIR pipeline artifacts at git-TRACKED paths. The next `git pull` (this repo commits data every session) refuses to merge, and the internet's standard remedies for that error — reset --hard, checkout ., stash, clean -fdx — destroy their review decisions, hand-written games, upload log, and photos. Two layers. The README's "Bring your own shelves" now leads with `uv tool install git+…` and running in a directory of your own: data lands untracked by construction and a bug fix is `uv tool upgrade`, which cannot touch it. And because nobody re-reads a README, Config gains tracked_data_warning(): if artifacts under data_dir are git-tracked, `bggpipe init` and the web dashboard both warn in plain words. The owner's exemption is data/.own_repo — a GITIGNORED marker, so the author's checkout is silent while a fresh clone of the same repo still gets the warning (a committed marker or config key would have shipped the exemption to exactly the people who need warning). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g --- .gitignore | 1 + README.md | 14 +++++++++---- src/bggpipe/config.py | 33 ++++++++++++++++++++++++++++++ src/bggpipe/init_wizard.py | 4 ++++ src/bggpipe/webreview.py | 3 +++ tests/test_config.py | 42 ++++++++++++++++++++++++++++++++++++++ tests/test_upload.py | 2 +- 7 files changed, 94 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index a4be516..e55cc11 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .env *.storage_state.json data/.lan_key +data/.own_repo playwright/.auth/ storage_state.json diff --git a/README.md b/README.md index 56a437a..564ef78 100644 --- a/README.md +++ b/README.md @@ -143,14 +143,20 @@ Straight-on, one shelf (or part of one) per shot, close enough that spine text i ## Bring your own shelves -This repo doubles as its author's live pipeline, so `data/` ships with his real artifacts — extracted titles, matches, and 2018 collection snapshots. Before running against *your* shelves, clear the data: +Install bggpipe as a tool and run it in its own directory — **not inside a clone of this repo**: ```sh -rm data/*.csv data/*.json data/*.xml data/STUB_DATA.marker -rm -rf data/bgg_cache data/extract_raw +uv tool install git+https://git.kestrelsnest.social/eric/bggpipe +mkdir ~/shelves && cd ~/shelves +bggpipe init # folders, credentials, one-time browser install +bggpipe web # or run stages from the terminal ``` -Two of those files deserve a word: +Everything the pipeline produces — photos, matches, review decisions, your upload log — lives in the directory where you run it. A bug fix is `uv tool upgrade bggpipe`, which by construction cannot touch your data. + +**Why not clone and run?** This repo doubles as its author's live pipeline: `data/` ships with their real artifacts, committed and updated often. Run inside a clone and *your* data lands at git-tracked paths — the next `git pull` will refuse to merge, and the usual remedies (`git reset --hard`, `git checkout .`, `git stash`, `git clean -fdx`) would destroy your review decisions, hand-written games, upload log, and photos. bggpipe detects this arrangement and warns at `init` and on the web dashboard; don't ignore it. Clone only to develop (see [Development](#development)), and keep your own pipeline runs elsewhere. + +Two files of the author's data deserve a word: - **`data/STUB_DATA.marker`** — normally absent. It appears only if the synthetic stub fixtures (from `scripts/write_stub_fixtures.py`) regenerate the CSVs, and the upload stage refuses to run while it exists — so placeholder data can never reach a real BGG account. The committed CSVs are real API data. - **`data/collection_snapshot_*.xml`** — with `BGG_API_TOKEN` set, `diff` fetches your collection live and you don't need these. Without a token (still waiting on approval?), you can use the logged-in-browser exemption: while signed in to BGG, save these two URLs as `data/collection_snapshot_base.xml` and `data/collection_snapshot_expansions.xml` (if you get a "queued" message, refresh after a few seconds): diff --git a/src/bggpipe/config.py b/src/bggpipe/config.py index 84bb0ca..1529391 100644 --- a/src/bggpipe/config.py +++ b/src/bggpipe/config.py @@ -9,6 +9,7 @@ account has exactly one home. from __future__ import annotations import os +import subprocess import tomllib import warnings from dataclasses import dataclass, replace @@ -74,6 +75,38 @@ class Config: def games_path(self) -> Path: return self.data_dir / "games.json" + def tracked_data_warning(self) -> str | None: + """The clone-and-run trap: pipeline data written at git-TRACKED + paths is one `git pull` + one panicked `git reset --hard` away + from destruction. Warn whenever artifacts under data_dir are + tracked — unless the owner has marked this checkout as their own + repo (the marker is gitignored, so a fresh clone never inherits + the exemption).""" + if (self.data_dir / ".own_repo").exists(): + return None + try: + tracked = subprocess.run( # noqa: S603 + ["git", "ls-files", "--", str(self.data_dir)], # noqa: S607 + capture_output=True, + text=True, + timeout=10, + cwd=self.data_dir.resolve().parent, + ) + except (OSError, subprocess.TimeoutExpired): + return None # no git, no repo, no trap + if tracked.returncode != 0 or not tracked.stdout.strip(): + return None + return ( + f"your pipeline data ({self.data_dir}) sits at git-TRACKED " + "paths — a `git pull` here can refuse to merge, and the usual " + "remedies (reset --hard, checkout ., stash) would destroy your " + "review decisions, hand-written games, and upload log. Run " + "bggpipe from a directory outside this repository (see README: " + "Bring your own shelves). If this repo is genuinely where you " + f"version your own data, `touch {self.data_dir}/.own_repo` to " + "accept the arrangement and silence this warning." + ) + @property def local_games_path(self) -> Path: # hand-written metadata for games BGG doesn't have — the only diff --git a/src/bggpipe/init_wizard.py b/src/bggpipe/init_wizard.py index f66729d..f898d72 100644 --- a/src/bggpipe/init_wizard.py +++ b/src/bggpipe/init_wizard.py @@ -88,6 +88,7 @@ class InitReport: keys_written: list[str] = field(default_factory=list) keys_missing: list[str] = field(default_factory=list) browser_installed: bool = False + warnings: list[str] = field(default_factory=list) def _env_file_keys(env_path: Path) -> set[str]: @@ -153,6 +154,9 @@ def run_init( report = InitReport() typer.echo("bggpipe setup — checks what exists, fills only the gaps.\n") + if trap := cfg.tracked_data_warning(): + typer.echo(f"WARNING: {trap}\n", err=True) + report.warnings.append(trap) # -- directories ---------------------------------------------------- for path in (project_dir / cfg.photos_dir, project_dir / cfg.data_dir): diff --git a/src/bggpipe/webreview.py b/src/bggpipe/webreview.py index d2a0d98..73eff65 100644 --- a/src/bggpipe/webreview.py +++ b/src/bggpipe/webreview.py @@ -354,6 +354,9 @@ def create_app( # server-side degradations, shown in-browser (quarantined dismiss file, # unreadable thumbnails, torn artifacts) app_warnings: list[str] = list(startup_notes) + if trap := cfg.tracked_data_warning(): + typer.echo(f"WARNING: {trap}", err=True) + app_warnings.append(trap) def warn_once(note: str) -> None: if note not in app_warnings: diff --git a/tests/test_config.py b/tests/test_config.py index 0b997cf..563fa2e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os from pathlib import Path import pytest @@ -96,3 +97,44 @@ model = "claude-sonnet-5" pytest.warns(UserWarning, match="anthorpic"), ): load_config(p) + + +def _git(tmp_path, *args): + import subprocess + + subprocess.run( + ["git", *args], + cwd=tmp_path, + check=True, + capture_output=True, + env={ + "PATH": os.environ["PATH"], + "HOME": str(tmp_path), + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@t", + }, + ) + + +def test_tracked_data_warning_fires_only_inside_a_repo(tmp_path): + """The clone-and-run trap: data at git-tracked paths is one panicked + `git reset --hard` from destruction. Outside a repo, silence.""" + data = tmp_path / "data" + data.mkdir() + (data / "matches.csv").write_text("x") + cfg = Config(data_dir=data) + assert cfg.tracked_data_warning() is None # no repo, no trap + + _git(tmp_path, "init") + assert cfg.tracked_data_warning() is None # repo, but nothing tracked + _git(tmp_path, "add", "data/matches.csv") + _git(tmp_path, "commit", "-m", "seed") + warning = cfg.tracked_data_warning() + assert warning and "git-TRACKED" in warning and ".own_repo" in warning + + # the owner's exemption is a gitignored marker: a fresh clone of this + # repo would NOT carry it, so only the author's checkout is silenced + (data / ".own_repo").touch() + assert cfg.tracked_data_warning() is None diff --git a/tests/test_upload.py b/tests/test_upload.py index d1d6827..255cc1f 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -20,8 +20,8 @@ from bggpipe.upload import ( _scrub, annotate_queue, build_queue, - stale_jobs, run_upload, + stale_jobs, verify_uploads, )