From 3dc52c7f0d3f019fc3b72fff29e9915de791a602 Mon Sep 17 00:00:00 2001 From: Anoop Narang Date: Thu, 24 Sep 2026 18:56:01 +0530 Subject: [PATCH 1/3] chore: record the fork's conventions where agents will read them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in this repo said it was a fork. `AGENTS.md` and `README.md` are upstream's and describe the project, so an agent arriving here saw an ordinary liquid-cache checkout and had to learn the rest by breaking something: branching off a stale local `main`, cutting an upstream PR from our `main` and carrying the whole patch stack into it, or re-reporting the reclaim race as new. `CLAUDE.md` is a file upstream does not have, which is the point. A convention recorded in an upstream-owned file conflicts on every sync, and this fork has already paid enough for history problems. Adding a file upstream will never create cannot conflict. It states what differs here and nothing else: that `main` is upstream plus our patches, that upstream PRs branch from upstream under `upstream/`, that the toolchain is `cargo +1.96.0`, that the shuttle feature has to be run with a `shuttle_` filter or ~49 unrelated tests fail by design, that `dev-tools` needs a generated asset, and that the disk reclaim race is known and open. The guard workflow enforces the branch rule rather than trusting it. `main` is upstream plus our patches, so a correct `upstream/**` branch cannot have `main` in its ancestry; one `merge-base --is-ancestor` tests exactly that, and tests the real property rather than a proxy like which files are present. It runs on push, so the author learns before a PR exists. Its failure output explains rather than rejects: what was detected, why it matters — the reviewer sees a diff far larger than the change, mixed with work never meant for them — and the commands to redo the branch from upstream and to check before pushing. Both paths were run against real branches: this one fails, and `upstream/keep-column-free-conjuncts` passes. --- .github/workflows/upstream-branch-guard.yml | 82 +++++++++++++++++++++ CLAUDE.md | 66 +++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 .github/workflows/upstream-branch-guard.yml create mode 100644 CLAUDE.md diff --git a/.github/workflows/upstream-branch-guard.yml b/.github/workflows/upstream-branch-guard.yml new file mode 100644 index 00000000..5cc3f417 --- /dev/null +++ b/.github/workflows/upstream-branch-guard.yml @@ -0,0 +1,82 @@ +# An `upstream/**` branch exists to become a pull request against +# datafusion-contrib/liquid-cache. Our `main` is upstream plus our patches, so a +# branch cut from `main` carries every one of those patches into the upstream +# diff. This fails that on push, before a PR exists, and explains why. +name: upstream branch guard + +on: + push: + branches: ['upstream/**'] + +permissions: + contents: read + +jobs: + guard: + name: branch is based on upstream, not on our main + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check ancestry + run: | + set -euo pipefail + git fetch --quiet origin main:refs/remotes/origin/main + + if ! git merge-base --is-ancestor refs/remotes/origin/main HEAD; then + echo "OK: this branch does not contain our main." + carried=$(git rev-list --count refs/remotes/origin/main..HEAD) + echo "It is $carried commit(s) ahead of our main's line, which is what an upstream branch should look like." + exit 0 + fi + + # Contaminated. Say what is wrong, why it matters, and how to fix it. + extra=$(git rev-list --count refs/remotes/origin/main) + cat < FETCH_HEAD + git cherry-pick + + HOW TO CHECK BEFORE PUSHING + This command must print nothing. Anything it prints is a commit of + ours that would land in the upstream diff: + + git log --oneline origin/main..HEAD ^FETCH_HEAD + + ONE MORE THING + Reproduce the bug on upstream's tree before opening the PR: apply + your test alone, watch it fail there, then apply the fix. A fix + whose code still exists upstream is not evidence the bug does. + + (For scale: our main carries $extra commits of history.) + + MSG + echo "::error::Branch descends from our main; an upstream PR from it would carry this fork's patches. See the log above." + exit 1 diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..e5375f03 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,66 @@ +# Working in this fork + +`hotdata-dev/liquid-cache` is a fork of `datafusion-contrib/liquid-cache`. +`main` is upstream's `0033b15` with our patches on top — upstream's history is +fully contained, so `git merge-base main ` resolves. + +`AGENTS.md` and `README.md` are upstream's and describe the project itself. +This file is ours and describes only what differs here. Nothing in this repo +should edit an upstream-owned file to record a fork convention: that conflicts +on every sync. Add a file upstream does not have instead. + +## Branches + +- Work off `main`. `git fetch fork` first — a local `main` goes stale with no + signal, and branching off a stale one silently drops everything merged since. +- **Upstream PRs branch from upstream, not from `main`**, and are named + `upstream/`. A branch cut from `main` carries our whole patch stack + into the PR diff. + + ``` + git fetch https://github.com/datafusion-contrib/liquid-cache main + git checkout -b upstream/ FETCH_HEAD + ``` + + `.github/workflows/upstream-branch-guard.yml` fails any `upstream/**` branch + that descends from `main`, on push, before a PR exists. + +- Before raising an upstream PR, reproduce the bug **on upstream's tree** — + apply the test alone, watch it fail, then apply the fix. A fix whose code + still exists upstream is not evidence the bug does; that mistake has cost us + a withdrawn PR. Several of our patches repair machinery upstream does not + have (`DiskResidue`, `reclaim_orphaned_disk`, `settle`) and are not + upstreamable at all. + +## Building and testing + +- **`cargo +1.96.0`.** The dependency tree needs 1.95+ (`vortex-*`, `sysinfo`) + and DataFusion 55 needs 1.94. A bare `cargo` on an older default fails + resolution with a wall of `requires rustc 1.9x` lines. +- **Shuttle tests need a filter**: `cargo +1.96.0 test -p liquid-cache + --features shuttle --lib shuttle_`. The feature swaps `crate::sync` to + shuttle primitives for the whole test build, so running it unfiltered fails + ~49 unrelated tests with "Are you accessing a Shuttle primitive outside of a + Shuttle test?". That is by design, not a regression. +- **`dev-tools` needs `dev/dev-tools/assets/tailwind.css`**, which CI generates + and the repo does not carry. To run its tests locally, create a placeholder + and delete it before committing. Do not habitually pass + `--exclude dev-tools`: it hides real failures, including trace-snapshot + breakage from new cache events. +- After a structural edit, compare the **test inventory**, not just the pass + count. A `#[cfg(feature = "shuttle")]` test can be deleted without moving the + default-build total at all, and CI stays green because a missing test is not + a failing one. + +## Known open issue + +The disk reclaim path has an unclosed race. A store key is +`(entry id, identity)` and identities are reused — the file-id pool hands a +re-opened path its previous record. `reclaim_orphaned_disk` consults the index +before deleting, but a put that has landed while its index record is not yet +installed is invisible to that check, and t4 applies puts and tombstones by +LSN, so the later `remove` wins and deletes live bytes. + +Closing it needs a per-write generation in the store key, which also makes +`DiskResidue::superseded` unreachable and removes the in-place-overwrite case. +Not yet done. Do not re-report it as new. From 6d3111b66a2920dd721cf8a4c8932f817f9c7724 Mon Sep 17 00:00:00 2001 From: Anoop Narang Date: Thu, 24 Sep 2026 19:03:03 +0530 Subject: [PATCH 2/3] fix: test where an upstream branch diverged, not whether it has main's tip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the guard passes the case it exists for. It asked `git merge-base --is-ancestor main HEAD`, which tests whether a branch contains `main`'s *current* tip. `main` moves with every merge, so a branch cut from `main` last week has today's tip nowhere in its history and the check reported "OK". Verified: a branch at `9298d97`, our own main a few hours ago, passed while carrying 19 fork commits. Every case I tested cut from the tip as it stood, which is the one arrangement where the wrong test gives the right answer. Test the merge base instead. A branch cut from upstream shares only upstream history with `main`, so that point is a commit upstream has; a branch cut from `main` shares a fork commit, and upstream never contains one. Run against six refs — upstream's tip, a branch off upstream, our main's tip, our main as of two merges ago, a commit mid fork history, and this branch — it is right on all six. The self-check printed in the failure was wrong in the same spirit. `git log origin/main..HEAD ^FETCH_HEAD` excludes everything reachable from `main`, which is every fork patch, so it lists the author's own commits on a clean branch and a contaminated one alike. It can never be empty, and the instruction said it must be. Now `git log FETCH_HEAD..HEAD`, described as what it is: a list that should contain only your own commits. The failure also now names the branch point, the upstream tip, how many fork commits are carried and the first five of them, rather than asserting that some are. `CLAUDE.md` said `git fetch fork`, which is one local remote naming and not what a fresh clone has. It says `origin` and notes the alternative, and carries the corrected self-check. --- .github/workflows/upstream-branch-guard.yml | 82 +++++++++++++-------- CLAUDE.md | 19 ++++- 2 files changed, 66 insertions(+), 35 deletions(-) diff --git a/.github/workflows/upstream-branch-guard.yml b/.github/workflows/upstream-branch-guard.yml index 5cc3f417..ce9b39a4 100644 --- a/.github/workflows/upstream-branch-guard.yml +++ b/.github/workflows/upstream-branch-guard.yml @@ -1,7 +1,8 @@ # An `upstream/**` branch exists to become a pull request against -# datafusion-contrib/liquid-cache. Our `main` is upstream plus our patches, so a -# branch cut from `main` carries every one of those patches into the upstream -# diff. This fails that on push, before a PR exists, and explains why. +# datafusion-contrib/liquid-cache. Our `main` is upstream plus this fork's +# patches, so a branch cut from `main` carries every one of those patches into +# the upstream diff. This fails that on push, before a PR exists, and explains +# why. name: upstream branch guard on: @@ -20,63 +21,82 @@ jobs: with: fetch-depth: 0 - - name: Check ancestry + - name: Check where this branch diverged run: | set -euo pipefail - git fetch --quiet origin main:refs/remotes/origin/main - if ! git merge-base --is-ancestor refs/remotes/origin/main HEAD; then - echo "OK: this branch does not contain our main." - carried=$(git rev-list --count refs/remotes/origin/main..HEAD) - echo "It is $carried commit(s) ahead of our main's line, which is what an upstream branch should look like." + git fetch --quiet origin main:refs/remotes/origin/main + git fetch --quiet https://github.com/datafusion-contrib/liquid-cache main + upstream=$(git rev-parse FETCH_HEAD) + + # Where this branch and our main last shared history. A branch cut + # from upstream shares only upstream history with our main, so that + # point is a commit upstream has. A branch cut from our main shares a + # fork commit, and upstream never contains one. + # + # Testing the merge base rather than our main's tip matters: `main` + # moves with every merge, so a branch cut from `main` last week has + # today's tip nowhere in its history and would look clean. + base=$(git merge-base HEAD refs/remotes/origin/main) + + if git merge-base --is-ancestor "$base" "$upstream"; then + own=$(git rev-list --count "$upstream"..HEAD) + echo "OK: this branch diverged from upstream history, not from our fork's." + echo "It adds $own commit(s) on top of upstream, which is what an upstream branch should look like." exit 0 fi - # Contaminated. Say what is wrong, why it matters, and how to fix it. - extra=$(git rev-list --count refs/remotes/origin/main) + carried=$(git rev-list --count "$upstream".."$base") cat < FETCH_HEAD git cherry-pick HOW TO CHECK BEFORE PUSHING - This command must print nothing. Anything it prints is a commit of - ours that would land in the upstream diff: + This must list only the commits you wrote. Anything else in it is + a fork patch that would land in the upstream diff: - git log --oneline origin/main..HEAD ^FETCH_HEAD + git fetch https://github.com/datafusion-contrib/liquid-cache main + git log --oneline FETCH_HEAD..HEAD ONE MORE THING Reproduce the bug on upstream's tree before opening the PR: apply your test alone, watch it fail there, then apply the fix. A fix whose code still exists upstream is not evidence the bug does. - (For scale: our main carries $extra commits of history.) - MSG - echo "::error::Branch descends from our main; an upstream PR from it would carry this fork's patches. See the log above." + echo "::error::Branch is based on our main and carries $carried fork commit(s); an upstream PR from it would include them. See the log above." exit 1 diff --git a/CLAUDE.md b/CLAUDE.md index e5375f03..10ac13a1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,8 +11,10 @@ on every sync. Add a file upstream does not have instead. ## Branches -- Work off `main`. `git fetch fork` first — a local `main` goes stale with no - signal, and branching off a stale one silently drops everything merged since. +- Work off `main`. Fetch first — a local `main` goes stale with no signal, and + branching off a stale one silently drops everything merged since. The remote + is `origin` in a fresh clone; if you cloned upstream and added this fork as a + second remote, use that name instead. - **Upstream PRs branch from upstream, not from `main`**, and are named `upstream/`. A branch cut from `main` carries our whole patch stack into the PR diff. @@ -22,8 +24,17 @@ on every sync. Add a file upstream does not have instead. git checkout -b upstream/ FETCH_HEAD ``` - `.github/workflows/upstream-branch-guard.yml` fails any `upstream/**` branch - that descends from `main`, on push, before a PR exists. + Before pushing, this must list only the commits you wrote — anything else is + a fork patch that would land in the upstream diff: + + ``` + git log --oneline FETCH_HEAD..HEAD + ``` + + `.github/workflows/upstream-branch-guard.yml` checks the same property on + push, before a PR exists. It tests where the branch diverged rather than + whether it contains `main`'s current tip, because `main` moves with every + merge and a branch cut from it last week contains today's tip nowhere. - Before raising an upstream PR, reproduce the bug **on upstream's tree** — apply the test alone, watch it fail, then apply the fix. A fix whose code From e8bd74e6d08cfc8111f1c1d960950358f11db1f2 Mon Sep 17 00:00:00 2001 From: Anoop Narang Date: Thu, 24 Sep 2026 19:05:45 +0530 Subject: [PATCH 3/3] docs: re-fetch upstream before the self-check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FETCH_HEAD` holds whatever the last fetch wrote. The bullet above tells the author to fetch their own remote, and after that `FETCH_HEAD` is our `main` — so `git log FETCH_HEAD..HEAD` would exclude every fork patch and show a contaminated branch as clean. The workflow's copy already repeated the fetch; this one now does too. --- CLAUDE.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 10ac13a1..be5bc539 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,9 +25,12 @@ on every sync. Add a file upstream does not have instead. ``` Before pushing, this must list only the commits you wrote — anything else is - a fork patch that would land in the upstream diff: + a fork patch that would land in the upstream diff. Re-fetch upstream on the + line above it: `FETCH_HEAD` holds whatever the last fetch wrote, so after a + `git fetch origin` it is *our* `main` and the check hides every fork patch. ``` + git fetch https://github.com/datafusion-contrib/liquid-cache main git log --oneline FETCH_HEAD..HEAD ```